Conversation
WalkthroughThis PR removes the StorageAPI file-upload implementation and its tests, and changes the React Native package index to re-export Formbricks as the default. Several timestamp calculations are simplified to use Date.now(). The setup sync check (shouldSyncConfig) now compares only environmentId and appUrl; handleErrorOnFirstSetup uses Date.now(). Survey listener notification switches to for-of iteration over the Set. shouldDisplayBasedOnPercentage now uses a direct Math.random() probability check. Other edits are formatting and comment cleanups. Pre-merge checks❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (3)
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
packages/react-native/src/index.ts(1 hunks)packages/react-native/src/lib/common/file-upload.ts(0 hunks)packages/react-native/src/lib/common/setup.ts(2 hunks)packages/react-native/src/lib/common/tests/file-upload.test.ts(0 hunks)packages/react-native/src/lib/common/utils.ts(2 hunks)packages/react-native/src/lib/environment/state.ts(1 hunks)packages/react-native/src/lib/survey/store.ts(1 hunks)packages/react-native/src/lib/user/state.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/react-native/src/lib/common/file-upload.ts
- packages/react-native/src/lib/common/tests/file-upload.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/react-native/src/lib/common/utils.ts (1)
packages/react-native/src/types/error.ts (1)
Result(11-11)
🔇 Additional comments (6)
packages/react-native/src/lib/environment/state.ts (1)
101-101: LGTM: Consistent Date.now()-based expiry calculationClearer and consistent with other modules.
packages/react-native/src/lib/user/state.ts (1)
38-38: LGTM: Expiry calculation aligned to Date.now()Matches environment state handling.
packages/react-native/src/lib/common/utils.ts (1)
18-30: LGTM: wrapThrowsAsync formatting onlyNo behavior change; reads cleaner.
packages/react-native/src/lib/common/setup.ts (2)
415-415: LGTM: Date.now() for error expiryConsistent and clear 10‑minute window computation.
229-231: Remove unnecessaryenvironmentpresence check in shouldSyncConfigCondition still requires
existingConfig.environment, which contradicts the PR goal and can skip valid syncs when env is absent but ids match.- return Boolean( - existingConfig?.environment && - existingConfig.environmentId === configInput.environmentId && - existingConfig.appUrl === configInput.appUrl - ); + return Boolean( + existingConfig?.environmentId === configInput.environmentId && + existingConfig?.appUrl === configInput.appUrl + );Likely an incorrect or invalid review comment.
packages/react-native/src/index.ts (1)
41-41: Potential export mismatch: re‑export assumes namedFormbricksFile: packages/react-native/src/index.ts (line 41) — the current re‑export expects a named export
Formbricks; if the target module exports a default, replace with the default re‑export:-export { Formbricks as default } from "@/components/formbricks"; +export { default } from "@/components/formbricks";
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
packages/react-native/src/lib/common/utils.ts(2 hunks)packages/react-native/src/lib/survey/store.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/react-native/src/lib/common/utils.ts (1)
packages/react-native/src/types/error.ts (1)
Result(11-11)
🔇 Additional comments (2)
packages/react-native/src/lib/survey/store.ts (2)
23-25: Good fix: iterate a snapshot of listenersUsing a spread copy avoids mid-iteration mutation issues. LGTM.
33-35: Consistent snapshot iteration in resetMirrors the set path and prevents skipped/double notifications. LGTM.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/react-native/src/lib/common/utils.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-24T10:29:57.939Z
Learnt from: Dhruwang
PR: formbricks/react-native#27
File: packages/react-native/src/lib/common/utils.ts:186-187
Timestamp: 2025-09-24T10:29:57.939Z
Learning: User claims there are checks to ensure displayPercentage is not zero in the Formbricks React Native codebase, but no such validation logic was found in the code search results.
Applied to files:
packages/react-native/src/lib/common/utils.ts
🧬 Code graph analysis (1)
packages/react-native/src/lib/common/utils.ts (1)
packages/react-native/src/types/error.ts (1)
Result(11-11)
🔇 Additional comments (1)
packages/react-native/src/lib/common/utils.ts (1)
18-30: Normalize non-Error throws; avoid unsafe castEnsure Result.error is always an Error and preserve a useful message.
- async (...args: A): Promise<Result<T>> => { - try { - return { - ok: true, - data: await fn(...args), - }; - } catch (error) { - return { - ok: false, - error: error as Error, - }; - } - }; + async (...args: A): Promise<Result<T>> => { + try { + return { + ok: true, + data: await fn(...args), + }; + } catch (error: unknown) { + const err = + error instanceof Error + ? error + : new Error( + typeof error === "string" + ? error + : `Non-Error thrown: ${JSON.stringify(error)}` + ); + return { ok: false, error: err }; + } + };
2 participants
No description provided.