Upgrade Expo to SDK 57 and React Native 0.86#300
Conversation
|
Warning Review limit reached
Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
📝 WalkthroughWalkthroughExpo SDK and React Native dependencies were upgraded, with Gradle and keyboard-controller patches registered and documented. A native iOS system-appearance module and theme wiring were added, alongside analytics guards and authentication-flow animation updates. ChangesExpo and React Native upgrade
System appearance and theme wiring
Analytics and onboarding behavior
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ThemeProvider
participant SystemColorHook
participant DayovaSystemAppearance
participant UIKitObserver
ThemeProvider->>SystemColorHook: request system color scheme
SystemColorHook->>DayovaSystemAppearance: getColorScheme and subscribe
DayovaSystemAppearance->>UIKitObserver: observe trait changes
UIKitObserver->>DayovaSystemAppearance: emit color scheme change
DayovaSystemAppearance->>SystemColorHook: deliver onChange event
SystemColorHook->>ThemeProvider: update resolved theme
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Native iOS validation completeValidated commit
Native build results
Application exerciseExercised the existing development account/data through cold launch and authenticated-session restoration, home data, learning plans, plan details and session content, repeated create-modal open/close, the full new-exam flow, profile text input with the software keyboard, settings, notifications and notification-time sheets, learning-time screens/editor, light/dark/system appearance, live OS appearance changes, background/foreground resume, safe areas, navigation/back gestures, sheets/modals, and keyboard avoidance. A development-only exam and learning plan were created to validate the full flow. No production data was accessed or mutated. Log findingsFinal Debug and Release log passes contained no native crash/exception, React Native or JavaScript error, Fabric/New Architecture failure, missing native module, or safe-area/layout warning. Background/foreground restoration retained the active plan-detail screen. A manually built local production Release reports an Expo Updates HTTP 400 because plain Fixes made during iOS validation
Final validation suite
The Expo SDK 57 changelog, SDK 56→57 upgrade helper, and React Native 0.86 release notes were reviewed. The PR is non-draft and is ready for CI/CodeRabbit revalidation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/lib/system-color-scheme.ios.ts (1)
5-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
useSyncExternalStoreoveruseState+useEffectfor external subscriptions.Since React 18,
useSyncExternalStoreis the recommended hook for subscribing to external state (like a native module). It prevents "tearing" (where the UI might render an old value if the system appearance changes before theuseEffectfires) and simplifies the code.♻️ Proposed refactor using `useSyncExternalStore`
-import { useEffect, useState } from "react"; +import { useSyncExternalStore } from "react"; import type { ResolvedTheme } from "~/lib/theme-preference"; import DayovaSystemAppearance from "../../modules/dayova-system-appearance"; +const subscribe = (callback: () => void) => { + const subscription = DayovaSystemAppearance.addListener("onChange", callback); + return () => subscription.remove(); +}; + +const getSnapshot = () => DayovaSystemAppearance.getColorScheme(); + export function useSystemColorScheme(): ResolvedTheme { - const [colorScheme, setColorScheme] = useState<ResolvedTheme>(() => - DayovaSystemAppearance.getColorScheme(), - ); - - useEffect(() => { - const subscription = DayovaSystemAppearance.addListener( - "onChange", - (event) => setColorScheme(event.colorScheme), - ); - - return () => subscription.remove(); - }, []); - - return colorScheme; + return useSyncExternalStore(subscribe, getSnapshot); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/system-color-scheme.ios.ts` around lines 5 - 20, Update useSystemColorScheme to use React’s useSyncExternalStore instead of useState and useEffect for the DayovaSystemAppearance subscription. Provide a subscribe callback that registers the existing onChange listener and returns its removal cleanup, and provide a snapshot callback that reads DayovaSystemAppearance.getColorScheme().src/lib/theme-css.test.ts (1)
20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a more robust regex for extracting the CSS block.
The current regex
\n\t\}depends strictly on the specific whitespace and indentation of the file. Ifglobal.cssis reformatted (e.g., using spaces instead of tabs, or different line endings), the test will fail.Using
[^}]+to match everything up to the first closing brace is more resilient to formatting changes.♻️ Proposed refactor
- const darkRoot = css.match(/\.dark:root\s*\{([\s\S]*?)\n\t\}/)?.[1]; + const darkRoot = css.match(/\.dark:root\s*\{([^}]+)\}/)?.[1];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/theme-css.test.ts` at line 20, Update the darkRoot extraction regex in the theme CSS test to terminate at the first closing brace using a formatting-independent pattern such as [^}]+, instead of requiring a specific newline and tab sequence. Preserve the existing .dark:root block matching and captured-content behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/analytics.ts`:
- Around line 17-21: Update the analytics authentication flow to obtain
isConvexAuthenticated from useConvexAuth() rather than destructuring it from
useAuth(). Add the required convex/react import, and keep the existing user &&
isConvexAuthenticated guard for the api.users.getMe query so analytics activity
can execute when Convex authentication is valid.
---
Nitpick comments:
In `@src/lib/system-color-scheme.ios.ts`:
- Around line 5-20: Update useSystemColorScheme to use React’s
useSyncExternalStore instead of useState and useEffect for the
DayovaSystemAppearance subscription. Provide a subscribe callback that registers
the existing onChange listener and returns its removal cleanup, and provide a
snapshot callback that reads DayovaSystemAppearance.getColorScheme().
In `@src/lib/theme-css.test.ts`:
- Line 20: Update the darkRoot extraction regex in the theme CSS test to
terminate at the first closing brace using a formatting-independent pattern such
as [^}]+, instead of requiring a specific newline and tab sequence. Preserve the
existing .dark:root block matching and captured-content behavior.
🪄 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: d71a9963-88ed-4cd6-8ee0-256fd1c22adb
📒 Files selected for processing (18)
app.config.tsdocs/bottom-sheets.mdmodules/dayova-system-appearance/LICENSEmodules/dayova-system-appearance/expo-module.config.jsonmodules/dayova-system-appearance/index.tsmodules/dayova-system-appearance/ios/DayovaSystemAppearance.podspecmodules/dayova-system-appearance/ios/DayovaSystemAppearanceModule.swiftmodules/dayova-system-appearance/src/DayovaSystemAppearance.types.tsmodules/dayova-system-appearance/src/DayovaSystemAppearanceModule.tssrc/app/_layout.tsxsrc/components/analytics-identity.tsxsrc/features/auth/dayova-auth-flow.tsxsrc/lib/analytics.tssrc/lib/system-color-scheme.ios.tssrc/lib/system-color-scheme.tssrc/lib/theme-css.test.tssrc/lib/theme-variables.tssrc/lib/theme.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app.config.ts
Native iOS validation complete (final pass)Validated commit
Native results
App exerciseExercised cold launch/session restoration, Home data, learning plans and plan details, learning session/quiz content, repeated create-modal open/close, new-exam flow, profile input and keyboard behavior, learning-time editor, notification settings and notification-time sheet, Light/Dark/System appearance, background/foreground resume, navigation gestures, sheets/modals, safe areas, and keyboard avoidance. Test-only records were created only in the development environment; production data was not mutated. Log reviewNo native crashes/exceptions, JavaScript errors in the final clean Debug run, missing native modules, Fabric/New Architecture failures, safe-area/layout warnings, or runtime-version incompatibilities were found. Xcode 26.6 emits non-fatal upstream dependency warnings (React Native/Expo/Reanimated/Screens/SVG/keyboard-controller). The manual Release launch reports the expected Expo Updates HTTP 400 for a missing A transient navigation-context error and localhost connection refusals in earlier diagnostic logs came from hot-loading a partially compiled Metro stylesheet and deliberately stopping/restarting Metro while diagnosing the appearance issue; neither recurred after the clean full bundle. Fixes added during iOS validation
Final suite
Remaining limitation: no physical iPhone was available, so device execution and signed archive installation were not performed. The unsigned arm64 device Release build passed. No TestFlight/App Store submission was made. |
|
@FleetAdmiralJakob I am to tired to review the PR yet, but Ill try to review tomorrow, because I want to read all changes without time pressure. Will add it to my todo with High Priority! |
|
Native iOS follow-up completed on The reported appearance-module floor is fixed: One important attribution correction: the generated Dayova application target is still iOS 17.0 because Validation environment:
Results:
Signed-run logs have no native crash, JS exception, Fabric/New Architecture failure, missing module, safe-area/layout failure, keychain error, or runtime-version mismatch. Remaining disclosed diagnostics are the existing future The PR body is updated with the current native module, 130-test count, deployment-target rationale, full validation results, theme-token maintenance note, and remaining limitations. |
|
Please change target branch to 1.0.4, because we cant send it OTA. Directly merge is not possible. Because this blocks the other OTA updates. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/contexts/mobile-app/CONTEXT.md`:
- Around line 17-21: Update the documentation links in the mobile-app context
document to use relative paths from docs/contexts/mobile-app/ that correctly
reach the modules directory and the mobile-app ADR file, preserving the existing
link targets and descriptions.
🪄 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: 8c052cb1-9c25-474b-90a4-ec9fd9e216da
📒 Files selected for processing (6)
CONTEXT-MAP.mddocs/contexts/mobile-app/CONTEXT.mddocs/contexts/mobile-app/adr/0001-use-local-ios-system-appearance-bridge.mddocs/contexts/platform/CONTEXT.mdmodules/dayova-system-appearance/README.mdsrc/lib/ios-appearance-module.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/contexts/platform/CONTEXT.md
- src/lib/ios-appearance-module.test.ts
2 participants
Summary
Upgrades Dayova from Expo SDK 56 to stable Expo SDK 57 and aligns the React Native, Expo, and native-support dependency set.
~56.0.12→~57.0.70.85.3→0.86.0^4.5.0, Worklets0.10.0, Gesture Handler^2.32.0, and Keyboard Controller1.21.9^3.7.8mainOTA-safety and portable Expo-script changes reconciled; unused-code analysis now suppliesAPP_VARIANTportablyThis PR also fixes pre-existing unused theme exports, includes two narrowly scoped Android compatibility patches, and carries the iOS appearance fixes found during native validation.
Changelog and compatibility review
Reviewed:
The generated iOS project and checked-in Android project build against the SDK 57 / React Native 0.86 native stack.
iOS deployment target and appearance compatibility
Expo SDK 57 supports iOS 16.4+. Dayova-owned native code preserves that baseline:
DayovaSystemAppearance.podspecdeclares iOS 16.4.registerForTraitChanges.traitCollectionDidChange.arm64-apple-ios16.4-simulator.The generated Dayova app target remains iOS 17.0 because the native Clerk Expo SDK/config plugin requires and writes iOS 17.0. That requirement already existed on the PR merge base; the Expo SDK 57 upgrade and appearance module did not introduce it. It is documented in
docs/contexts/platform/CONTEXT.mdfor later re-evaluation.Android compatibility fixes
React Native Gradle plugin on Windows
React Native 0.86's Gradle settings plugin applies the Foojay toolchain resolver. Gradle 9.3.1 can miscompile that settings script on Windows before plugin resolution, consistent with gradle/gradle#36323.
patches/@react-native__gradle-plugin@0.86.0.patchremoves only the unnecessary Foojay settings block. Dayova already requires JDK 17. Autolinking, codegen, and the application plugin are unchanged.Keyboard controller with Fabric modals
On React Native 0.86, opening Dayova's React Native modal on Android produced:
patches/react-native-keyboard-controller@1.21.9.patchcarries the Fabric surface ID from the modal'stopShowevent into the focused-input observer. The ordinary non-modal path retains the package's existing lookup. Both patches are documented inpatches/README.mdwith verification and removal criteria.Theme cleanup and maintenance
Knip-reported theme implementation details are now private, and the obsolete light-only
NAV_THEMEalias is removed whileNAV_THEMESremains.Dark-theme tokens are mirrored between CSS and
src/lib/theme-variables.tsbecause CSS and native consumers need different representations.src/lib/theme-css.test.tsdetects drift; color changes must update both.Validation
Final validated head:
8a52c3d(review fixes plus latest-main reconciliation after native validation).Repository and Expo checks
pnpm install --frozen-lockfilewith pnpm 10.25.0pnpm checkpnpm format:checkpnpm test— 32 files / 143 testspnpm check:unusednpx --yes expo-doctor@latest— 20 checkspnpm exec expo install --checkgit diff --checkNative iOS
Environment:
iPhone18,3) on iOS 26.5Results:
.xcresult.Sign to Run Locally) passed and launched.The complete iOS walkthrough covered:
The final iOS-floor patch changes only the appearance observer. On exact head
73834a8, Debug and Release were rebuilt, launched, and re-smoked for cold launch, live appearance/status-bar repaint, and resume. The retained simulator no longer had an authenticated Clerk session, so the broader authenticated walkthrough remains the immediately preceding native-validation run on687970c; no auth or feature-flow code changed afterward.No data was edited or submitted. No physical iPhone was connected.
Native Android
Validated on Windows/JDK 17/Gradle 9.3.1 and a Pixel 9 on Android 16:
Log findings and limitations
Signed Debug/Release logs contained no native crash, JavaScript exception, Fabric/New Architecture failure, missing native module, layout/safe-area failure, keychain entitlement failure, or runtime-version mismatch.
Observed diagnostics:
UIScenelifecycle warning on iOS 26.5.channel: productionineas.json; local update-resource generation passed with runtime1.0.3.-34018diagnostics. Simulator-signed builds eliminate them.Maintenance note
Re-evaluate both pnpm patches when React Native, Gradle, or Keyboard Controller changes. Re-evaluate the app-wide iOS 17.0 minimum when Clerk lowers its native requirement or is replaced. Keep mirrored CSS/native theme tokens synchronized; the drift test enforces this.
Summary by CodeRabbit