Update package module#45
Conversation
|
Mushaheed Syed seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughThis PR bumps the SDK to 0.34.0, adds three new feature-option URL fields (privacyPolicyUrl, termsOfServiceUrl, potentialFailureReasonsUrl) across TS/Android/iOS bindings, updates example defaults to "example", and makes several iOS/project build and toolchain config adjustments. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
user-workspace/index.js (1)
7-11: Standard React Native Web bootstrap pattern looks good.The browser environment guard using
typeof document !== 'undefined'is the correct idiomatic approach for React Native Web support.As an optional defensive improvement, you could add a null check for the root element to provide a clearer error message if the HTML is misconfigured:
🛡️ Optional: Add null safety for rootTag
if (typeof document !== 'undefined') { + const rootTag = document.getElementById('root'); + if (!rootTag) { + throw new Error('Root element with id "root" not found in DOM'); + } AppRegistry.runApplication(appName, { - rootTag: document.getElementById('root'), + rootTag, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user-workspace/index.js` around lines 7 - 11, Add a null check for the root element returned by document.getElementById('root') before calling AppRegistry.runApplication; if the element is null, log or throw a clear error (e.g., using console.error or throw new Error) explaining the missing/misconfigured root element so startup fails fast and with a helpful message, then only call AppRegistry.runApplication(appName, { rootTag }) when rootTag is non-null.turbo.json (1)
39-39: RemoveRCT_REMOVE_LEGACY_ARCHfrombuild:ios.envin turbo.json — it's not consumed anywhere in the repository.This variable appears only in turbo.json and is not used by any iOS build scripts, Podfiles, or CI workflows. Keeping it unnecessarily broadens Turbo's cache invalidation surface without providing any build impact.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@turbo.json` at line 39, Remove the unused environment variable entry "RCT_REMOVE_LEGACY_ARCH" from the build:ios.env array in turbo.json; locate the build:ios.env block in turbo.json and delete the "RCT_REMOVE_LEGACY_ARCH" item so Turbo's cache invalidation surface is not broadened by an unused variable.user-workspace/src/App.tsx (1)
32-32: Defaulting TEE mode totrueoverrides the SDK's auto default.Line 32 initializes
selectedTeeOptionValuewithtrue, which forcesuseTeeOperator: trueon every verification call. The SDK defaults tonull(feature-flag driven), but this workspace explicitly enables TEE. To align with the SDK's auto mode, initialize withnull:♻️ Suggested change
- const [selectedTeeOptionValue, setSelectedTeeOptionValue] = useState<boolean | null>(true); + const [selectedTeeOptionValue, setSelectedTeeOptionValue] = useState<boolean | null>(null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user-workspace/src/App.tsx` at line 32, selectedTeeOptionValue is being initialized to true which forces useTeeOperator to true on every verification call; change the useState initial value for selectedTeeOptionValue in App.tsx from true to null so the SDK can use its feature-flag-driven default (update the useState(...) call that declares selectedTeeOptionValue and setSelectedTeeOptionValue).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 8: Fix the typo in the changelog entry: update the line "* Fix an issue
where some websites where not trusted to run in webview" to use "were" instead
of "where" so it reads "* Fix an issue where some websites were not trusted to
run in webview".
In `@documentation/migration.md`:
- Line 5: Replace the repeated misspelling "overriden" with the correct
"overridden" in the migration documentation wherever it appears (e.g., in the
sentence containing "Make sure if you are using the latest versions of
`ReclaimInAppSdk` cocoapod if you have overriden this dependency in your
`Podfile`"), and update all other occurrences listed (lines referenced in the
review: 5, 11, 17, 23, 29, 35, 41, 49, 128, 167) so every instance of the token
"overriden" is changed to "overridden".
In `@samples/example_new_arch/src/App.overrides.tsx`:
- Line 39: The inline comment "// originally from \"example\"," is ambiguous;
replace it with an instruction-style comment that tells users what to change
(e.g., "Replace with your own provider ID" or "Set to your provider's ID, e.g.,
'my-provider'") so it's clear what value to supply; update the comment near the
same location in App.overrides.tsx to reference "provider ID" or the specific
identifier expected by the surrounding config/code.
In `@user-workspace/src/App.overrides.tsx`:
- Line 32: Prettier formatting errors exist on the changed lines — fix the
formatting for the state declaration and the JSX block around lines 141-143 so
they match the project's Prettier rules; update the const [inputText,
setInputText] = useState('example') line to follow the repo's spacing/quote
conventions and reformat the JSX using the component/prop names around the block
at lines 141-143 (where the offending JSX lives) or simply run the project's
Prettier/format command to automatically apply the correct spacing, line breaks,
and wrapping so lint/CI passes.
---
Nitpick comments:
In `@turbo.json`:
- Line 39: Remove the unused environment variable entry "RCT_REMOVE_LEGACY_ARCH"
from the build:ios.env array in turbo.json; locate the build:ios.env block in
turbo.json and delete the "RCT_REMOVE_LEGACY_ARCH" item so Turbo's cache
invalidation surface is not broadened by an unused variable.
In `@user-workspace/index.js`:
- Around line 7-11: Add a null check for the root element returned by
document.getElementById('root') before calling AppRegistry.runApplication; if
the element is null, log or throw a clear error (e.g., using console.error or
throw new Error) explaining the missing/misconfigured root element so startup
fails fast and with a helpful message, then only call
AppRegistry.runApplication(appName, { rootTag }) when rootTag is non-null.
In `@user-workspace/src/App.tsx`:
- Line 32: selectedTeeOptionValue is being initialized to true which forces
useTeeOperator to true on every verification call; change the useState initial
value for selectedTeeOptionValue in App.tsx from true to null so the SDK can use
its feature-flag-driven default (update the useState(...) call that declares
selectedTeeOptionValue and setSelectedTeeOptionValue).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7d7f9028-2e35-4307-a98a-2ec90bf9e8d0
⛔ Files ignored due to path filters (2)
user-workspace/ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (22)
CHANGELOG.mdInappRnSdk.podspecandroid/build.gradleandroid/src/main/java/com/reclaimprotocol/inapprnsdk/InappRnSdkModule.ktdocumentation/install-no-framework.mddocumentation/migration.mddocumentation/overrides.mdios/InappRnSdk.mmios/inapp_rn_sdk/Api.swiftpackage.jsonsamples/example_expo/package.jsonsamples/example_new_arch/package.jsonsamples/example_new_arch/src/App.overrides.tsxsamples/example_new_arch/src/App.tsxsrc/NativeInappRnSdk.tsturbo.jsonuser-workspace/index.jsuser-workspace/ios/InappRnSdkExample.xcodeproj/project.pbxprojuser-workspace/ios/InappRnSdkExample/Info.plistuser-workspace/ios/Podfileuser-workspace/src/App.overrides.tsxuser-workspace/src/App.tsx
| * Update [internal] dependencies | ||
| * Update [internal] tests | ||
| * Update webview provider | ||
| * Fix an issue where some websites where not trusted to run in webview |
There was a problem hiding this comment.
Typo: "where" should be "were".
Minor grammatical error in the changelog entry.
📝 Proposed fix
-* Fix an issue where some websites where not trusted to run in webview
+* Fix an issue where some websites were not trusted to run in webview📝 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.
| * Fix an issue where some websites where not trusted to run in webview | |
| * Fix an issue where some websites were not trusted to run in webview |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` at line 8, Fix the typo in the changelog entry: update the line
"* Fix an issue where some websites where not trusted to run in webview" to use
"were" instead of "where" so it reads "* Fix an issue where some websites were
not trusted to run in webview".
| ## 0.15.0 | ||
|
|
||
| - 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.29.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.34.0](https://cocoapods.org/pods/ReclaimInAppSdk). |
There was a problem hiding this comment.
Fix repeated typo in migration docs (overriden → overridden).
The same spelling issue appears on Line 5, Line 11, Line 17, Line 23, Line 29, Line 35, Line 41, Line 49, Line 128, and Line 167.
Also applies to: 11-11, 17-17, 23-23, 29-29, 35-35, 41-41, 49-49, 128-128, 167-167
🧰 Tools
🪛 LanguageTool
[grammar] ~5-~5: Ensure spelling is correct
Context: ... ReclaimInAppSdk cocoapod if you have overriden this dependency in your Podfile. Late...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@documentation/migration.md` at line 5, Replace the repeated misspelling
"overriden" with the correct "overridden" in the migration documentation
wherever it appears (e.g., in the sentence containing "Make sure if you are
using the latest versions of `ReclaimInAppSdk` cocoapod if you have overriden
this dependency in your `Podfile`"), and update all other occurrences listed
(lines referenced in the review: 5, 11, 17, 23, 29, 35, 41, 49, 128, 167) so
every instance of the token "overriden" is changed to "overridden".
| return JSON.stringify({ | ||
| "id": "669eca16d7e0758c94dfc03f", | ||
| // originally from "6d3f6753-7ee6-49ee-a545-62f1b1822ae5", | ||
| // originally from "example", |
There was a problem hiding this comment.
Clarify the inline comment to avoid confusion.
On Line 39, // originally from "example", is ambiguous and doesn’t guide users on what to do. Prefer an instruction-style comment (e.g., replace with your own provider ID).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@samples/example_new_arch/src/App.overrides.tsx` at line 39, The inline
comment "// originally from \"example\"," is ambiguous; replace it with an
instruction-style comment that tells users what to change (e.g., "Replace with
your own provider ID" or "Set to your provider's ID, e.g., 'my-provider'") so
it's clear what value to supply; update the comment near the same location in
App.overrides.tsx to reference "provider ID" or the specific identifier expected
by the surrounding config/code.
| export default function App() { | ||
| const [verificationMethod, setVerificationMethod] = useState<VerificationMode>('providerId'); | ||
| const [inputText, setInputText] = useState('6d3f6753-7ee6-49ee-a545-62f1b1822ae5'); | ||
| const [inputText, setInputText] = useState('example'); |
There was a problem hiding this comment.
Fix prettier violations on changed lines to prevent lint failure.
Line 32 and Lines 141-143 are currently failing formatting checks from static analysis.
Proposed formatting fix
- const [inputText, setInputText] = useState('example');
+ const [inputText, setInputText] = useState('example');
- // privacyPolicyUrl: 'https://reclaimprotocol.org/privacy-policy',
- // termsOfServiceUrl: 'https://reclaimprotocol.org/terms-of-service',
- // potentialFailureReasonsUrl: 'https://reclaimprotocol.org/potential-failure-reasons',
+ // privacyPolicyUrl: 'https://reclaimprotocol.org/privacy-policy',
+ // termsOfServiceUrl: 'https://reclaimprotocol.org/terms-of-service',
+ // potentialFailureReasonsUrl: 'https://reclaimprotocol.org/potential-failure-reasons',Also applies to: 141-143
🧰 Tools
🪛 ESLint
[error] 32-32: Delete ··
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@user-workspace/src/App.overrides.tsx` at line 32, Prettier formatting errors
exist on the changed lines — fix the formatting for the state declaration and
the JSX block around lines 141-143 so they match the project's Prettier rules;
update the const [inputText, setInputText] = useState('example') line to follow
the repo's spacing/quote conventions and reformat the JSX using the
component/prop names around the block at lines 141-143 (where the offending JSX
lives) or simply run the project's Prettier/format command to automatically
apply the correct spacing, line breaks, and wrapping so lint/CI passes.
|
Promptless prepared a documentation update related to this change. Triggered by PR #45 Added documentation for the new Review at https://app.gopromptless.ai/suggestions/4e8fde15-7c3a-4074-b53f-4e6c7a96d632 |
3 participants
Summary by CodeRabbit
New Features
Bug Fixes
Chores