-
Notifications
You must be signed in to change notification settings - Fork 180
SELF-702: Refactor navigation structure and dev utilities #994
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
Changes from all commits
660a2e8
d4f7eb1
34124d8
23bc8d5
57e97c7
7debec2
4280994
98ec73b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,19 @@ | ||||||
| // SPDX-FileCopyrightText: 2025 Social Connect Labs, Inc. | ||||||
| // SPDX-License-Identifier: BUSL-1.1 | ||||||
| // NOTE: Converts to Apache-2.0 on 2029-06-11 per LICENSE. | ||||||
|
|
||||||
| import { lazy, type LazyExoticComponent } from 'react'; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Import ComponentType to avoid React namespace type issues Referencing React.ComponentType without importing the React namespace can fail under stricter TS configs (NodeNext/Bundler). Import the type explicitly. -import { lazy, type LazyExoticComponent } from 'react';
+import { lazy, type LazyExoticComponent, type ComponentType } from 'react';📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| // Helper around React.lazy that exposes the underlying dynamic import | ||||||
| // so callers can manually preload a screen when debugging or profiling. | ||||||
| // Prefer using React.lazy directly and opt into this only when you need | ||||||
| // to eagerly load a component. | ||||||
| export function lazyWithPreload<T extends React.ComponentType<any>>( | ||||||
| factory: () => Promise<{ default: T }>, | ||||||
| ) { | ||||||
|
Comment on lines
+11
to
+13
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Remove any and add explicit return type for stronger type-safety Eliminate any, align with static analysis, and make the API surface explicit. Also type preload via typeof factory to prevent drift. -export function lazyWithPreload<T extends React.ComponentType<any>>(
- factory: () => Promise<{ default: T }>,
-) {
+export function lazyWithPreload<T extends ComponentType<unknown>>(
+ factory: () => Promise<{ default: T }>,
+): LazyExoticComponent<T> & { preload: typeof factory } {
🧰 Tools🪛 GitHub Check: build-deps[warning] 11-11: |
||||||
| const Component = lazy(factory) as LazyExoticComponent<T> & { | ||||||
| preload: () => Promise<{ default: T }>; | ||||||
| }; | ||||||
| Component.preload = factory; | ||||||
| return Component; | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,8 +16,8 @@ const AccountRecoveryScreen = lazy( | |
| const AccountVerifiedSuccessScreen = lazy( | ||
| () => import('@/screens/recovery/AccountVerifiedSuccessScreen'), | ||
| ); | ||
| const PassportDataNotFound = lazy( | ||
| () => import('@/screens/recovery/PassportDataNotFoundScreen'), | ||
| const DocumentDataNotFound = lazy( | ||
| () => import('@/screens/recovery/DocumentDataNotFoundScreen'), | ||
| ); | ||
|
Comment on lines
+19
to
21
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainPreload recovery fallback screen and verify route/key propagation
Apply within these hunks: -const DocumentDataNotFound = lazy(
+const DocumentDataNotFound = lazyWithPreload(
() => import('@/screens/recovery/DocumentDataNotFoundScreen'),
);Add the helper import: import { lazyWithPreload } from '@/utils/lazyWithPreload';Quick checks: Also applies to: 49-57 🏁 Script executed: #!/bin/bash
rg -nP "PassportDataNotFound\b|navigate\\(['\"]PassportDataNotFound['\"]|name:\\s*['\"]PassportDataNotFound['\"]" -SLength of output: 124 🏁 Script executed: #!/bin/bash
rg -nP "DocumentDataNotFound\b|navigate\(['\"]DocumentDataNotFound['\"]\)|name:\s*['\"]DocumentDataNotFound['\"]" -SLength of output: 699 Preload recovery fallback screens
🤖 Prompt for AI Agents |
||
| const RecoverWithPhraseScreen = lazy( | ||
| () => import('@/screens/recovery/RecoverWithPhraseScreen'), | ||
|
|
@@ -46,8 +46,8 @@ const recoveryScreens = { | |
| animation: 'slide_from_bottom', | ||
| } as NativeStackNavigationOptions, | ||
| }, | ||
| PassportDataNotFound: { | ||
| screen: PassportDataNotFound, | ||
| DocumentDataNotFound: { | ||
| screen: DocumentDataNotFound, | ||
| options: { | ||
| headerShown: false, | ||
| gestureEnabled: false, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,13 +5,13 @@ | |
| import { lazy } from 'react'; | ||
| import type { NativeStackNavigationOptions } from '@react-navigation/native-stack'; | ||
|
|
||
| const PassportDataNotFound = lazy( | ||
| () => import('@/screens/recovery/PassportDataNotFoundScreen'), | ||
| const DocumentDataNotFound = lazy( | ||
| () => import('@/screens/recovery/DocumentDataNotFoundScreen'), | ||
| ); | ||
|
Comment on lines
+8
to
10
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Web parity: consider lazyWithPreload here too Keep recovery.web.ts consistent with native for predictable chunking/preload behavior. -const DocumentDataNotFound = lazy(
+const DocumentDataNotFound = lazyWithPreload(
() => import('@/screens/recovery/DocumentDataNotFoundScreen'),
);Add: import { lazyWithPreload } from '@/utils/lazyWithPreload';Also applies to: 13-15 🤖 Prompt for AI Agents |
||
|
|
||
| const recoveryScreens = { | ||
| PassportDataNotFound: { | ||
| screen: PassportDataNotFound, | ||
| DocumentDataNotFound: { | ||
| screen: DocumentDataNotFound, | ||
| options: { | ||
| headerShown: false, | ||
| gestureEnabled: false, | ||
|
|
||
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.
🛠️ Refactor suggestion
Back arrow and icon tint may be invisible on dark headers
You set a black header and white title but didn’t set headerTintColor, so back chevron and header buttons may be low-contrast depending on theme/platform. Set headerTintColor to white in the shared options.
const devHeaderOptions: NativeStackNavigationOptions = { headerStyle: { backgroundColor: black, }, headerTitleStyle: { color: white, }, + headerTintColor: white, headerBackTitle: 'close', };📝 Committable suggestion
🤖 Prompt for AI Agents