Split out of #2153 (phase 2 step 2 of the App.tsx decomposition, under #2129 / #2126).
What happened
eslint-plugin-react-hooks v6's recommended set includes the compiler-powered rules (react-hooks/refs, react-hooks/set-state-in-effect). Those run the React Compiler over a component and report nothing at all when compilation of that component bails out — which is what App.tsx was doing. Lifting ~560 lines of the OAuth-recovery cluster out of it in #2153 dropped it below whatever the bail-out was keyed on, so the compiler now analyses the component and reports nine findings that were latent the whole time:
| Rule |
Sites |
react-hooks/refs |
the render-time "latest ref" writes (sandboxUrlRef, listedResourcesRef), the initialConfigSettledRef seed, and the two createAppBridgeFactory({ … }) calls that receive them |
react-hooks/set-state-in-effect |
the connectedServerId status effect, and the deep-link auto-connect effect |
(The one seam #2153 itself introduced, setupClientForServerRef, is not in that list: review moved its publication into a layout effect, so it satisfies the rule outright.)
None of them is new code and none is a regression: each is a deliberate, already-commented pattern (sandboxUrlRef even carries a comment explaining why it must be written every render). #2153 suppressed each with a narrowly-scoped eslint-disable-next-line naming this issue, because lint has no warning tier (#2085) and fixing them is real behaviour change rather than a refactor.
What to do
For each suppressed site, decide between:
- Fix it — e.g.
connectedServerId is a candidate for plain derived state rather than a status-driven effect.
- Keep it and say so once — the latest-ref writes exist so a synchronous, memoized
setupClientForServer reads the current value rather than the one captured by the render a caller started in. If that stays, the suppression should carry the argument rather than a pointer to this issue.
Either way the eslint-disable comments should be gone or re-justified when this closes. Note the suppressions are load-bearing in the meantime: reportUnusedDisableDirectives means a directive that stops being needed becomes a warning, and a warning fails lint.
Done when
Split out of #2153 (phase 2 step 2 of the
App.tsxdecomposition, under #2129 / #2126).What happened
eslint-plugin-react-hooksv6's recommended set includes the compiler-powered rules (react-hooks/refs,react-hooks/set-state-in-effect). Those run the React Compiler over a component and report nothing at all when compilation of that component bails out — which is whatApp.tsxwas doing. Lifting ~560 lines of the OAuth-recovery cluster out of it in #2153 dropped it below whatever the bail-out was keyed on, so the compiler now analyses the component and reports nine findings that were latent the whole time:react-hooks/refssandboxUrlRef,listedResourcesRef), theinitialConfigSettledRefseed, and the twocreateAppBridgeFactory({ … })calls that receive themreact-hooks/set-state-in-effectconnectedServerIdstatus effect, and the deep-link auto-connect effect(The one seam #2153 itself introduced,
setupClientForServerRef, is not in that list: review moved its publication into a layout effect, so it satisfies the rule outright.)None of them is new code and none is a regression: each is a deliberate, already-commented pattern (
sandboxUrlRefeven carries a comment explaining why it must be written every render). #2153 suppressed each with a narrowly-scopedeslint-disable-next-linenaming this issue, becauselinthas no warning tier (#2085) and fixing them is real behaviour change rather than a refactor.What to do
For each suppressed site, decide between:
connectedServerIdis a candidate for plain derived state rather than a status-driven effect.setupClientForServerreads the current value rather than the one captured by the render a caller started in. If that stays, the suppression should carry the argument rather than a pointer to this issue.Either way the
eslint-disablecomments should be gone or re-justified when this closes. Note the suppressions are load-bearing in the meantime:reportUnusedDisableDirectivesmeans a directive that stops being needed becomes a warning, and a warning failslint.Done when
react-hookssuppression added by Decompose App.tsx phase 2 step 2: extract useOAuthRecovery #2153 inclients/web/src/App.tsxis either removed (the finding fixed) or replaced with its own standalone justificationnpm run cigreen