chore: update package-lock.json with peer dependency flags#1
Conversation
Reorganize peer dependency flags in package-lock.json after npm install.
📝 WalkthroughSummary by CodeRabbit
WalkthroughProject-wide updates: dependency and tooling bumps, new Babel config, expanded Expo plugins and build config, added Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 analysis the entire project and check if there are a missing expo react native files , that might work properly on web but not that on Andriod apk files , and check if there any problems might block the build status on eas build and the overall experience on Android as apk , also dig deeper on some problem i am facing when I built the app as .apk file and faced some problems like "[ 11:32 Tue, 14 Apr Unable to load script. Make sure you're running Metro or that your bundle 'index.android.bundle' is packaged correctly for release. The device must either be USB connected (with bundler set to "localhost:8081") or be on the same Wi-Fi network as your computer (with bundler set to your computer IP) to connect to Metro. If you're using USB on a physical device, make sure you also run this command: adb reverse tcp:8081 tcp:8081 jniLoadScript FromAssets CatalystInstanceImpl.java loadScriptFromAssets CatalystInstanceImpl.java:245 loadScript JSBundleLoader.kt:33 runJSBundle CatalystInstanceImpl.java:274 createReactContext ReactInstanceManager.java:1525 lambda$runCreateReactContextOnNewThread$2 ReactInstanceManager.java:1222 lambda$PrBhihCbbAFk4ZReAALGanVLCyc null run D8$$SyntheticClass run |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll do a deep analysis of your project structure to identify all potential issues. 🏁 Script executed: Length of output: 11348 🏁 Script executed: Length of output: 18194
🔴
|
| File | Line | Code | Guard |
|---|---|---|---|
AppContext.tsx |
95, 113 | localStorage.getItem/setItem |
✅ Line 93: if (Platform.OS === 'web') |
AppContext.tsx |
650–651 | localStorage.removeItem |
✅ Line 649: if (Platform.OS === 'web') |
LanguageContext.tsx |
779 | window.location.reload() |
✅ Line 776: if (Platform.OS === 'web' && !skipReload) |
OnboardingScreen.tsx |
268 | window.location.reload() |
✅ Line 266: if (Platform.OS === 'web' && ...) |
autoBackup.ts |
~79 | localStorage.setItem |
✅ Inside Platform.OS === 'web' block |
Summary Checklist
| # | Issue | Severity | Fix |
|---|---|---|---|
| 1 | assembleDebug in eas.json preview |
🔴 Critical | Remove gradleCommand, use buildType: "apk" only |
| 2 | Missing babel.config.js |
🔴 Critical | Create with babel-preset-expo preset |
| 3 | Missing babel-preset-expo in devDeps |
🔴 Critical | npm install --save-dev babel-preset-expo |
| 4 | expo-local-authentication not in plugins |
🟠 High | Add to app.json plugins |
| 5 | edgeToEdgeEnabled: true without package |
🟠 High | Install react-native-edge-to-edge |
| 6 | expo-file-system Next API |
🟠 Medium | Test on release APK, monitor for issues |
| 7 | Keystore committed to repo | 🟡 Security | Add to .gitignore, use eas credentials |
Start with fix #1 — it will immediately solve the "Unable to load script" error. Then apply fixes #2 and #3 to ensure a clean EAS build.
…nore, and autoBackup.ts async/await fixes
Ensure backup files are deleted with await to prevent race conditions and ensure deletions complete before proceeding. Also add babel.config.js to include Babel configuration for the project build/transpilation.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Line 20: The repo added *.keystore to .gitignore but the committed keystore
attery.keystore remains tracked and must be removed from tracking and treated as
compromised; run git rm --cached attery.keystore and commit (e.g., "chore: stop
tracking committed keystore") to untrack it, then rotate the signing key and
migrate to EAS-managed credentials to mitigate exposure.
In `@app.json`:
- Line 43: The plugin entry for "expo-local-authentication" should be changed
from a bare string to an object that supplies a custom iOS permission message
via the faceIDPermission key; update the app.json plugins array so the
"expo-local-authentication" entry is an object with {"faceIDPermission":"Allow
$(PRODUCT_NAME) to use Face ID to authenticate."} (or an app-specific message)
to set NSFaceIDUsageDescription explicitly and provide better context for users.
In `@eas.json`:
- Around line 7-9: The preview EAS profile is using a debug Gradle command that
produced an APK expecting Metro (causing "Unable to load script:
index.android.bundle"); remove any custom gradleCommand like
":app:assembleDebug" from the preview profile so EAS falls back to the default
release-style assemble (keep "android": { "buildType": "apk" }) and the JS will
be bundled into the APK at build time; additionally, explicitly add
"distribution": "internal" to the preview profile to document intent, and if you
need a debug/dev APK in future create a separate "development" profile with
"developmentClient": true instead of using assembleDebug in preview.
In `@package.json`:
- Around line 31-36: Two dependencies use caret ranges but should use Expo's
tilde pinning: change "react-native-edge-to-edge" from ^1.8.1 to ~1.8.1 and
"react-native-web" from ^0.21.0 to ~0.21.0 in package.json; update the version
strings for those package keys so they match the tilde convention used by other
Expo-validated packages and re-run npx expo install --check to verify
compatibility.
In `@src/utils/autoBackup.ts`:
- Around line 26-28: Remove the misleading awaits on synchronous object-methods:
drop the await when calling backupDir.create() (Directory.create), when calling
file.write(...) (File.write) and when calling file.delete() (File.delete) since
these are synchronous in expo-file-system 19.0.21; alternatively, if you truly
need async behavior, replace those calls with the legacy async APIs
(FileSystem.makeDirectoryAsync, FileSystem.writeAsStringAsync,
FileSystem.deleteAsync) and await those instead.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 46ddf338-d679-4668-a297-743112cfc993
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.gitignoreapp.jsonbabel.config.jseas.jsonpackage.jsonsrc/utils/autoBackup.ts
| *.p12 | ||
| *.key | ||
| *.mobileprovision | ||
| *.keystore |
There was a problem hiding this comment.
Adding *.keystore is good, but the already-tracked attery.keystore needs to be untracked.
Per the prior diagnostic comment in this PR, a keystore file (attery.keystore) was committed to the repository. Adding the pattern to .gitignore does not remove the file from Git history or stop tracking the existing copy — the committed keystore remains a credential exposure. After merging, run:
git rm --cached attery.keystore
git commit -m "chore: stop tracking committed keystore"Strongly recommended to also rotate the signing key and migrate to EAS-managed credentials, since the previously committed keystore must be considered compromised.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitignore at line 20, The repo added *.keystore to .gitignore but the
committed keystore attery.keystore remains tracked and must be removed from
tracking and treated as compromised; run git rm --cached attery.keystore and
commit (e.g., "chore: stop tracking committed keystore") to untrack it, then
rotate the signing key and migrate to EAS-managed credentials to mitigate
exposure.
| } | ||
| } | ||
| ], | ||
| "expo-local-authentication", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🌐 Web query:
In Expo SDK 54, what config options does the expo-local-authentication config plugin accept, and is faceIDPermission required to set NSFaceIDUsageDescription?
💡 Result:
In Expo SDK 54, the expo-local-authentication config plugin accepts one option: faceIDPermission, which is a string for iOS that sets the NSFaceIDUsageDescription permission message. Its default value is "Allow $(PRODUCT_NAME) to use Face ID.". The faceIDPermission option is not strictly required because it has a default value. However, Apple requires the NSFaceIDUsageDescription key in Info.plist for apps using Face ID; without it (or if customized to an empty string), the app may crash or fall back to passcode authentication on iOS devices with Face ID.
Citations:
- 1: https://docs.expo.dev/versions/v54.0.0/sdk/local-authentication
- 2: https://docs.expo.dev/versions/latest/sdk/local-authentication/
- 3: https://docs.expo.dev/versions/v55.0.0/sdk/local-authentication/
🏁 Script executed:
# Find and examine AppContext.tsx to verify LocalAuthentication.authenticateAsync usage
find . -name "AppContext.tsx" -type fRepository: oryno-dev/Attenary
Length of output: 91
🏁 Script executed:
# Look at app.json to see the actual plugin configuration
find . -name "app.json" -type fRepository: oryno-dev/Attenary
Length of output: 73
🏁 Script executed:
# Check AppContext.tsx around lines 340-350 for LocalAuthentication usage
cat -n ./src/context/AppContext.tsx | sed -n '330,360p'Repository: oryno-dev/Attenary
Length of output: 1069
🏁 Script executed:
# Check app.json for the plugin configuration around line 43
cat -n ./app.json | sed -n '35,55p'Repository: oryno-dev/Attenary
Length of output: 592
Consider passing custom faceIDPermission config to expo-local-authentication for a more contextual message.
The plugin is currently configured as a bare string. While this will automatically set NSFaceIDUsageDescription to its default value ("Allow $(PRODUCT_NAME) to use Face ID."), providing an explicit config with a message specific to your app's authentication context (e.g., "Allow $(PRODUCT_NAME) to use Face ID to authenticate.") is a recommended best practice and improves the user experience on iOS.
♻️ Suggested expansion
- "expo-local-authentication",
+ [
+ "expo-local-authentication",
+ {
+ "faceIDPermission": "Allow $(PRODUCT_NAME) to use Face ID to authenticate."
+ }
+ ],
"react-native-edge-to-edge",
"expo-secure-store"📝 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.
| "expo-local-authentication", | |
| [ | |
| "expo-local-authentication", | |
| { | |
| "faceIDPermission": "Allow $(PRODUCT_NAME) to use Face ID to authenticate." | |
| } | |
| ], | |
| "react-native-edge-to-edge", | |
| "expo-secure-store" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app.json` at line 43, The plugin entry for "expo-local-authentication" should
be changed from a bare string to an object that supplies a custom iOS permission
message via the faceIDPermission key; update the app.json plugins array so the
"expo-local-authentication" entry is an object with {"faceIDPermission":"Allow
$(PRODUCT_NAME) to use Face ID to authenticate."} (or an app-specific message)
to set NSFaceIDUsageDescription explicitly and provide better context for users.
| "android": { | ||
| "buildType": "apk", | ||
| "gradleCommand": ":app:assembleDebug" | ||
| "buildType": "apk" | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Correct fix — this resolves the "Unable to load script: index.android.bundle" error.
Removing gradleCommand: ":app:assembleDebug" lets EAS fall back to its default release-style assemble for the preview profile, which bundles the JS into the APK at build time. The previous debug command produced an APK that expected Metro at runtime, which is exactly what the stacktrace in CatalystInstanceImpl was reporting.
A couple of small follow-ups worth considering, not blocking:
- Add
"distribution": "internal"explicitly so the intent ofpreviewis documented andeas buildwon't surprise contributors who default to store distribution. - If you ever do want a debug APK in the future for native debugging, create a separate
"development"profile with"developmentClient": truerather than reaching forassembleDebugagain.
♻️ Optional clarification
"preview": {
+ "distribution": "internal",
"android": {
"buildType": "apk"
}
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eas.json` around lines 7 - 9, The preview EAS profile is using a debug Gradle
command that produced an APK expecting Metro (causing "Unable to load script:
index.android.bundle"); remove any custom gradleCommand like
":app:assembleDebug" from the preview profile so EAS falls back to the default
release-style assemble (keep "android": { "buildType": "apk" }) and the JS will
be bundled into the APK at build time; additionally, explicitly add
"distribution": "internal" to the preview profile to document intent, and if you
need a debug/dev APK in future create a separate "development" profile with
"developmentClient": true instead of using assembleDebug in preview.
| "react-native-edge-to-edge": "^1.8.1", | ||
| "react-native-gesture-handler": "~2.28.0", | ||
| "react-native-safe-area-context": "~5.6.0", | ||
| "react-native-screens": "~4.16.0", | ||
| "react-native-svg": "15.12.1", | ||
| "react-native-web": "^0.21.0", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use Expo's tilde pinning for react-native-edge-to-edge and react-native-web.
react-native-edge-to-edge (^1.8.1) and react-native-web (^0.21.0) currently use caret ranges, while every other Expo-validated package in this manifest uses tilde (~). Expo SDK 54 only validates a specific set of versions for native-bridged packages; allowing minor-version drift on these can cause incompatibilities surfacing only on the Android APK build (e.g., new-arch interop in react-native-edge-to-edge). Aligning with Expo's pinning convention also keeps npx expo install --check quiet.
♻️ Proposed change
- "react-native-edge-to-edge": "^1.8.1",
+ "react-native-edge-to-edge": "~1.8.1",
@@
- "react-native-web": "^0.21.0",
+ "react-native-web": "~0.21.0",📝 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.
| "react-native-edge-to-edge": "^1.8.1", | |
| "react-native-gesture-handler": "~2.28.0", | |
| "react-native-safe-area-context": "~5.6.0", | |
| "react-native-screens": "~4.16.0", | |
| "react-native-svg": "15.12.1", | |
| "react-native-web": "^0.21.0", | |
| "react-native-edge-to-edge": "~1.8.1", | |
| "react-native-gesture-handler": "~2.28.0", | |
| "react-native-safe-area-context": "~5.6.0", | |
| "react-native-screens": "~4.16.0", | |
| "react-native-svg": "15.12.1", | |
| "react-native-web": "~0.21.0", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 31 - 36, Two dependencies use caret ranges but
should use Expo's tilde pinning: change "react-native-edge-to-edge" from ^1.8.1
to ~1.8.1 and "react-native-web" from ^0.21.0 to ~0.21.0 in package.json; update
the version strings for those package keys so they match the tilde convention
used by other Expo-validated packages and re-run npx expo install --check to
verify compatibility.
| if (!backupDir.exists) { | ||
| await backupDir.create(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the installed expo-file-system 19.0.21 type definitions to determine
# whether create / write / delete on Directory and File return Promises.
fd -t f -e d.ts . node_modules/expo-file-system 2>/dev/null | head -50
fd -t f 'next' node_modules/expo-file-system 2>/dev/null
rg -nP -C2 '\b(create|write|delete)\s*\(' node_modules/expo-file-system 2>/dev/null --type=ts | head -80Repository: oryno-dev/Attenary
Length of output: 44
🌐 Web query:
In expo-file-system 19.0.21 (Expo SDK 54 stable next API), do Directory.create(), File.write(), and File.delete() return Promises, or are they synchronous? What are the recommended async variants?
💡 Result:
In expo-file-system 19.0.21 (the stable next API in Expo SDK 54), Directory.create(options?), File.write(content), and File.delete are all synchronous methods that return void. They perform operations immediately and throw errors synchronously if they fail (e.g., if the path doesn't exist, no permissions, or already exists without idempotent/overwrite options). Examples from docs and types: - Directory.create(options?: DirectoryCreateOptions): void - File.write(content: string | Uint8Array): void - Directory/File.delete: void They have no Promise return; they are sync by design for the new object-oriented API. Recommended async variants (where available): - For copy/move: Use the async copy and move methods (return Promise), or legacy async functions like FileSystem.makeDirectoryAsync, FileSystem.writeAsStringAsync, FileSystem.deleteAsync. - For read operations: Use async methods like file.text, file.bytes (return Promise), with Sync counterparts like textSync, bytesSync. - For creation/writing/deletion specifically, no direct async versions in the new API; use try-catch for error handling, or fall back to legacy async API from expo-file-system/legacy (e.g., makeDirectoryAsync, writeAsStringAsync, deleteAsync) if async behavior is needed. The new API prioritizes sync for simplicity/performance, with some methods recently made async (e.g., copy/move in recent updates).
Citations:
- 1: https://docs.expo.dev/versions/latest/sdk/filesystem/
- 2: https://cdn.jsdelivr.net/npm/expo-file-system@19.0.19/build/ExpoFileSystem.types.d.ts
- 3: https://tessl.io/registry/tessl/npm-expo-file-system/19.0.0/files/docs/directory-operations.md
- 4: https://docs.expo.dev/versions/v54.0.0/sdk/filesystem
- 5: https://github.com/expo/expo/blob/main/packages/expo-file-system/CHANGELOG.md
- 6: https://docs.expo.dev/versions/unversioned/sdk/filesystem/
- 7: https://expo.dev/blog/expo-file-system
- 8: https://www.ezfolders.com
- 9: https://github.com/expo/expo/blob/68661df5/packages/expo-file-system/build/ExpoFileSystem.types.d.ts
Remove unnecessary await statements for synchronous methods.
Directory.create() (line 27), File.write() (line 104), and File.delete() (line 146) are synchronous methods that return void in expo-file-system 19.0.21. The added await statements are harmless—they resolve immediately without waiting—but they misrepresent the API design and create confusion.
Either remove the await keywords or, if actual async behavior is needed, switch to the legacy async API (FileSystem.makeDirectoryAsync, FileSystem.writeAsStringAsync, FileSystem.deleteAsync). The new object-oriented API does not provide async variants for create/write/delete operations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/autoBackup.ts` around lines 26 - 28, Remove the misleading awaits
on synchronous object-methods: drop the await when calling backupDir.create()
(Directory.create), when calling file.write(...) (File.write) and when calling
file.delete() (File.delete) since these are synchronous in expo-file-system
19.0.21; alternatively, if you truly need async behavior, replace those calls
with the legacy async APIs (FileSystem.makeDirectoryAsync,
FileSystem.writeAsStringAsync, FileSystem.deleteAsync) and await those instead.
|
@coderabbitai , App Crashed View config getter callback for component 'path' must be a function (received "undefined"). Make sure to start component names with a capital letter. Envar Laint Violation View.config getter callback for component Must be function Treceived undetinet Make sure to start component nases with a capital letter at Invariant laddress at indes android bundle:1:154374) at get faddress at index androad bundle 1:2329117 of completelork (address at index android bundle 5123001 at completelmatofwork (addruss at andex android bundle 1:530524) at performünitofwork (address at Index android bundle: 529800) at workloopSync (address at toder android bundle: 1 528787) At render Root Sync (address at indes, android bundle 1.5280141 at por formerünkoot (address at index anarojo bundle: 1 526040) at purforaSyn KOH (address at iles.androll bundle 1:478386) anushSyntMor katrossRoots imp (ddress at indiex android.bohole:|:477293) at batchediupdatestani (södress at index android.bundle: 1-543000) at hatchedupdates81 (address at indita android bundle:114689101 at receiveRoptNodeIDEvent (address at Index android tongle: 1 469198) at recaleetouches (address at index android bundle 539667 at apply (native) at callFunction address at andex, android, bundle 1:1638851 at anonymous (address-at index andraxd bundle t:162328) guard (address at index android bundle 1 1632091 Error An error occurred. The app will attempt to restart. OK Attenary issues2 second issues when I click check in the app fully crash , reboot on onboarding found where why that happening, and investigate on any another issues in the entire project |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 14786 🏁 Script executed: Length of output: 33354
🔴 Issue
|
| Issue | Root Cause | Fix |
|---|---|---|
| App crash on Check In/Out | <svg>/<path> HTML tags in CheckInModal.tsx + CheckOutModal.tsx |
Replace with <Svg>/<Path> from react-native-svg |
| Onboarding every launch | Navigation mounts before AsyncStorage load completes | Add if (loading) return <splash> in Navigation.tsx |
Both fixes are small and surgical — no architectural changes needed. Start with the loading guard fix first (it's one line of logic), then fix the two modal SVG icon components.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/navigation/Navigation.tsx (1)
76-92:⚠️ Potential issue | 🔴 CriticalCritical: Rules of Hooks violation — early return before
useState/useEffectwill crash the app.The early
returnon lines 78–84 executes before theuseStatecalls on lines 85–87 and theuseEffects. On the first renderloadingistrue, so React only callsuseApp(). OnceloadData()finishes andloadingflips tofalse, the next render callsuseApp()+ 3×useState+ 2×useEffect, violating the Rules of Hooks. React will throw "Rendered more hooks than during the previous render" and crash on virtually every cold start.Move the loading check after all hook declarations:
🐛 Proposed fix
const Navigation = () => { const { appData, loading } = useApp(); - // Wait for data to load before rendering navigation - if (loading) { - return ( - <View style={{ flex: 1, backgroundColor: '#0f172a', justifyContent: 'center', alignItems: 'center' }}> - <ActivityIndicator size="large" color="#6366f1" /> - </View> - ); - } const [showOnboarding, setShowOnboarding] = useState(!appData.onboardingCompleted); const [updateInfo, setUpdateInfo] = useState<UpdateInfo | null>(null); const [showUpdateModal, setShowUpdateModal] = useState(false); useEffect(() => { setShowOnboarding(!appData.onboardingCompleted); }, [appData.onboardingCompleted]); useEffect(() => { if (!showOnboarding && Platform.OS !== 'web') { checkForUpdate() .then(update => { if (update) { setUpdateInfo(update); setShowUpdateModal(true); } }) .catch(error => { console.log('Update check failed (non-critical):', error?.message || error); }); } }, [showOnboarding]); const handleDismissUpdate = () => { setShowUpdateModal(false); }; + // Wait for data to load before rendering navigation + if (loading) { + return ( + <View style={{ flex: 1, backgroundColor: '#0f172a', justifyContent: 'center', alignItems: 'center' }}> + <ActivityIndicator size="large" color="#6366f1" /> + </View> + ); + } + return ( <NavigationContainer>Alternatively, gate the mount in
App.jsbefore<Navigation />renders (cleaner): conditionally render<Navigation />only afterloading === falsein theProvidercomponent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/navigation/Navigation.tsx` around lines 76 - 92, The loading early-return is placed before hook declarations, violating the Rules of Hooks; move the loading check so that all hooks (the useState calls for showOnboarding, updateInfo, showUpdateModal and the useEffect that calls setShowOnboarding based on appData.onboardingCompleted) are declared first, then return the ActivityIndicator when loading is true, or alternatively stop rendering <Navigation /> higher up and only mount Navigation when useApp().loading is false; ensure useApp() remains the first hook call and the useEffect for setShowOnboarding keeps its dependency on appData.onboardingCompleted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/navigation/Navigation.tsx`:
- Around line 76-92: The loading early-return is placed before hook
declarations, violating the Rules of Hooks; move the loading check so that all
hooks (the useState calls for showOnboarding, updateInfo, showUpdateModal and
the useEffect that calls setShowOnboarding based on appData.onboardingCompleted)
are declared first, then return the ActivityIndicator when loading is true, or
alternatively stop rendering <Navigation /> higher up and only mount Navigation
when useApp().loading is false; ensure useApp() remains the first hook call and
the useEffect for setShowOnboarding keeps its dependency on
appData.onboardingCompleted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e7dc8f19-0c66-4619-8649-41a812070d37
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
src/components/CheckInModal.tsxsrc/components/CheckOutModal.tsxsrc/navigation/Navigation.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/navigation/Navigation.tsx (1)
76-83:⚠️ Potential issue | 🔴 CriticalLoading gate doesn't fully fix the onboarding-on-every-launch bug —
showOnboardinginitial state is still stale.
showOnboardingis initialized byuseState(!appData.onboardingCompleted)using the defaultappData(before storage load). WhenloadData()finishes inAppContextit batchessetAppData(...)andsetLoading(false), so on the first render whereloading === false:
appData.onboardingCompletedalready reflects the loaded value (e.g.true),- but
showOnboardingis still the stale initialtrue— the syncuseEffectat lines 81–83 only schedulessetShowOnboarding(false)after this render,- so
NavigationContainermounts withinitialRouteName='Onboarding'.initialRouteNameis only honored on the navigator's first mount, so the subsequent state update doesn't redirect the user.The
navigation.replace('Main')inOnboardingScreen(context snippet 3) only masks this with a brief flash of the onboarding screen — the underlying issue (and the symptom you reported) isn't actually resolved by the loading gate alone.Simplest fix: derive
showOnboardingdirectly from context instead of mirroring it into local state. By the time we reach the navigator (post loading-gate),appDatais the loaded value.🛠️ Proposed fix
const Navigation = () => { const { appData, loading } = useApp(); - const [showOnboarding, setShowOnboarding] = useState(!appData.onboardingCompleted); const [updateInfo, setUpdateInfo] = useState<UpdateInfo | null>(null); const [showUpdateModal, setShowUpdateModal] = useState(false); - useEffect(() => { - setShowOnboarding(!appData.onboardingCompleted); - }, [appData.onboardingCompleted]); - useEffect(() => { - if (!showOnboarding && Platform.OS !== 'web') { + if (!loading && appData.onboardingCompleted && Platform.OS !== 'web') { checkForUpdate() .then(update => { if (update) { setUpdateInfo(update); setShowUpdateModal(true); } }) .catch(error => { console.log('Update check failed (non-critical):', error?.message || error); }); } - }, [showOnboarding]); + }, [loading, appData.onboardingCompleted]); // Wait for data to load before rendering navigation if (loading) { return ( <View style={{ flex: 1, backgroundColor: '#0f172a', justifyContent: 'center', alignItems: 'center' }}> <ActivityIndicator size="large" color="#6366f1" /> </View> ); } + const showOnboarding = !appData.onboardingCompleted; + const handleDismissUpdate = () => {This guarantees
initialRouteNameis computed from the freshly-loadedappDataon the navigator's first mount, removing both the stale-state race and the dependency onOnboardingScreen's fallback redirect. As a bonus it also avoids firing the update check before storage load completes.Also applies to: 100-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/navigation/Navigation.tsx` around lines 76 - 83, The local state showOnboarding (initialized via useState(!appData.onboardingCompleted) and synced in the useEffect) can be stale and causes the navigator to mount with the wrong initialRouteName; instead remove the local showOnboarding state and derive the value directly from context (use appData.onboardingCompleted and loading to compute whether to show onboarding) so initialRouteName for NavigationContainer is computed from the loaded appData on first mount; update any references (including the update-check gating) to use the derived value and remove the now-unnecessary setShowOnboarding and its useEffect to avoid the race that causes the onboarding flash and reliance on OnboardingScreen navigation.replace.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/navigation/Navigation.tsx`:
- Around line 76-83: The local state showOnboarding (initialized via
useState(!appData.onboardingCompleted) and synced in the useEffect) can be stale
and causes the navigator to mount with the wrong initialRouteName; instead
remove the local showOnboarding state and derive the value directly from context
(use appData.onboardingCompleted and loading to compute whether to show
onboarding) so initialRouteName for NavigationContainer is computed from the
loaded appData on first mount; update any references (including the update-check
gating) to use the derived value and remove the now-unnecessary
setShowOnboarding and its useEffect to avoid the race that causes the onboarding
flash and reliance on OnboardingScreen navigation.replace.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 025983c3-8216-42c7-9ca7-5194a64f4f6a
📒 Files selected for processing (1)
src/navigation/Navigation.tsx
Summary
Changes
package-lock.jsonwith updated peer dependency flags for various @babel packages