refactor(mobile): add a robust @/ import alias for apps/mobile/src - #13152
juliusmarminge wants to merge 2 commits into
Conversation
Mobile has 100+ imports at ../../../ depth and no alias to shorten them, while web resolves ~/* to its src. Add "@/*" tsconfig paths plus a Metro resolveRequest alias (extraNodeModules cannot host it: Metro parses "@/foo" as the scoped package "@/foo"), mirror it for vitest in the root vite config, and migrate a bounded set of representative deep imports. Co-authored-by: Apex <noreply@callstack.com>
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a bounded import-path refactor with matching TypeScript and test-tool configuration; all migrated imports continue to target the same existing modules. It does not add user-facing behavior, alter product defaults, or suppress static-analysis diagnostics. You can add or adjust custom eligibility rules. Learn more. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Limit details: You’ve used all 10 included reviews currently available. 📝 WalkthroughWalkthroughTypeScript and Vite now resolve the mobile ChangesMobile import aliases
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~8 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to The mobile alias configuration consistently targets the mobile source directory, so the selected migrated imports are ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
Expo 57 resolves tsconfig compilerOptions.paths in Metro by default (exp.experiments.tsconfigPaths defaults to true in @expo/cli's instantiateMetro), and its resolver is importer-scoped: it skips requests originating in node_modules, which the plain prefix hook was not. Verified by exporting both platforms from apps/mobile: iOS Bundled (3886 modules) and Android Bundled (4041 modules). Co-authored-by: Apex <noreply@callstack.com>
Dismissing prior approval to re-evaluate 84cc687
The problem
Audit #8: mobile has 101 imports at
../../../depth and 15 at../../../../, while web resolves~/*against itssrc(814 uses). Mobile had no tsconfigpaths, so every deep import is edit friction and every file move silently breaks relative paths across the app.The fix
apps/mobile/tsconfig.json—"paths": { "@/*": ["./src/*"] }, matching the web~/*precedent. Expo 57's Metro already resolves tsconfigpathsnatively (exp.experiments.tsconfigPathsdefaults totruein@expo/cli'sinstantiateMetro), so no Metro config change is needed — and Expo's resolver is importer-scoped (it skips requests originating innode_modules), which a plain prefix hook is not.vite.config.ts(root) — mobile unit tests run under the root vitest config, which has no knowledge of tsconfig paths, so the mapping is mirrored there as a@/→apps/mobile/src/alias. The existing web~alias is kept byte-for-byte via the array form.Migrated a bounded, representative set of deep imports rather than all ~200 files, mixing runtime values, type-only imports,
.tsand.tsx, across four features (sharing, connection, threads, settings). The rest can migrate incrementally — both styles coexist.Verification (no UI change → no screenshots)
expo export --dev, apps/mobile):iOS Bundled apps/mobile/index.ts (3886 modules)andAndroid Bundled apps/mobile/index.ts (4041 modules)— the full app graph, including the migrated@/imports, resolves through Expo's tsconfig-paths resolver with no Metro override.tsc --noEmitinapps/mobile— clean with the migrated@/imports.incoming-share-model,incoming-share-inbox,environmentSections,new-task-project-selection— 4 files / 57 tests passed, all resolving@/lib/...value imports at runtime. Web alias regression:browserDefaults.test.ts(~imports) passes unchanged.Revisions
Initial head
2efab1ealso added a customconfig.resolver.resolveRequesthook inmetro.config.js. Audit review (and a re-run of both platform exports) confirmed Expo's built-in tsconfig-paths resolution makes the hook redundant, and that it would additionally hijack a dependency's own@/…imports fromnode_modules, which Expo's importer-scoped resolver correctly leaves alone.84cc687removes it;metro.config.jsis back to unchanged.Model:
callstack/Apex; harness: Apex in T3 Code (pi).Summary by CodeRabbit