Repository navigation
Convert eligible ref-mirror effects to useEffectEvent (bd-t18384ia) - #938
Merged
shikokuchuo merged 3 commits intoOct 27, 2026
Merged
Conversation
Phase 5 of the hub-client useEffect refactor: triaged all 12 audited ref-mirror sites and converted the 3 whose call sites sit lexically inside an effect (useAuthProbe, useSessionKeepAlive, useWindowFileDrop). The other 9 fail the useEffectEvent contract enforced by eslint-plugin-react-hooks v7 rules-of-hooks (references only from effect scope in the same component; render-phase calls throw): useRouting reads inside a setState updater, useDownloadAs inside a click handler, Preview/ReactPreview inside render-scope useCallbacks, useSlideThumbnails recursively, and the three Editor refs cross hook/Monaco-callback boundaries. They stay as ref-mirrors, with the rationale recorded in the plan. useAuthProbe and useSessionKeepAlive gained 'latest callback without re-arming' characterization tests (pinned green before the conversion); useWindowFileDrop already had one. Verified: npm run test:ci all three legs green; npm run build:all green; eslint clean on the five touched files.
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #936. Phase 5 (final) of
claude-notes/plans/2026-10-24-hub-client-useeffect-refactor.md(bd-t18384ia).What changed
Converted the 3 ref-mirror effects whose call sites sit lexically inside an effect:
useAuthProbeuseSessionKeepAlive(both callbacks)useWindowFileDropWhat didn't change, and why
The other 9 audited sites fail the
useEffectEventcontract enforced by eslint-plugin-react-hooks v7rules-of-hooks: an effect event may only be referenced from effect scope in the same component, and render-phase calls throw. They stay as ref-mirrors; per-site rationale is recorded in the plan.useRouting— read inside asetStateupdater (runs during render)useDownloadAs— read inside a click handlerPreview/ReactPreview— read inside render-scopeuseCallbacksuseSlideThumbnails— recursive re-invocation from inside auseCallbackEditor.tsx(3 refs) — cross hook / Monaco-callback boundariesTests
useAuthProbe/useSessionKeepAlivegained "latest callback without re-arming the interval" characterization tests, pinned green before converting;useWindowFileDropalready had one.npm run test:cigreen (unit, integration, WASM);npm run build:allgreen; eslint clean on the five touched files.Browser smoke
import-document.spec.tstests on chromium, including the four window-fallback drops (top bar, dialog backdrop, preview pane ×2)./auth/meprobes fired through the converted effect event, no logout on network errors, reconnect restored the online pill.No user-visible behavior change, so no changelog entry (same call as Phases 2–4).