fix(imodel-browser): prevent AbortController from breaking fetch in React 18 Strict Mode#221
Merged
Merged
Conversation
…eact 18 Strict Mode React 18 Strict Mode double-invokes effects (mount → cleanup → mount). The previous implementation stored AbortController in useState, with a separate cleanup-only effect that called abort(). On Strict Mode's simulated unmount, the abort fired and needsUpdate=false prevented a re-fetch, leaving the hook permanently stuck at status=Fetching. Fix: move AbortController to useRef (abortControllerRef) and add a Symbol-based active request tracker (activeRequestRef). Each fetch run assigns a unique Symbol as the active request ID. The .then()/.catch() callbacks check this ID before applying results, so stale fetches from aborted effect runs are silently dropped. A separate unmount-only effect ([] deps) clears activeRequestRef and aborts any in-flight request on true unmount. Also: - Add --forceExit to jest to prevent the process hanging due to MSW keeping the Node event loop alive after tests complete. - Add pnpm-lock.yaml to .gitignore to prevent per-package lockfiles (created by the pnpm install workaround for the rush install pnpm 7.32.2 bug) from polluting diffs. - Add a regression test: 'completes fetch when mounted inside React.StrictMode' placed before tests that spy on window.fetch to avoid test isolation issues. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
…wser-react Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ct namespace Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bump pnpmVersion from 7.32.2 to 7.33.7 (latest pnpm 7.x). Add ^24.11.0 to nodeSupportedVersionRange per iTwin.js supported platforms — Node 24 LTS is supported as of 24.11.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
aruniverse
approved these changes
Jun 26, 2026
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.
Why
Fixes #128.
React 18 Strict Mode double-invokes effects in development:
mount -> cleanup -> mount. The previous implementation storedAbortControllerinuseStateand had a dedicated cleanup effect:On Strict Mode's simulated unmount, this cleanup fired and aborted the in-flight fetch. Because
needsUpdatewas already set tofalseinside the effect, no re-fetch was triggered. The hook was permanently stuck atstatus: Fetching.As a small boy-scout cleanup,
useIModelData.tsalso switches from theReactnamespace import to named hook imports (useCallback,useEffect, etc.) since it is not a component file.Also, bump node version and pnpm patch
What changed
useIModelData.tsAbortControllerfromuseState->useRef(abortControllerRef). A ref mutation doesn't schedule a re-render, so it can't cause the stale-closure / infinite-loop problems that motivated theuseStateapproach.activeRequestRef = useRef<symbol>(). Each fetch run assigns a newSymbol()as the active request ID. The.then()/.catch()callbacks compare against this ref before applying results -- stale fetches from aborted effect runs are silently dropped.useEffect(() => () => abort(), [abortController])cleanup.[]deps) that clearsactiveRequestRefand aborts any in-flight request on true component unmount.abortController,iModels, andmorePagesAvailablefrom the main effect's dependency array (they were either causing spurious re-runs or are now unnecessary).React.useXxxnamespace calls.useIModelData.test.ts{ wrapper: StrictMode }and assertsstatus === Completewith the correct iModel data.it.eachcases that spy onwindow.fetchto avoid test isolation issues with spy leakage.afterEachto calljest.restoreAllMocks()alongsideserver.resetHandlers().rush.json/ci.yml^24.11.0tonodeSupportedVersionRangeper the iTwin.js supported platforms policy (Node 24 LTS supported as of 24.11.0).7.32.2to7.33.7(latest pnpm 7.x).node-versionfrom20.xto24.x.How it works