Conversation
WalkthroughThis pull request introduces infrastructure to enforce exact dependency version pinning across the monorepo. New configuration files ( 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/check-exact-deps.mjs`:
- Line 57: The directory traversal using for (const entry of
fs.readdirSync(dirPath, { withFileTypes: true })) is non-deterministic; call
fs.readdirSync into a variable (e.g., entries), sort the Dirent objects by their
name (entries.sort((a,b) => a.name.localeCompare(b.name))) and then iterate over
the sorted list so the violation lists produced by the script
(scripts/check-exact-deps.mjs) are stable and deterministic across runs and CI.
- Around line 25-27: The JSON read/parse of each manifest is unwrapped and will
throw without indicating which file failed; wrap the fs.readFileSync/JSON.parse
for each manifestPath in a try/catch, and on error log or print the failing
manifestPath (manifestPath) along with the error before exiting; update the loop
that reads manifestPaths and the code around JSON.parse(fs.readFileSync(...)) to
catch errors and include manifestPath context in the error message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8133ed50-9826-4ea4-bb65-383f9d4092db
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.ncurc.json.npmrcapps/playground/.npmrcpackage.jsonpackages/react-native/.eslintrc.cjspackages/react-native/package.jsonscripts/check-exact-deps.mjs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa056e99a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
pandeymangg
left a comment
pandeymangg
left a comment
There was a problem hiding this comment.
changes look good, I just left two comments 🙏
| ); | ||
| }); | ||
|
|
||
| test("reloads config after migrating legacy user state", async () => { |
There was a problem hiding this comment.
I think we can now remove the migration code from the setup file, its been a pretty long time since this was added, what do you think?
| }; | ||
|
|
||
| export { Formbricks as default } from "@/components/formbricks"; | ||
| export { Formbricks } from "@/components/formbricks"; |
There was a problem hiding this comment.
I think this makes Formbricks as a named export instead of default
Using import Formbricks from "@formbricks/react-native" errors out in the playground and you can only import Formbricks as a named import import {Formbricks} from "@formbricks/react-native". This would require atleast a minor release, do we want to do that? Otherwise we should ignore the lint error
2 participants
Summary
.npmrc,.ncurc.json, andscripts/check-exact-deps.mjsReviewer Note
pnpm lintwas previously blocked by a broken ESLint setup (packages/react-native/.eslintrc.cjsand a missing@vitest/eslint-plugin). Once that was fixed, the package had many existing lint violations, so one broad cleanup commit updated a lot of files.packages/react-native/srcis lint-driven and mechanical. The main logic changes to review are insrc/lib/common/setup.ts,src/lib/common/storage.ts,src/lib/common/event-listeners.ts, and the added/expanded tests.Suggested Review Order
package.json,.npmrc,apps/playground/.npmrc,.ncurc.json,scripts/check-exact-deps.mjs,pnpm-lock.yamlpackages/react-native/.eslintrc.cjs,packages/react-native/package.jsonpackages/react-native/srcchanges, treating most of them as lint cleanupVerification
pnpm installnode scripts/check-exact-deps.mjspnpm lintpnpm --filter @formbricks/react-native buildpnpm --filter @formbricks/react-native testpnpm --filter @formbricks/react-native test:coverageNotes
event-listeners.ts100%,setup.ts97.22% lines / 88.75% branches