-
Notifications
You must be signed in to change notification settings - Fork 10
refactor: code restructuring and cleanup #453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds EXPO_UPDATE_URL to development, staging, and production env files; removes explicit Doppler project/config flags from several package.json scripts; relocates navigation tracking hooks from feature to shared exports; narrows the authContext barrel exports while directly exposing AuthContextProvider and useAuthContext; changes AuthContextProvider to defer startTrackingUser via a useRef/useEffect (no longer awaited inside signIn); tightens feature-flag typing to include Sequence Diagram(s)sequenceDiagram
%% Subtle accessible coloring via rect notes
participant User
participant AuthProvider as AuthContextProvider
participant AuthService
participant Tracker as startTrackingUser
participant Analytics
rect rgba(200,200,255,0.06)
Note over User,AuthProvider: Sign-in (signIn no longer awaits tracking)
end
User->>AuthProvider: signIn(credentials)
AuthProvider->>AuthService: authenticate(credentials)
AuthService-->>AuthProvider: userData
AuthProvider->>AuthProvider: setIsUserLoggedIn(true) / store user
Note over AuthProvider: does NOT await Tracker here
AuthProvider-->>User: resolve signIn
rect rgba(200,255,200,0.06)
Note over AuthProvider,Tracker: useEffect triggers once when user appears
end
AuthProvider->>Tracker: startTrackingUser(user) [via useEffect]
Tracker->>Analytics: identify / send events
Analytics-->>Tracker: ack
Tracker-->>AuthProvider: trackingComplete
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used📓 Path-based instructions (2)src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.{tsx,ts}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🔇 Additional comments (1)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @package.json:
- Line 29: Package.json scripts like "start:dev" (and the related scripts noted
around lines 33-35, 42, 55) now rely on Doppler defaults but the repository
lacks a doppler.yaml; add a doppler.yaml in the repo root containing the
explicit project/config mapping used previously (e.g., the rn-starter project
and dev config) so the doppler run calls work without local interactive setup,
or alternatively update the README to document the required developer steps (run
`doppler setup` or provide DOPPLER_TOKEN env var) and include the exact commands
and environment variables needed to run the package.json scripts locally.
In @src/domain/contexts/authContext/AuthContextProvider.tsx:
- Around line 60-66: The effect and signIn both call startTrackingUser causing
duplicate tracking; remove the direct await startTrackingUser(userDataPayload)
call from signIn (keep setUser and its existing error handling) and centralize
tracking in the useEffect: inside the useEffect that depends on user and
startTrackingUser, set hasTrackedUserRef.current = true before invoking
startTrackingUser, and invoke startTrackingUser within an async try/catch (await
the call and log/handle errors) instead of using void so tracking errors are
handled consistently.
In @src/infra/featureFlags/featureFlags.types.ts:
- Around line 12-15: Test mocks in AppUpdateNeeded.test.tsx are missing the new
discriminator field from VersionFlagType; update the mock objects returned in
that test to include type: 'version' alongside the existing version property so
they conform to the VersionFlagType interface (mirror how defaultFlags.ts
populates VersionFlagType). Locate the mocks/fixture objects or mock return
values used in
src/shared/components/appUpdateNeeded/__tests__/AppUpdateNeeded.test.tsx and add
type: 'version' to each returned flag object.
In @src/shared/uiKit/bottomSheet/BottomSheet.tsx:
- Around line 72-80: Remove the inline style on the Box and instead create a
dynamic style using the Unistyles StyleSheet API that incorporates bottomInset;
e.g., define a new style (e.g., bottomInsetStyle or bottomBoxStyle) via the
Unistyles StyleSheet creation utility that takes bottomInset as the dynamic
value, then pass that style to the Box (e.g., style={[styles.bottomBox,
bottomInsetStyle]}) so BottomSheetView/Box use the StyleSheet-managed style
rather than style={{ marginBottom: bottomInset }}.
🧹 Nitpick comments (2)
src/shared/components/maintenanceMode/MaintenanceMode.tsx (1)
37-40: Remove redundant width and height properties.
StyleSheet.absoluteFillObjectalready positions the container to fill its parent completely (position: 'absolute'withleft: 0, right: 0, top: 0, bottom: 0), making the explicitwidth: '100%'andheight: '100%'redundant.♻️ Simplify the container style
container: { ...StyleSheet.absoluteFillObject, backgroundColor: theme.colors.bg_base, - width: '100%', - height: '100%', },src/shared/components/appUpdateNeeded/AppUpdateNeeded.tsx (1)
84-87: Remove redundant width and height properties.
StyleSheet.absoluteFillObjectalready positions the container to fill its parent completely (position: 'absolute'withleft: 0, right: 0, top: 0, bottom: 0), making the explicitwidth: '100%'andheight: '100%'redundant.♻️ Simplify the container style
container: { ...StyleSheet.absoluteFillObject, backgroundColor: theme.colors.bg_base, - width: '100%', - height: '100%', },
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (28)
.env.development.env.production.env.stagingpackage.jsonsrc/app/(protected)/(tabs)/features/(blogPost)/[blogPostId].tsxsrc/app/_layout.tsxsrc/domain/contexts/authContext/AuthContextProvider.tsxsrc/domain/contexts/authContext/components/index.tssrc/domain/contexts/authContext/hooks/index.tssrc/domain/contexts/authContext/index.tssrc/domain/contexts/authContext/useAuthContext.tssrc/domain/navigation/index.tssrc/domain/navigation/navigation.types.tssrc/features/navigation/hooks/index.tssrc/features/navigation/index.tssrc/features/navigation/utils/index.tssrc/infra/featureFlags/defaultFlags.tssrc/infra/featureFlags/featureFlags.types.tssrc/shared/components/appUpdateNeeded/AppUpdateNeeded.tsxsrc/shared/components/maintenanceMode/MaintenanceMode.tsxsrc/shared/hooks/index.tssrc/shared/hooks/useAppScreenTracking.tssrc/shared/hooks/useAppStateTracking.tssrc/shared/hooks/useIsNewStoreVersionAvailable.tssrc/shared/hooks/useRunOnMount.tssrc/shared/uiKit/bottomSheet/BottomSheet.tsxsrc/shared/utils/convertStringToKebabCase.tssrc/shared/utils/index.ts
💤 Files with no reviewable changes (5)
- src/domain/contexts/authContext/hooks/index.ts
- src/domain/contexts/authContext/components/index.ts
- src/features/navigation/index.ts
- src/features/navigation/utils/index.ts
- src/features/navigation/hooks/index.ts
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Always use TypeScript for code files
Use ES modules withimportstatements instead ofrequire
Always wrap arrow function arguments in parentheses
Prefer async/await for promises and always handle error cases
Always fix ESLint warnings and errors before finishing a task
Runyarn lint,yarn lint:ts, andyarn prettifyand fix potential errors before finishing a task
Files:
src/domain/navigation/index.tssrc/domain/contexts/authContext/useAuthContext.tssrc/domain/contexts/authContext/index.tssrc/infra/featureFlags/featureFlags.types.tssrc/shared/hooks/index.tssrc/domain/contexts/authContext/AuthContextProvider.tsxsrc/shared/components/appUpdateNeeded/AppUpdateNeeded.tsxsrc/app/_layout.tsxsrc/shared/components/maintenanceMode/MaintenanceMode.tsxsrc/infra/featureFlags/defaultFlags.tssrc/shared/hooks/useIsNewStoreVersionAvailable.tssrc/shared/hooks/useAppScreenTracking.tssrc/shared/uiKit/bottomSheet/BottomSheet.tsxsrc/app/(protected)/(tabs)/features/(blogPost)/[blogPostId].tsxsrc/shared/hooks/useRunOnMount.tssrc/shared/utils/index.ts
src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{tsx,ts}: Always use arrow functions with extracted Props interfaces for React components
Don't useReact.FCtype for components; prefer interfaces to types for Props
UseComponentProps<typeof MyComponent>to extract Props type instead of manually defining them
Never define inline styles; always use Unistyles's StyleSheet API for styling
Maintain theme consistency using Unistyles theme file located atsrc/domain/theme/unistyles.ts
Files:
src/domain/navigation/index.tssrc/domain/contexts/authContext/useAuthContext.tssrc/domain/contexts/authContext/index.tssrc/infra/featureFlags/featureFlags.types.tssrc/shared/hooks/index.tssrc/domain/contexts/authContext/AuthContextProvider.tsxsrc/shared/components/appUpdateNeeded/AppUpdateNeeded.tsxsrc/app/_layout.tsxsrc/shared/components/maintenanceMode/MaintenanceMode.tsxsrc/infra/featureFlags/defaultFlags.tssrc/shared/hooks/useIsNewStoreVersionAvailable.tssrc/shared/hooks/useAppScreenTracking.tssrc/shared/uiKit/bottomSheet/BottomSheet.tsxsrc/app/(protected)/(tabs)/features/(blogPost)/[blogPostId].tsxsrc/shared/hooks/useRunOnMount.tssrc/shared/utils/index.ts
src/**/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Create an index.ts file for components, hooks, or utils used outside the current folder using
export * from Xsyntax
Files:
src/domain/navigation/index.tssrc/domain/contexts/authContext/index.tssrc/shared/hooks/index.tssrc/shared/utils/index.ts
🧠 Learnings (5)
📚 Learning: 2026-01-07T07:41:02.477Z
Learnt from: CR
Repo: tsyirvo/react-native-starter PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T07:41:02.477Z
Learning: Applies to src/**/index.ts : Create an index.ts file for components, hooks, or utils used outside the current folder using `export * from X` syntax
Applied to files:
src/domain/navigation/index.tssrc/domain/contexts/authContext/index.tssrc/shared/hooks/index.tssrc/shared/utils/index.ts
📚 Learning: 2026-01-07T07:41:02.477Z
Learnt from: CR
Repo: tsyirvo/react-native-starter PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T07:41:02.477Z
Learning: Applies to src/**/*.{ts,tsx} : Use ES modules with `import` statements instead of `require`
Applied to files:
src/domain/contexts/authContext/useAuthContext.tssrc/domain/contexts/authContext/index.tssrc/shared/hooks/index.tssrc/app/_layout.tsxsrc/shared/utils/index.ts
📚 Learning: 2026-01-07T07:41:02.477Z
Learnt from: CR
Repo: tsyirvo/react-native-starter PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T07:41:02.477Z
Learning: Applies to src/**/*.{tsx,ts} : Maintain theme consistency using Unistyles theme file located at `src/domain/theme/unistyles.ts`
Applied to files:
src/domain/contexts/authContext/useAuthContext.ts
📚 Learning: 2026-01-07T07:41:02.477Z
Learnt from: CR
Repo: tsyirvo/react-native-starter PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T07:41:02.477Z
Learning: Applies to src/**/*.{tsx,ts} : Never define inline styles; always use Unistyles's StyleSheet API for styling
Applied to files:
src/shared/components/appUpdateNeeded/AppUpdateNeeded.tsxsrc/shared/components/maintenanceMode/MaintenanceMode.tsx
📚 Learning: 2026-01-07T07:41:02.477Z
Learnt from: CR
Repo: tsyirvo/react-native-starter PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T07:41:02.477Z
Learning: Applies to src/**/*.{ts,tsx} : Run `yarn lint`, `yarn lint:ts`, and `yarn prettify` and fix potential errors before finishing a task
Applied to files:
package.json
🧬 Code graph analysis (1)
src/shared/uiKit/bottomSheet/BottomSheet.tsx (1)
src/shared/uiKit/primitives/Box.tsx (1)
Box(31-45)
🪛 dotenv-linter (4.0.0)
.env.production
[warning] 4-4: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
.env.staging
[warning] 4-4: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
.env.development
[warning] 4-4: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🔇 Additional comments (21)
src/shared/hooks/useAppScreenTracking.ts (1)
5-5: LGTM! Clean import path refactoring.The migration to the aliased
$shared/utilsimport path improves code organization and maintainability.src/app/_layout.tsx (1)
33-34: LGTM! Appropriate hook relocation.Moving these tracking hooks from
$features/navigationto$shared/hooksis the correct architectural choice since they're used at the root layout level and provide app-wide functionality.src/shared/utils/index.ts (1)
2-2: LGTM! Proper barrel export.The export follows the project's coding guidelines for index.ts files and enables the shared utility to be imported via the
$shared/utilsalias.src/shared/hooks/index.ts (1)
1-1: LGTM! Proper barrel exports for relocated hooks.The new exports correctly expose
useAppScreenTrackinganduseAppStateTrackingvia the shared hooks index, following the project's coding guidelines and enabling the architectural refactoring.Also applies to: 3-3
src/domain/navigation/index.ts (1)
1-1: LGTM! Follows barrel export pattern correctly.The export statement correctly follows the coding guideline for index files, centralizing navigation type exports at the domain level.
src/app/(protected)/(tabs)/features/(blogPost)/[blogPostId].tsx (1)
4-4: Verify that the type is exported from the new location and all usages are updated consistently.The import path change from
$features/navigationto$domain/navigationaligns with the architectural refactoring. Please confirm that:
BlogPostScreenParamsis exported from$domain/navigation- All files importing
BlogPostScreenParamsor related navigation types have been updated to use the new domain-level import path- No remaining imports from the old
$features/navigationpath exist in the codebase.env.production (1)
4-4: Linter warning is a false positive for empty environment variable values.The dotenv-linter warning about quote characters can be safely ignored. Using
""for empty environment variable values is the standard convention to distinguish an explicitly empty string from an undefined/unset variable..env.development (1)
4-4: Linter warning is a false positive for empty environment variable values.The dotenv-linter warning about quote characters can be safely ignored. Using
""for empty environment variable values is the standard convention to distinguish an explicitly empty string from an undefined/unset variable..env.staging (1)
4-4: Linter warning is a false positive for empty environment variable values.The dotenv-linter warning about quote characters can be safely ignored. Using
""for empty environment variable values is the standard convention to distinguish an explicitly empty string from an undefined/unset variable.src/shared/components/appUpdateNeeded/AppUpdateNeeded.tsx (1)
28-30: LGTM! Good defensive programming.The payload validation correctly guards against both falsy payloads and payloads missing the
versionproperty, preventing potential runtime errors downstream.src/infra/featureFlags/defaultFlags.ts (1)
18-19: LGTM! Properly implements the updatedVersionFlagTypeinterface.The default configuration objects now correctly include the
type: 'version'discriminator, maintaining consistency with the updatedVersionFlagTypeinterface. The implementation is type-safe and follows the discriminated union pattern.src/shared/hooks/useRunOnMount.ts (3)
1-1: LGTM: Clean import syntax.The combined type and value import follows TypeScript best practices.
4-6: Verify React compiler compatibility for future-proofing.Disabling
react-compiler/react-compilersuggests this pattern may not be compatible with React 19's new compiler. While theexhaustive-depsdisable is expected for mount-only effects, the compiler disable could indicate deeper architectural concerns.Confirm whether this is a temporary workaround or if the hook pattern needs refactoring for React 19 compatibility.
3-3: The type change toEffectCallbackis correct—cleanup support is intentional and actively being used.The hook is already being used with cleanup functions (e.g.,
useAppFocusManager.tsreturns a subscription cleanup), confirming that theEffectCallbacktype is appropriate. The hook nameuseRunOnMountaccurately describes its behavior: it runs the callback on mount with an empty dependency array, and supports optional cleanup on unmount, which aligns with standarduseEffectsemantics. No changes needed.src/shared/hooks/useIsNewStoreVersionAvailable.ts (1)
22-24: The runtime guard is necessary due to the unsafe cast ingetFlagPayloadSync.The
!('version' in payload)check is good defensive programming. WhileVersionFlagTypeis a simple interface (not a discriminated union as originally assumed), the guard is still essential becausegetFlagPayloadSyncperforms an unsafe type cast (as T | undefinedon line 14 ofuseGetRemoteConfigSync.ts). This means the payload could be any shape at runtime. The guard ensures the expectedversionproperty exists before accessing it.src/domain/contexts/authContext/useAuthContext.ts (1)
3-3: LGTM! Import path correctly updated.The import path change from parent directory to sibling file reference aligns with the module reorganization.
src/domain/contexts/authContext/AuthContextProvider.tsx (1)
18-18: LGTM! Import path correctly updated.The import path change from parent directory to sibling file reference aligns with the module reorganization.
src/domain/contexts/authContext/index.ts (1)
2-3: LGTM! Export structure simplified correctly.The refactoring from subdirectory barrel exports to direct file exports simplifies the public API surface and correctly follows the coding guideline to use
export * from Xsyntax.src/shared/uiKit/bottomSheet/BottomSheet.tsx (3)
5-5: LGTM!The import of
BottomSheetViewis appropriate for handling the non-scrollable case.
63-70: LGTM!The scrollable branch implementation is clean and follows the coding guidelines.
104-106: Verify asymmetric bottom spacing between scrollable and non-scrollable branches.The
bottomSheetWrapperstyle applies a fixedmarginBottom: theme.spacing.spacing_24to both branches. However, the non-scrollable branch (lines 72-80) additionally appliesbottomInsetto the innerBox, while the scrollable branch (lines 63-70) does not.Please verify this asymmetry is intentional—perhaps
BottomSheetScrollViewhandles safe area insets automatically whileBottomSheetViewdoes not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shared/components/appUpdateNeeded/__tests__/AppUpdateNeeded.test.tsx (1)
61-61: Fix typo in test description.The test name contains a typo: "udpdate" should be "update".
✏️ Proposed fix
- it('should open the stores to allow users to udpdate', async () => { + it('should open the stores to allow users to update', async () => {
🤖 Fix all issues with AI agents
In @src/shared/components/appUpdateNeeded/__tests__/AppUpdateNeeded.test.tsx:
- Around line 23-26: Update the mocked return type for
FeatureFlags.useGetRemoteConfigSync so getFlagPayloadSync's generic matches the
discriminated union shape (include the literal type field), e.g., change the
generic from T = { version: string } to T = { type: 'version'; version: string }
and apply the same change to the other three test cases that mock
getFlagPayloadSync (the other occurrences in this file). Ensure the mocked
object still returns ({ type: 'version', version: '2.0.0' }) as T.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/domain/contexts/authContext/AuthContextProvider.tsxsrc/shared/components/appUpdateNeeded/__tests__/AppUpdateNeeded.test.tsxsrc/shared/uiKit/bottomSheet/BottomSheet.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/domain/contexts/authContext/AuthContextProvider.tsx
- src/shared/uiKit/bottomSheet/BottomSheet.tsx
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Always use TypeScript for code files
Use ES modules withimportstatements instead ofrequire
Always wrap arrow function arguments in parentheses
Prefer async/await for promises and always handle error cases
Always fix ESLint warnings and errors before finishing a task
Runyarn lint,yarn lint:ts, andyarn prettifyand fix potential errors before finishing a task
Files:
src/shared/components/appUpdateNeeded/__tests__/AppUpdateNeeded.test.tsx
src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{tsx,ts}: Always use arrow functions with extracted Props interfaces for React components
Don't useReact.FCtype for components; prefer interfaces to types for Props
UseComponentProps<typeof MyComponent>to extract Props type instead of manually defining them
Never define inline styles; always use Unistyles's StyleSheet API for styling
Maintain theme consistency using Unistyles theme file located atsrc/domain/theme/unistyles.ts
Files:
src/shared/components/appUpdateNeeded/__tests__/AppUpdateNeeded.test.tsx
src/shared/components/appUpdateNeeded/__tests__/AppUpdateNeeded.test.tsx
Show resolved
Hide resolved
e622fc0 to
b4faa2d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/domain/contexts/authContext/AuthContextProvider.tsx:
- Around line 60-66: The effect using hasTrackedUserRef currently sets it to
true on first sign-in and never resets, so reset hasTrackedUserRef.current to
false when the user becomes null (sign-out) inside the same or a separate
useEffect watching `user` so subsequent sign-ins trigger tracking; in addition,
replace the `void startTrackingUser(user)` call with proper async handling
(await or .catch) and surface/log errors consistently with `signIn`/`signOut`
(e.g., try { await startTrackingUser(user) } catch (err) { process/log or
rethrow }) to avoid silently ignoring failures; update references to
`hasTrackedUserRef`, `useEffect`, and `startTrackingUser` accordingly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.coderabbit.yamlsrc/domain/contexts/authContext/AuthContextProvider.tsxsrc/shared/components/appUpdateNeeded/__tests__/AppUpdateNeeded.test.tsxsrc/shared/uiKit/bottomSheet/BottomSheet.tsx
✅ Files skipped from review due to trivial changes (1)
- .coderabbit.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- src/shared/components/appUpdateNeeded/tests/AppUpdateNeeded.test.tsx
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Always use TypeScript for code files
Use ES modules withimportstatements instead ofrequire
Always wrap arrow function arguments in parentheses
Prefer async/await for promises and always handle error cases
Always fix ESLint warnings and errors before finishing a task
Runyarn lint,yarn lint:ts, andyarn prettifyand fix potential errors before finishing a task
Files:
src/domain/contexts/authContext/AuthContextProvider.tsxsrc/shared/uiKit/bottomSheet/BottomSheet.tsx
src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{tsx,ts}: Always use arrow functions with extracted Props interfaces for React components
Don't useReact.FCtype for components; prefer interfaces to types for Props
UseComponentProps<typeof MyComponent>to extract Props type instead of manually defining them
Never define inline styles; always use Unistyles's StyleSheet API for styling
Maintain theme consistency using Unistyles theme file located atsrc/domain/theme/unistyles.ts
Files:
src/domain/contexts/authContext/AuthContextProvider.tsxsrc/shared/uiKit/bottomSheet/BottomSheet.tsx
🧠 Learnings (1)
📚 Learning: 2026-01-07T07:41:02.477Z
Learnt from: CR
Repo: tsyirvo/react-native-starter PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T07:41:02.477Z
Learning: Applies to src/**/*.{tsx,ts} : Never define inline styles; always use Unistyles's StyleSheet API for styling
Applied to files:
src/shared/uiKit/bottomSheet/BottomSheet.tsx
🧬 Code graph analysis (1)
src/shared/uiKit/bottomSheet/BottomSheet.tsx (1)
src/shared/uiKit/primitives/Box.tsx (1)
Box(31-45)
🔇 Additional comments (4)
src/shared/uiKit/bottomSheet/BottomSheet.tsx (2)
5-5: Excellent refactor to use Unistyles StyleSheet API!The inline style
style={{ marginBottom: bottomInset }}has been successfully removed and replaced with a proper StyleSheet-based approach usingstyles.viewWrapperwithrt.insets.bottom. This fully addresses the previous review comment and aligns with the coding guideline.Also applies to: 43-43, 72-80, 96-109
62-80: Clarify the intentional spacing difference between scrollable and non-scrollable branches.The non-scrollable branch applies additional bottom margin that the scrollable branch does not:
- Scrollable (
BottomSheetScrollView):marginBottom: spacing_24only- Non-scrollable (
BottomSheetView):marginBottom: spacing_24 + rt.insets.bottomThis difference should either be:
- Justified with a code comment explaining why
BottomSheetScrollViewdoesn't need safe area bottom inset handling (if@gorhom/bottom-sheethandles it automatically), or- Made consistent by applying
viewWrapperstyle to the scrollable branch as wellsrc/domain/contexts/authContext/AuthContextProvider.tsx (2)
2-2: LGTM!The addition of
useEffectanduseRefto the imports is necessary for the new tracking logic, and the relative import path update is appropriate.Also applies to: 18-18
68-90: LGTM! Duplicate tracking issue resolved.The removal of the direct
startTrackingUsercall fromsignIncorrectly addresses the duplicate tracking issue flagged in the previous review. The dependency array now correctly reflects only the functions actually used withinsignIn.
b4faa2d to
bc66acb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shared/components/appUpdateNeeded/__tests__/AppUpdateNeeded.test.tsx (1)
61-61: Fix typo in test description.The test name contains a typo: "udpdate" should be "update".
📝 Proposed fix
- it('should open the stores to allow users to udpdate', async () => { + it('should open the stores to allow users to update', async () => {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.coderabbit.yamlsrc/domain/contexts/authContext/AuthContextProvider.tsxsrc/shared/components/appUpdateNeeded/__tests__/AppUpdateNeeded.test.tsxsrc/shared/uiKit/bottomSheet/BottomSheet.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/shared/uiKit/bottomSheet/BottomSheet.tsx
- src/domain/contexts/authContext/AuthContextProvider.tsx
- .coderabbit.yaml
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Always use TypeScript for code files
Use ES modules withimportstatements instead ofrequire
Always wrap arrow function arguments in parentheses
Prefer async/await for promises and always handle error cases
Always fix ESLint warnings and errors before finishing a task
Runyarn lint,yarn lint:ts, andyarn prettifyand fix potential errors before finishing a task
Files:
src/shared/components/appUpdateNeeded/__tests__/AppUpdateNeeded.test.tsx
src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{tsx,ts}: Always use arrow functions with extracted Props interfaces for React components
Don't useReact.FCtype for components; prefer interfaces to types for Props
UseComponentProps<typeof MyComponent>to extract Props type instead of manually defining them
Never define inline styles; always use Unistyles's StyleSheet API for styling
Maintain theme consistency using Unistyles theme file located atsrc/domain/theme/unistyles.ts
Files:
src/shared/components/appUpdateNeeded/__tests__/AppUpdateNeeded.test.tsx
🔇 Additional comments (1)
src/shared/components/appUpdateNeeded/__tests__/AppUpdateNeeded.test.tsx (1)
24-25: Type annotations now correctly match the discriminated union shape.The generic type parameter
T = { type: 'version'; version: string }now properly matches the mocked return type{ type: 'version', version: '2.0.0' }. This resolves the type annotation inconsistency identified in the previous review.Also applies to: 37-38, 50-51, 65-66
Summary by CodeRabbit
Chores
Bug Fixes
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.