Fix TypeScript errors and lint violations in Expo Router migration#406
Conversation
Co-authored-by: manuthecoder <77016441+manuthecoder@users.noreply.github.com>
Co-authored-by: manuthecoder <77016441+manuthecoder@users.noreply.github.com>
Co-authored-by: manuthecoder <77016441+manuthecoder@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR finishes the Expo Router migration cleanup by resolving remaining TypeScript and ESLint violations, primarily around Expo Router param serialization/typing and unused navigation props introduced by the navigation rewrite.
Changes:
- Serialize complex route params (e.g.,
organizationfallback data) and add typeduseLocalSearchParamsgenerics in dynamic routes. - Remove/rename unused imports and variables to satisfy ESLint (
navigation→_navigation, import ordering, dead imports). - Small type fixes and safety tweaks (e.g.,
formatEntityNamessignature,Transaction.tsxts-expect cleanup, receipts selection memo typing).
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/util.ts | Updates helpers to support Expo Router param serialization and adjusts yellowpages name typing. |
| src/lib/yellowpages/data.json | Updates bundled metadata timestamp in yellowpages dataset. |
| src/lib/yellowpages/.last-updated | Updates dataset build timestamp marker. |
| src/lib/useDigitalWallet.ts | Adds targeted ESLint suppression for intentional require() usage. |
| src/components/transaction/types/WiseTransaction.tsx | Renames unused navigation prop to _navigation to satisfy lint. |
| src/components/transaction/types/TransferTransaction.tsx | Fixes import ordering and renames unused navigation prop. |
| src/components/transaction/types/InvoiceTransaction.tsx | Renames unused navigation prop to _navigation. |
| src/components/transaction/types/ExpensePayoutTransaction.tsx | Renames unused navigation prop to _navigation. |
| src/components/transaction/types/DonationTransaction.tsx | Renames unused navigation prop to _navigation. |
| src/components/transaction/types/CheckTransaction.tsx | Renames unused navigation prop to _navigation. |
| src/components/transaction/types/CardChargeTransaction.tsx | Renames unused navigation prop to _navigation. |
| src/components/transaction/types/BankFeeTransaction.tsx | Renames unused navigation prop to _navigation. |
| src/components/transaction/types/BankAccountTransaction.tsx | Renames unused navigation prop to _navigation. |
| src/components/transaction/types/AchTransferTransaction.tsx | Renames unused navigation prop to _navigation. |
| src/components/transaction/Transaction.tsx | Removes now-unnecessary @ts-expect-error for Icon glyph typing. |
| src/components/organizations/TransactionWrapper.tsx | Removes unused import after navigation migration. |
| package-lock.json | Lockfile updates reflecting dependency tree changes (dev flags/removed peers). |
| components/Navbar.tsx | Narrows options typing and renames unused navigation prop. |
| app/login/index.tsx | Removes dead navigation/SWR imports after router migration. |
| app/(app)/receipts/selection.tsx | Extends parsed transaction typing and guards memo access with optional chaining. |
| app/(app)/cards/card-grants/[id].tsx | Adds typed useLocalSearchParams to avoid `string |
| app/(app)/_layout.tsx | Uses non-null assertion for SWRCacheProvider consumption (context provided by parent layout). |
| app/(app)/(events)/index.tsx | Serializes fallback org data in navigation params for Expo Router. |
| app/(app)/(events)/[id]/transactions/index.tsx | Types route params and parses serialized fallback org data for offline SWR. |
| app/(app)/(events)/[id]/index.tsx | Types route params and parses serialized fallback org data; updates orgId typing. |
| app/(app)/(events)/[id]/donations/index.tsx | Adds typed useLocalSearchParams for dynamic route param. |
| app/(app)/(events)/[id]/cards/order.tsx | Prefixes intentionally-unused locals to satisfy lint rules. |
| app/(app)/(events)/[id]/account-numbers.tsx | Types route params and parses serialized fallback org data for offline SWR. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fallbackData: params.fallbackData | ||
| ? (JSON.parse(params.fallbackData) as Organization | OrganizationExpanded) | ||
| : undefined, |
There was a problem hiding this comment.
params.fallbackData is parsed with JSON.parse without any error handling. If the route is opened from a deeplink with a malformed fallbackData value, this will throw and crash the page. Recommend checking typeof params.fallbackData === "string" and using a try/catch to fall back to undefined when parsing fails.
| fallbackData: params.fallbackData | ||
| ? (JSON.parse(params.fallbackData) as OrganizationExpanded) | ||
| : undefined, |
There was a problem hiding this comment.
params.fallbackData is parsed with JSON.parse without validating the input or handling malformed JSON. If a user opens this route via deeplink with an invalid fallbackData query param, the screen will throw during render. Consider guarding for typeof params.fallbackData === "string" and wrapping the parse in try/catch, defaulting to undefined when parsing fails.
| organization={organization} | ||
| orgId={params.id} | ||
| isFirst={item.isFirst} | ||
| orgId={params.id as `org_${string}`} |
There was a problem hiding this comment.
TransactionWrapper defaults isFirst to false, but this render path no longer passes item.isFirst. That likely regresses styling (top rounding/spacing) for the first transaction in a section and also defeats the component’s memoization that depends on isFirst. Pass isFirst={item.isFirst} here (similar to how mock transactions pass top={item.isFirst}).
| orgId={params.id as `org_${string}`} | |
| orgId={params.id as `org_${string}`} | |
| isFirst={item.isFirst} |
| organization={organization} | ||
| orgId={params.id} | ||
| isFirst={item.isFirst} | ||
| orgId={params.id as `org_${string}`} |
There was a problem hiding this comment.
Using params.id as \org_${string}`removes type safety and can mask bad route params at runtime. SinceOrganization.idis already typed as a template literal, prefer typing the route params as{ id: org_${string}; ... }` so this component doesn’t need assertions and incorrect IDs are caught earlier.
| <TapToPayBanner | ||
| onDismiss={handleDismissTapToPayBanner} | ||
| orgId={params.id} | ||
| orgId={params.id as `org_${string}`} |
There was a problem hiding this comment.
Casting params.id to org_${string} here hides the fact that the route param is currently typed as a plain string. Prefer updating useLocalSearchParams’ generic to { id: org_${string}; fallbackData?: string } so you don’t need repeated as assertions and you retain end-to-end type safety for org IDs.
| const baseParams = { | ||
| id: organization.id, | ||
| fallbackData: organization as unknown, | ||
| fallbackData: JSON.stringify(organization), | ||
| }; |
There was a problem hiding this comment.
Serializing the entire organization object into route params (fallbackData) can leak sensitive fields (e.g., account_number/routing_number on OrganizationExpanded) into the URL/navigation state and can also create very large query strings. Prefer passing only the minimal primitives needed for rendering (e.g., id + name) and relying on SWR cache/offline cache for the full object, or explicitly stripping sensitive fields before stringifying.
| fallbackData: params.fallbackData | ||
| ? (JSON.parse(params.fallbackData) as Organization | OrganizationExpanded) | ||
| : undefined, |
There was a problem hiding this comment.
params.fallbackData is parsed with JSON.parse without validating that it’s a single string or handling malformed JSON. A bad/deeplinked value (or repeated query param producing string[]) will throw and crash the screen. Consider guarding with typeof params.fallbackData === "string" and wrapping parsing in a try/catch (falling back to undefined on failure).
Summary of the problem
PR #399 migrated HCB Mobile from React Native Navigation to Expo Router, but left 29 TypeScript compilation errors and 20 ESLint errors that blocked merging. The root causes: Expo Router route params must be serializable primitives (not objects),
useLocalSearchParams()returnsstring | string[]by default, and several navigation-related variables/imports became dead code post-migration.Describe your changes
fallbackDataserialization — Route params cannot carry complex objects across Expo Router navigations:Typed
useLocalSearchParams— All dynamic route files now use explicit generics to avoidstring | string[]errors:Other TypeScript fixes:
SWRCacheProvidernull context in_layout.tsx— non-null assertion (context is always provided by parent layout)Navbaroptionsprop typed as{ title?: string }instead ofunknownselection.tsxtransaction type extended withmemo+ optional chaining on possibly-null accessformatEntityNamessignature acceptsstring | undefined | nullfromgetName()to match actualMerchant/Categoryimplementations@ts-ignoreinTransaction.tsx(underlying error was resolved upstream)ESLint fixes (20 errors → 0):
navigationdestructured params tonavigation: _navigationin 10 transaction type componentsgetTransactionTitle,router,useSWRConfigTransferTransaction.tsxandutil.tseligibleOrganizations/navigationlocals inorder.tsxandtransactions/index.tsxeslint-disablefor intentional conditionalrequire()inuseDigitalWallet.ts(iOS watch connectivity, must stay as try/catch require)Checklist
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.