Update module dependency to 0.6.0 #2
Conversation
WalkthroughThis update introduces version 0.6.0 of the SDK, making significant API and configuration changes across Android, iOS, TypeScript, and documentation. Key updates include new verification options ( Changes
Sequence Diagram(s)sequenceDiagram
participant JS as React Native JS
participant Native as Native Module (Android/iOS)
participant SDK as ReclaimInAppSdk
JS->>Native: setVerificationOptions({claimCreationType, canAutoSubmit, isCloseButtonVisible, ...})
Native->>SDK: setVerificationOptions(...)
SDK-->>Native: ack
Native-->>JS: ack
JS->>Native: onSessionCreateRequest()
Native->>SDK: createSession(appId, providerId, timestamp, signature)
SDK-->>Native: sessionId (string)
Native-->>JS: sessionId (string)
Possibly related PRs
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (3)
lib/module/index.js (1)
254-258:⚠️ Potential issueFix inconsistency in error handling for session creation
There's an inconsistency in how session creation responses are handled. Success cases use
replyWithStringto return a string result, but the error case still usesreplywith a boolean value.Apply this change to ensure consistent error handling:
try { let result = await sessionManagement.onSessionCreateRequest(event); NativeReclaimInappModule.replyWithString(replyId, result); } catch (error) { console.error(error); - NativeReclaimInappModule.reply(replyId, false); + NativeReclaimInappModule.replyWithString(replyId, ''); }lib/commonjs/index.js (2)
259-264:⚠️ Potential issueUse
replyWithString()for both success and failure paths ofonSessionCreateRequest.The callback contract for session creation has changed from a boolean to a string.
Returning a boolean in the failure branch (reply(replyId, false)) violates the new native signature and can crash on platforms that now exclusively deserialize a string.- } catch (error) { - console.error(error); - _NativeInappRnSdk.default.reply(replyId, false); + } catch (error) { + console.error(error); + // Empty string signals failure while respecting the expected type. + _NativeInappRnSdk.default.replyWithString(replyId, ''); }
318-339: 🛠️ Refactor suggestionDispose stale Attestor-Auth listeners and always pass deterministic booleans.
When
fetchAttestorAuthenticationRequestis removed on a subsequent call, the previous subscription survives, leaking listeners.
Add athis.disposeAttestorAuthRequestListener();before theif (canUseAttestorAuthenticationRequest)block.
canDeleteCookiesBeforeVerificationStartscan beundefined, yet the native layer treats it as non-nullable. Explicitly default tofalse(or your desired default) to avoid an “Expected Bool but got null” native exception.+ // Ensure we don’t leak an old subscription + this.disposeAttestorAuthRequestListener(); if (options) { … - args = { - canDeleteCookiesBeforeVerificationStarts: options.canDeleteCookiesBeforeVerificationStarts, + args = { + canDeleteCookiesBeforeVerificationStarts: + options.canDeleteCookiesBeforeVerificationStarts ?? false,
🧹 Nitpick comments (13)
CHANGELOG.md (2)
3-3: Refine bullet text for clarity
The phrase “Update claim creation updates UI” is redundant. Consider simplifying to “Update claim creation UI” for readability.
Apply this diff:- * Update claim creation updates UI + * Update claim creation UI
5-5: Standardize “in-app” terminology
For consistency, hyphenate “inapp” as “in-app” in the dependency line. For example:- * Updates inapp module dependency to 0.6.0 + * Update in-app module dependency to 0.6.0android/build.gradle (1)
121-121: Consider centralizing the SDK version
Right now the dependency version is hard-coded here. To avoid future drift, you could introduce aninappSdkVersioningradle.propertiesorbuildscript.extand reference it:- implementation "org.reclaimprotocol:inapp_sdk:0.6.0" + implementation "org.reclaimprotocol:inapp_sdk:${inappSdkVersion}"This makes bumping the SDK uniform across all Gradle files.
README.md (1)
64-66: Verify migration link formatting
The new migration step link for 0.6.0 currently appears as raw text. For consistency with the other list items, prepend it with a dash and ensure it’s rendered as a Markdown list item:- - Migration steps for [0.6.0](...) + - Migration steps for [0.6.0](...)documentation/migration.md (1)
5-10: Add determiner for clarity
Line 36 of the static analysis hints flagged a missing determiner. In the iOS section, revise:- Make sure if you are using the latest versions of `ReclaimInAppSdk` cocoapod if you have overriden this dependency in your `Podfile`. + Make sure that you are using the latest version of the `ReclaimInAppSdk` CocoaPod if you have overridden this dependency in your `Podfile`.This improves readability and corrects “CocoaPod” casing.
lib/typescript/commonjs/src/index.d.ts (2)
73-82: Add JSDoc & defaults for newly-added VerificationOptions.For consumers reading the typings (especially in editors), document the default values you apply in
PlatformImpl.setVerificationOptions()so behaviour is discoverable without source diving.Example:
/** * The claim construction mode. Defaults to `'standalone'`. */ claimCreationType?: 'standalone' | 'meChain';
108-114: Align the promise description with the new return type.
onSessionCreateRequestnow resolves to astring(sessionId) – not a boolean.
Clarify in the comment that rejecting the promise (or resolving with an empty string) aborts creation.src/specs/NativeInappRnSdk.ts (1)
267-274: Remove lingering “session ID” references in comments.The payload no longer contains
sessionId; keeping outdated docs misleads integrators.Update:
/** * The session timestamp … /// ... * The cryptographic signature … */ios/InappRnSdk.mm (1)
240-251: Constructor calls become brittle as parameter list grows.
ReclaimApiVerificationOptionsnow takes five positional arguments; a future adjustment will cascade fragile changes across both branches.
Consider adopting a designated initialiser pattern or Objective-CNSDictionaryto improve maintainability.src/index.ts (1)
123-138: Minor: document default values in the interface
claimCreationType,canAutoSubmit, andisCloseButtonVisibledefault to'standalone',true, andtruerespectively insetVerificationOptions, but the interface doc only mentions the last two defaults. Consider amending the JSDoc forclaimCreationTypeso callers do not assumeundefinedmeans no preference.android/src/main/java/com/reclaimprotocol/inapp_rn_sdk/InappRnSdkModule.kt (2)
314-323:metadatafromlogSessionis silently droppedThe new
logSessionsignature carries an optionalmetadatamap, but the bridge forwards onlylogType.
If JS clients need the extra context, emit it with the event:- args.putString("logType", logType) + args.putString("logType", logType) + metadata?.let { + args.putString("metadataJson", JSONObject(it).toString()) + }(Adjust the key & type as per the JS spec.)
175-181: Nit: redundant nullable-default logic
canDeleteCookiesBeforeVerificationStartsis forced totruewhen missing, but the corresponding TypeScript default is leftundefined.
For consistency, consider mirroring TS defaults or deferring to the SDK’s own default.ios/inapp_rn_sdk/Api.swift (1)
462-466:metadataargument not forwarded to JSThe updated
logSessionincludes ametadatadictionary but the bridge discards it:self._logSession(appId, providerId, sessionId, logType)Pass it along (e.g., as JSON) so React Native consumers can act on it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
android/generated/jni/react/renderer/components/RNInappRnSdkSpec/RNInappRnSdkSpecJSI.his excluded by!**/generated/**ios/generated/RNInappRnSdkSpec/RNInappRnSdkSpec.his excluded by!**/generated/**ios/generated/RNInappRnSdkSpecJSI.his excluded by!**/generated/**lib/commonjs/index.js.mapis excluded by!**/*.maplib/commonjs/specs/NativeInappRnSdk.js.mapis excluded by!**/*.maplib/module/index.js.mapis excluded by!**/*.maplib/module/specs/NativeInappRnSdk.js.mapis excluded by!**/*.maplib/typescript/commonjs/src/index.d.ts.mapis excluded by!**/*.maplib/typescript/commonjs/src/specs/NativeInappRnSdk.d.ts.mapis excluded by!**/*.maplib/typescript/module/src/index.d.ts.mapis excluded by!**/*.maplib/typescript/module/src/specs/NativeInappRnSdk.d.ts.mapis excluded by!**/*.map
📒 Files selected for processing (22)
CHANGELOG.md(1 hunks)InappRnSdk.podspec(1 hunks)README.md(5 hunks)android/build.gradle(3 hunks)android/src/main/java/com/reclaimprotocol/inapp_rn_sdk/InappRnSdkModule.kt(3 hunks)documentation/migration.md(1 hunks)example/android/app/src/main/AndroidManifest.xml(1 hunks)example/android/settings.gradle(1 hunks)example/src/App.overrides.tsx(0 hunks)ios/InappRnSdk.mm(4 hunks)ios/inapp_rn_sdk/Api.swift(7 hunks)lib/commonjs/index.js(2 hunks)lib/module/index.js(2 hunks)lib/typescript/commonjs/src/index.d.ts(2 hunks)lib/typescript/commonjs/src/specs/NativeInappRnSdk.d.ts(2 hunks)lib/typescript/module/src/index.d.ts(2 hunks)lib/typescript/module/src/specs/NativeInappRnSdk.d.ts(2 hunks)package.json(2 hunks)samples/example_new_arch/android/settings.gradle(1 hunks)samples/example_new_arch/package.json(1 hunks)src/index.ts(6 hunks)src/specs/NativeInappRnSdk.ts(3 hunks)
💤 Files with no reviewable changes (1)
- example/src/App.overrides.tsx
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/index.ts (1)
src/specs/NativeInappRnSdk.ts (1)
SessionCreateRequestEvent(257-278)
lib/typescript/commonjs/src/index.d.ts (1)
lib/typescript/commonjs/src/specs/NativeInappRnSdk.d.ts (1)
SessionCreateRequestEvent(224-245)
lib/typescript/module/src/index.d.ts (1)
lib/typescript/module/src/specs/NativeInappRnSdk.d.ts (1)
SessionCreateRequestEvent(224-245)
ios/inapp_rn_sdk/Api.swift (1)
android/src/main/java/com/reclaimprotocol/inapp_rn_sdk/InappRnSdkModule.kt (2)
createSession(296-312)fetchAttestorAuthenticationRequest(185-195)
🪛 LanguageTool
documentation/migration.md
[uncategorized] ~36-~36: A determiner appears to be missing. Consider inserting it.
Context: ...iden this dependency in your Podfile. Latest version on [cocoapods.org is 0.6.0](htt...
(AI_EN_LECTOR_MISSING_DETERMINER)
🔇 Additional comments (23)
samples/example_new_arch/package.json (1)
13-13: Update SDK dependency version
Upgraded@reclaimprotocol/inapp-rn-sdkto^0.6.0to align with the new release. Ensure the sample app is tested end-to-end with the updated SDK for any breaking API changes.example/android/settings.gradle (1)
10-10: Use version-agnostic Android Maven repository URL
Removed the hardcoded0.3.0segment to point at the generic/android/repopath, aligning with the 0.6.0 release. Confirm that both CI and local Gradle sync pick up the correct artifacts viaRECLAIM_STORAGE_BASE_URL.InappRnSdk.podspec (2)
14-14: Add upgrade guidance comment
The note pointing todocumentation/migration.mdand clarifying the iOS 13.0+ support is helpful for maintainers.
20-20: Bump iOS SDK dependency to 0.6.0
UpdatedReclaimInAppSdkto~> 0.6.0. Remember to runpod repo updateorpod update ReclaimInAppSdkand verify integration on an iOS 13.0+ device.package.json (3)
3-3: Update package version to 0.6.0
Bumped the package version to0.6.0. Ensure the changelog and release pipeline are updated to reflect this version.
11-11: Expose TS source via exports.source
Added asourceentry underexportspointing to./src/index.ts. This aids tooling and debugging—please verify that common bundlers and packagers support this field.
87-87: Loosen React Native devDependency version
Changedreact-nativefrom0.77.1to0.77to allow any patch release on the0.77line. Run the full test suite to confirm no regressions occur with newer patch releases.samples/example_new_arch/android/settings.gradle (1)
10-10: Use version-agnostic Android Maven URL
Removed the0.3.0version qualifier to point at/android/repofor the 0.6.0 SDK. Verify that Gradle synchronization succeeds and pulls the correct artifacts.android/build.gradle (2)
6-6: Repository URL updated to generic path
ThereclaimStorageUrldefault now omits the hard-coded version, pointing to/android/repoas intended by the migration docs. This matches the change across other Gradle files.
101-101: Consistent repository URL outside buildscript
You’ve mirrored the version-agnostic URL in the top-levelrepositoriesblock. Good catch—this keeps the build aligned.README.md (1)
63-64:Details
✅ Verification successful
Confirm consistency in repository URL examples
In the Android setup instructions you updatedreclaimStorageUrl, but double-check that every occurrence (in example settings.gradle and other docs) reflects the same/android/repopath.Run:
to ensure no old versioned URL remains.
🏁 Script executed:
rg "reclaimStorageUrl" -nLength of output: 2857
All
reclaimStorageUrlreferences are consistent
Verified that every code example in README.md, example/android/settings.gradle, android/build.gradle, and samples/example_new_arch/settings.gradle now uses the base “…/android/repo” URL. The only versioned paths live indocumentation/migration.md, where they’re intentionally shown for migration steps. No further changes needed.example/android/app/src/main/AndroidManifest.xml (2)
3-14: Review optional permissions for least privilege
You’ve added several optional permissions (CAMERA,LOCATION,RECORD_AUDIO, etc.). Ensure the SDK truly requires each of these and document their usage in the README; otherwise, you may be requesting more permissions than necessary.Please verify that each new permission maps to a feature in 0.6.0 and consider marking truly optional ones in your runtime permission requests.
22-23: Enable hardware acceleration
Great job explicitly enablingandroid:hardwareAccelerated="true"on both<application>and<activity>. This prevents graphical glitches on certain devices.Also applies to: 30-30
documentation/migration.md (1)
13-18: Migration snippet correctly updated
The diff example now shows removal of0.3.0in favor of/android/repo. It aligns with the Gradle changes and the README.lib/module/index.js (1)
317-321: Enhanced verification options successfully implementedThe implementation correctly adds the new verification options with sensible defaults:
claimCreationTypedefaults to 'standalone'canAutoSubmitdefaults to trueisCloseButtonVisibledefaults to trueThese align with the SDK update to version 0.6.0.
lib/typescript/module/src/index.d.ts (2)
73-81: Well-documented new verification optionsThe new verification options have been properly added with clear documentation:
claimCreationType: Controls the type of claim creation ('standalone' or 'meChain')canAutoSubmit: Controls automatic proof submissionisCloseButtonVisible: Controls close button visibilityAll marked as optional with clear documentation about defaults.
108-114: Session creation API updated correctlyThe return type for
onSessionCreateRequesthas been properly updated fromPromise<boolean>toPromise<string>, with improved documentation clarifying that it now returns a session ID.This aligns with the SDK's new session management approach in version 0.6.0.
lib/typescript/module/src/specs/NativeInappRnSdk.d.ts (2)
233-240: Updated session event propertiesThe
SessionCreateRequestEventinterface has been correctly updated to replace thesessionIdproperty withtimestampandsignatureproperties, aligning with the SDK's new session management approach.
282-293: New verification options well-documentedThe required properties added to the
VerificationOptionsinterface are properly typed and documented:
claimCreationType: Union type of 'standalone' | 'meChain'canAutoSubmit: Boolean with clear documentationisCloseButtonVisible: Boolean with clear documentationThis provides proper type checking for the new options.
lib/typescript/commonjs/src/specs/NativeInappRnSdk.d.ts (2)
233-240: Updated session event propertiesThe
SessionCreateRequestEventinterface has been correctly updated to replace thesessionIdproperty withtimestampandsignatureproperties, aligning with the SDK's new session management approach.
282-293: Verify type definition consistency across filesWhile the implementation and documentation of these new properties are correct, note that these properties are required here but optional in
lib/typescript/module/src/index.d.ts. This appears intentional based on how the API is used in different contexts, but should be verified.Confirm that this difference in optionality between the interfaces is intentional:
- In
lib/typescript/module/src/index.d.ts: Properties are optional (claimCreationType?, etc.)- In this file: Properties are required (no
?modifier)The implementation in
PlatformImpl.setVerificationOptions()does provide default values, which supports this design.src/specs/NativeInappRnSdk.ts (1)
320-333: Mark booleans as optional or provide defaults to avoid breaking callers.
canAutoSubmit,isCloseButtonVisible, etc. are required here but optional in the public wrapper.
Either:
- Make them optional (
?:) and default internally, or- Keep them required and synchronise the public
.d.ts.Current mismatch can surface as type-errors in consumer projects that import the spec typings directly.
ios/inapp_rn_sdk/Api.swift (1)
103-106: StaleautoSubmitparameter may break on next SDK bump
ReclaimVerification.Request.Paramsis now documented as not takingautoSubmit, yet the initializer still passesautoSubmit: true // unused.
If the Swift SDK removes this parameter in 0.6.x, this call site will no longer compile.- autoSubmit: true, // unused.Simply drop the argument to future-proof the code.
| OverridenSessionManagement * _Nullable sessionManagement; | ||
| if (overrides.sessionManagement().has_value()) { | ||
| sessionManagement = [[OverridenSessionManagement alloc] initWithHandler:[[OverridenSessionHandler alloc] initWith_createSession:^(NSString * _Nonnull appId, NSString * _Nonnull providerId, NSString * _Nonnull sessionId, NSString * _Nonnull replyId) { | ||
| sessionManagement = [[OverridenSessionManagement alloc] initWithHandler:[[OverridenSessionHandler alloc] initWith_createSession:^(NSString * _Nonnull appId, NSString * _Nonnull providerId, NSString * _Nonnull timestamp, NSString * _Nonnull signature, NSString * _Nonnull replyId) { | ||
| [self emitOnSessionCreateRequest:@{ | ||
| @"appId": appId, | ||
| @"providerId": providerId, | ||
| @"sessionId": sessionId, | ||
| @"timestamp": timestamp, | ||
| @"signature": signature, | ||
| @"replyId": replyId | ||
| }]; |
There was a problem hiding this comment.
Uninitialised pointer may pass garbage into native layer.
OverridenSessionManagement * _Nullable sessionManagement;sessionManagement is not initialised to nil and may retain an arbitrary pointer when overrides.sessionManagement() is absent, leading to undefined behaviour inside [api setOverridesWithProvider:… sessionManagement:sessionManagement …].
-OverridenSessionManagement * _Nullable sessionManagement;
+OverridenSessionManagement * _Nullable sessionManagement = nil;| NativeReclaimInappModule.onSessionCreateRequest(async (event) => { | ||
| const replyId = event.replyId; | ||
| try { | ||
| let result = await sessionManagement.onSessionCreateRequest(event); | ||
| NativeReclaimInappModule.reply(replyId, result); | ||
| NativeReclaimInappModule.replyWithString(replyId, result); | ||
| } catch (error) { | ||
| console.error(error); | ||
| NativeReclaimInappModule.reply(replyId, false); |
There was a problem hiding this comment.
Use replyWithString on the failure branch as well
onSessionCreateRequest now expects a string reply.
The success path correctly calls replyWithString, but the failure path still calls reply, which is wired to a boolean callback on the native side. This will leave the awaiting Kotlin / Swift handler without a reply and may trigger a memory leak or hang.
- } catch (error) {
- console.error(error);
- NativeReclaimInappModule.reply(replyId, false);
+ } catch (error) {
+ console.error(error);
+ // Send an empty string (or your preferred sentinel) to indicate failure
+ NativeReclaimInappModule.replyWithString(replyId, '');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| NativeReclaimInappModule.onSessionCreateRequest(async (event) => { | |
| const replyId = event.replyId; | |
| try { | |
| let result = await sessionManagement.onSessionCreateRequest(event); | |
| NativeReclaimInappModule.reply(replyId, result); | |
| NativeReclaimInappModule.replyWithString(replyId, result); | |
| } catch (error) { | |
| console.error(error); | |
| NativeReclaimInappModule.reply(replyId, false); | |
| NativeReclaimInappModule.onSessionCreateRequest(async (event) => { | |
| const replyId = event.replyId; | |
| try { | |
| let result = await sessionManagement.onSessionCreateRequest(event); | |
| NativeReclaimInappModule.replyWithString(replyId, result); | |
| } catch (error) { | |
| console.error(error); | |
| // Send an empty string (or your preferred sentinel) to indicate failure | |
| NativeReclaimInappModule.replyWithString(replyId, ''); | |
| } | |
| }); |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores