-
Notifications
You must be signed in to change notification settings - Fork 2
Updates inapp module dependency to 0.9.0 #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update advances the SDK to version 0.9.0, updates related dependencies across platforms, and introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant ReactNativeApp
participant InappRnSdkModule (Android/iOS)
participant ProviderCallbackHandler
ReactNativeApp->>InappRnSdkModule (Android/iOS): setOverrides(callback)
InappRnSdkModule (Android/iOS)->>ProviderCallbackHandler: fetchProviderInformation(...)
ProviderCallbackHandler-->>InappRnSdkModule (Android/iOS): (appId, providerId, sessionId, signature, timestamp, resolvedVersion, replyId)
InappRnSdkModule (Android/iOS)-->>ReactNativeApp: emitOnProviderInformationRequest({..., resolvedVersion, replyId})
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
android/src/main/java/com/reclaimprotocol/inapp_rn_sdk/InappRnSdkModule.kt (1)
91-97
:providerVersion
always instantiated – may introduce empty stringsIf the caller omits
providerVersion
, we still construct:ProviderVersion(resolvedVersion = "", versionExpression = "")Downstream native code may treat an empty
resolvedVersion
as invalid or “version 0”.
Safer pattern:- val providerVersion = request.getMap("providerVersion").let { ... + val providerVersion = request.getMap("providerVersion")?.let { pv -> + ProviderVersion( + resolvedVersion = getString(pv, "resolvedVersion") ?: "", + versionExpression = getString(pv, "versionExpression") ?: "" + ) + }and pass
providerVersion = providerVersion
(nullable) to avoid changing previous behaviour.ios/inapp_rn_sdk/Api.swift (1)
90-94
: Gracefully handle missing keys instead of falling back to empty stringsPassing an empty string when either
resolvedVersion
orversionExpression
is absent means “version""
” rather than “no specific version”, which may be semantically incorrect downstream. Consider initialising withnil
(if the SDK allows optionals) or throwing early to surface the configuration error.
🧹 Nitpick comments (8)
android/build.gradle (1)
116-122
: Extract version to Gradle property for maintainability.Consider defining
inappSdkVersion
inrootProject.ext
and usinggetExtOrDefault('inappSdkVersion')
to avoid manual sync across files.CHANGELOG.md (1)
1-5
: Minor consistency nit – use present-tense verbs for new entriesMost earlier entries start with a verb in third-person singular (e.g. “Updates inapp module dependency …”).
Line 3 currently reads “Add resolvedVersion …”, while line 4 reads “Updates …”. Consider either:* Adds resolvedVersion … * Updates inapp module …
or
* Add resolvedVersion … * Update inapp module …
for stylistic consistency.
src/specs/NativeInappRnSdk.ts (1)
325-336
: Document the newresolvedVersion
field & confirm it is truly mandatory
resolvedVersion
is introduced as a requiredstring
.
Two follow-ups:
- Add a short JSDoc comment explaining what the value represents (e.g. “Concrete provider version resolved from the expression”).
- Double-check that all event emitters (Android/iOS) will always have a non-empty value; otherwise mark it as optional (
resolvedVersion?: string
) to avoid a breaking change for existing TypeScript consumers.documentation/migration.md (1)
5-8
: Grammar / spelling
overriden
→overridden
... if you have overridden this dependency ...
README.md (1)
111-120
: Podfile snippets – keep version constraints in sync with migration docsYou pin the CocoaPod with
~> 0.9.0
here, while the migration guide for 0.9.0 explicitly recommendspod install --repo-update
without pinning.
Ensure both examples convey the same recommendation to avoid conflicting advice.Also applies to: 155-157
ios/inapp_rn_sdk/Api.swift (1)
345-353
: Completion handler never receives errors from the callback
fetchProviderInformation
forwards the reply throughApi.setReplyWithStringCallback
, but there is no path to propagate an error back to Swift if the JS side fails (e.g. rejects a promise). If the underlying SDK supports it, exposing a failure channel would improve debuggability.android/gradlew.bat (1)
56-63
:%JAVA_EXE%
path uses forward slashesWindows happily accepts them most of the time, but quoting & space handling can fail on very old shells or exotic setups (
C:\Program Files\Java\...
). Using back-slashes (%JAVA_HOME%\bin\java.exe
) matches the conventional Gradle wrapper template and avoids surprises.android/gradlew (1)
204-206
: Default heap size is quite low for modern Gradle builds
"-Xmx64m"
may lead to OOMs on moderate Android projects. Consider bumping to at least256m
(Gradle’s current default) unless CI constraints require otherwise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
android/generated/jni/react/renderer/components/RNInappRnSdkSpec/RNInappRnSdkSpecJSI.h
is excluded by!**/generated/**
android/gradle/wrapper/gradle-wrapper.jar
is excluded by!**/*.jar
ios/generated/RNInappRnSdkSpecJSI.h
is excluded by!**/generated/**
📒 Files selected for processing (14)
CHANGELOG.md
(1 hunks)InappRnSdk.podspec
(1 hunks)README.md
(3 hunks)android/build.gradle
(1 hunks)android/gradle/wrapper/gradle-wrapper.properties
(1 hunks)android/gradlew
(1 hunks)android/gradlew.bat
(1 hunks)android/src/main/java/com/reclaimprotocol/inapp_rn_sdk/InappRnSdkModule.kt
(2 hunks)documentation/migration.md
(3 hunks)ios/InappRnSdk.mm
(1 hunks)ios/inapp_rn_sdk/Api.swift
(3 hunks)package.json
(1 hunks)samples/example_new_arch/package.json
(1 hunks)src/specs/NativeInappRnSdk.ts
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
documentation/migration.md
7-7: Bare URL used
null
(MD034, no-bare-urls)
13-13: Bare URL used
null
(MD034, no-bare-urls)
🔇 Additional comments (9)
android/gradle/wrapper/gradle-wrapper.properties (1)
1-7
: Add Gradle wrapper properties for consistent builds.This file correctly configures the Gradle wrapper to use Gradle 8.12 with a 10-second network timeout and proper cache paths. Everything looks in order.
samples/example_new_arch/package.json (1)
14-14
: Bump sample project SDK dependency to ^0.9.0.Version ^0.9.0 aligns the sample with the main SDK bump. No issues detected.
InappRnSdk.podspec (1)
20-20
: Update CocoaPods dependency to ~> 0.9.0.The podspec now targets ReclaimInAppSdk 0.9.x, matching the new SDK version.
package.json (1)
3-3
: Update package version to 0.9.0.The package.json version bump aligns with releases and other dependency updates.
android/build.gradle (1)
121-121
: Update Android in-app SDK dependency to 0.9.0.The implementation dependency is correctly updated to match the SDK version bump.
android/src/main/java/com/reclaimprotocol/inapp_rn_sdk/InappRnSdkModule.kt (1)
270-281
: Event payload – make sure the JS side updates match
args.putString("resolvedVersion", resolvedVersion)
correctly forwards the new field.Action item: bump the event listener implementation in JS to read this extra argument, otherwise users will silently ignore it.
ios/inapp_rn_sdk/Api.swift (2)
90-94
: Avoid shadowing theproviderVersion
parameterRe-using the same identifier for both the incoming
[String: String]?
argument and the newly constructedProviderVersion
instance hurts readability and makes debugging harder. A tiny rename keeps intent clear:- let providerVersion = ReclaimVerification.Request.Params.ProviderVersion( + let providerVer = ReclaimVerification.Request.Params.ProviderVersion(and update the uses below.
[ suggest_nitpick ]
323-331
: Verify every objective-C/Swift/JS call site now suppliesresolvedVersion
The callback signature changed to:
(appId, providerId, sessionId, signature, timestamp, resolvedVersion, replyId)
Double-check that older clients, tests, or demo apps were updated; otherwise the bridge will crash at runtime with an “unrecognised selector” error.
ios/InappRnSdk.mm (1)
158-169
: Keep the emitted payload aligned with the TS interface
resolvedVersion
is now added to the dictionary. EnsureNativeEventEmitter
/ TS typings reflect the new field, otherwise consumers will silently ignore it.
## 0.8.3 | ||
|
||
- Make sure if you are using the latest versions of `ReclaimInAppSdk` cocoapod if you have overriden this dependency in your `Podfile`. Latest version on [cocoapods.org is 0.8.3](https://cocoapods.org/pods/ReclaimInAppSdk). | ||
- Make sure if you are using the latest versions of `ReclaimInAppSdk` cocoapod if you have overriden this dependency in your `Podfile`. Latest version on [cocoapods.org is 0.9.0](https://cocoapods.org/pods/ReclaimInAppSdk). | ||
- Run a `pod install --repo-update`. If this fails for reasons related to the `ReclaimInAppSdk`, try running `pod update ReclaimInAppSdk`. | ||
- Refer: https://github.com/reclaimprotocol/reclaim-inapp-reactnative-sdk/blob/main/README.md#ios-setup | ||
|
||
## 0.7.3 | ||
|
||
### iOS | ||
|
||
- Make sure if you are using the latest versions of `ReclaimInAppSdk` cocoapod if you have overriden this dependency in your `Podfile`. Latest version on [cocoapods.org is 0.7.0](https://cocoapods.org/pods/ReclaimInAppSdk). | ||
- Make sure if you are using the latest versions of `ReclaimInAppSdk` cocoapod if you have overriden this dependency in your `Podfile`. Latest version on [cocoapods.org is 0.9.0](https://cocoapods.org/pods/ReclaimInAppSdk). | ||
- Run a `pod install --repo-update`. If this fails for reasons related to the `ReclaimInAppSdk`, try running `pod update ReclaimInAppSdk`. | ||
- Refer: https://github.com/reclaimprotocol/reclaim-inapp-reactnative-sdk/blob/main/README.md#ios-setup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Older migration sections now point to 0.9.0 – likely misleading
Inside the 0.8.3, 0.7.3 and 0.3.0 sections, the text now tells users to install version 0.9.0 of ReclaimInAppSdk
.
That overwrites the historical instructions and defeats the purpose of keeping per-version migration notes.
Restore the original version numbers (0.8.3, 0.7.3, 0.3.0, …) inside those legacy blocks or add an explicit note that using the latest pod is also acceptable.
Also applies to: 98-101, 137-139
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
13-13: Bare URL used
null
(MD034, no-bare-urls)
21-21: Bare URL used
null
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In documentation/migration.md around lines 9 to 21 and also lines 98 to 101 and
137 to 139, the migration notes for versions 0.8.3, 0.7.3, and 0.3.0 incorrectly
instruct users to install version 0.9.0 of ReclaimInAppSdk, which misrepresents
the historical instructions. To fix this, revert the pod version references in
these legacy migration sections back to their respective original versions
(0.8.3, 0.7.3, 0.3.0) or alternatively add a clear note stating that using the
latest pod version 0.9.0 is also acceptable, preserving the integrity of the
per-version migration guidance.
Summary by CodeRabbit
New Features
resolvedVersion
, in provider information requests, enhancing the data available to the app when fetching provider details.Documentation
Chores