Repository navigation
[No QA] Split SettingsNavigatorParamList out of Navigation/types.ts to fix max-lines lint - #104367
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…of inline directive" This reverts commit 8e7ebf4.
Codecov Report✅ All modified and coverable lines are covered by tests. |
|
🚧 NicolasBonet has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/NicolasBonet in version: 9.5.14-0 🚀
|
|
No help site update is needed. This PR only moves navigation TypeScript types into new files and updates a lint rule, so nothing users see in the app changes. |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.5.14-3 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
src/libs/Navigation/types.tshovers right at the 4000-linemax-lineslimit, so its inline/* eslint-disable max-lines */keeps breaking lint: PRs that shrink the file fail with "unused directive", PRs that grow it failmax-lines(e.g. this run).This moves the 2194-line
SettingsNavigatorParamListunchanged intosrc/libs/Navigation/paramLists/(AccountSettingsParamList,WorkspaceSettingsParamList,DomainSettingsParamList), bringingtypes.tsdown to 1804 lines and removing the directive.SettingsNavigatorParamListis still exported fromtypes.ts, so no imports change. ThebackTolint rule andNAVIGATION.mdare updated to cover the new folder.Type-only move, no functional changes. I verified with the type checker that the old and new
SettingsNavigatorParamListare identical.Fixed Issues
$
PROPOSAL:
Tests
npx eslint --report-unused-disable-directives src/libs/Navigation/types.ts src/libs/Navigation/paramLists/.backTo?: Routes;param to any screen insrc/libs/Navigation/paramLists/WorkspaceSettingsParamList.tswithout aneslint-disablecomment.backTono-restricted-syntaxerror, then revert the change.npm run typecheckand verify it passes.Offline tests
N/A, lint-only change.
QA Steps
N/A, lint-only change ([No QA]).
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari