[NO QA] Restrict InteractionManager and TransitionTracker usage with 'no-restricted-imports' eslint rule#88525
Conversation
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
|
@dukenv0307 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #83072 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
LGTM |
|
@collectioneur would you merge main? |
|
@grgia Sure, merging now 😄 |
|
Merged main one more time, it looks like the tests are failing on main too 🙂 |
|
@collectioneur agh conflicts still, ping me when mergable 🙏 |
|
@grgia conflicts resolved 😄 |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@mrejdak msgd in slack, would you merge main |
|
@grgia Merged! |
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
…on tests Both useGettingStartedItems.test.ts and GettingStartedSectionTest.tsx asserted state immediately after renderHook / render returned, before the hook's Onyx subscriptions had fired their post-mount callbacks. The first render reflected initial empty Onyx state; the assertion ran against THAT, then the hook re-rendered milliseconds later with the hydrated values. In CI's parallel-worker environment the timing window made this fail deterministically on certain shards. Fix: add `await waitForBatchedUpdates()` after every renderHook / renderGettingStartedSection call. Same pattern the test files already use after Onyx setup; just extending it to post-render so the hook's own subscriptions also settle before the assertion. Confirmed flaky from inception: the test files were added in Expensify#88525 (merged ~24h ago) and immediately started failing on multiple unrelated branches (Expensify#89146 / fix/83781 / feat/84877 / wildan/88503 / Rory-RemoveUnnecessaryESLintDisable). Tracked in Expensify#89240. No production source changes — test-only patch. 76 lines added across the two files (one waitForBatchedUpdates call per renderHook / renderGettingStartedSection site).
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.3.65-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes required. This PR adds ESLint lint rules to restrict direct usage of |
Explanation of Change
Add ESLint
no-restricted-importsrules to prevent direct usage ofInteractionManager(fromreact-native) andTransitionTracker(internal primitive). Existing usages across ~125 files are grandfathered witheslint-disable-next-linecomments. This enforces that new code uses higher-level APIs (NavigationafterTransition/waitForTransition, KeyboardUtils, etc.) instead.Fixed Issues
$ #83072
Tests
// eslint-disable-next-line no-restricted-importscomment from any file importingInteractionManagerorTransitionTracker, then run ESLint on that file - verify a lint error is reported with the appropriate guidance messageOffline tests
N/A
QA Steps
No QA needed - this is a lint-only change with no runtime impact.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.