Skip to content

fix(imodel-browser): prevent AbortController from breaking fetch in React 18 Strict Mode#221

Merged
aruniverse merged 7 commits into
mainfrom
hl662/fix-abort-controller-strict-mode
Jun 26, 2026
Merged

fix(imodel-browser): prevent AbortController from breaking fetch in React 18 Strict Mode#221
aruniverse merged 7 commits into
mainfrom
hl662/fix-abort-controller-strict-mode

Conversation

@hl662

@hl662 hl662 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Why

Fixes #128.

React 18 Strict Mode double-invokes effects in development: mount -> cleanup -> mount. The previous implementation stored AbortController in useState and had a dedicated cleanup effect:

useEffect(() => () => abortController?.abort(), [abortController]);

On Strict Mode's simulated unmount, this cleanup fired and aborted the in-flight fetch. Because needsUpdate was already set to false inside the effect, no re-fetch was triggered. The hook was permanently stuck at status: Fetching.

As a small boy-scout cleanup, useIModelData.ts also switches from the React namespace 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.ts

  • Moved AbortController from useState -> useRef (abortControllerRef). A ref mutation doesn't schedule a re-render, so it can't cause the stale-closure / infinite-loop problems that motivated the useState approach.
  • Added activeRequestRef = useRef<symbol>(). Each fetch run assigns a new Symbol() 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.
  • Removed the standalone useEffect(() => () => abort(), [abortController]) cleanup.
  • Added an unmount-only cleanup effect ([] deps) that clears activeRequestRef and aborts any in-flight request on true component unmount.
  • Removed abortController, iModels, and morePagesAvailable from the main effect's dependency array (they were either causing spurious re-runs or are now unnecessary).
  • Switched to named React hook imports instead of React.useXxx namespace calls.

useIModelData.test.ts

  • Added regression test: "completes fetch when mounted inside React.StrictMode" -- uses { wrapper: StrictMode } and asserts status === Complete with the correct iModel data.
  • Placed the test before the it.each cases that spy on window.fetch to avoid test isolation issues with spy leakage.
  • Changed afterEach to call jest.restoreAllMocks() alongside server.resetHandlers().

rush.json / ci.yml

  • Added ^24.11.0 to nodeSupportedVersionRange per the iTwin.js supported platforms policy (Node 24 LTS supported as of 24.11.0).
  • Updated pnpm from 7.32.2 to 7.33.7 (latest pnpm 7.x).
  • Updated CI matrix node-version from 20.x to 24.x.

How it works

Strict Mode sequence (before fix):       Strict Mode sequence (after fix):
--------------------------------------   ----------------------------------------
1. Effect runs -> fetch starts           1. Effect runs -> fetch starts (requestId=A)
2. Cleanup fires -> abort() called       2. Cleanup fires -> activeRequestRef = undefined
3. fetch rejects with AbortError         3. fetch resolves -> requestId A != undefined -> dropped
4. needsUpdate=false -> no re-fetch      4. Effect re-runs -> fetch starts (requestId=B)
5. status stuck at Fetching [FAIL]       5. fetch resolves -> requestId B matches -> [PASS]

hl662 and others added 4 commits June 18, 2026 16:31
…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>
@ben-polinsky

This comment was marked as resolved.

hl662 and others added 3 commits June 26, 2026 12:28
…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>
@hl662 hl662 marked this pull request as ready for review June 26, 2026 18:24
@aruniverse aruniverse merged commit abedb75 into main Jun 26, 2026
5 checks passed
@aruniverse aruniverse deleted the hl662/fix-abort-controller-strict-mode branch June 26, 2026 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate AbortController() pre-emptively stopping requests

3 participants