Skip to content

fix: stop leaking scroll listeners under React StrictMode - #2894

Merged
n0rahh merged 2 commits into
Redocly:v2.xfrom
RClaudiuM:fix/2842-strictmode-scroll-listeners
Nov 10, 2026
Merged

n0rahh merged 2 commits into
Redocly:v2.xfrom
RClaudiuM:fix/2842-strictmode-scroll-listeners

Conversation

@RClaudiuM

Copy link
Copy Markdown

What/Why/How?

Fixes #2842 (v2: RedocStandalone under React Strict Mode).

AppStore attached its window scroll listener (ScrollService) and its menu subscriptions in the constructor. StoreBuilder builds the store in useMemo, which Strict Mode runs twice, so the store that is never rendered keeps its listener after unmount. Strict Mode's simulated unmount/remount of <Redoc> also disposed the live store, and onDidMount never bound it again, so the mounted docs lost their scroll-spy in development.

Changes:

  • ScrollService no longer binds in its constructor. bind() is idempotent and dispose() resets it.
  • MenuStore no longer subscribes in its constructor. subscribe() is idempotent and dispose() clears the handles.
  • AppStore.onDidMount() binds the scroll listener, subscribes the menu and re-registers the active-item observer, so a disposed store can be mounted again.
  • <Redoc> disposes the previous store and mounts the new one when its store prop changes. StoreBuilder creates a new store whenever the options object changes, e.g. an inline options={{ ... }}. Before this change the old store's listener stayed attached.

Not covered: a store disposed by the Strict Mode remount still has its search worker terminated. That is unchanged here.

The PR targets v2.x, because main (v3) no longer has StoreBuilder/AppStore.

Reference

Fixes #2842

Tests

New src/components/__tests__/Redoc.test.tsx counts the scroll listeners on window:

Test Before After
RedocStandalone in StrictMode, after unmount 1 left 0
Redoc in StrictMode, while mounted 0 (scroll-spy lost) 1
Redoc re-rendered with a new store 2 1 (old store disposed)
  • npx jest: 28 suites, 279 tests pass
  • npm run ts-check, eslint and prettier --check on the changed files, npm run bundle, npm run license-check: pass

Check yourself

  • Code is linted
  • Tested
  • All new/updated code is covered with tests

AppStore bound its window scroll listener and its menu subscriptions in
the constructor. StoreBuilder builds the store inside useMemo, which
StrictMode runs twice, so the store that was never rendered kept its
listener forever. StrictMode's simulated unmount/remount of <Redoc> also
disposed the live store, and onDidMount never bound it again.

Bind the scroll listener and the menu subscriptions in onDidMount
instead, make binding idempotent, and let a disposed store be mounted
again. <Redoc> now also disposes the previous store and mounts the new
one when its store prop changes, which StoreBuilder does whenever the
options object changes.

Fixes Redocly#2842
@RClaudiuM
RClaudiuM requested a review from a team as a code owner November 7, 2026 20:59

@n0rahh n0rahh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the PR! Here is one small finding:

Comment thread src/components/Redoc/Redoc.tsx Resolved

@n0rahh n0rahh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the contribution!

For the follow-up you mentioned (moving the worker cleanup to StoreBuilder, which owns the store), it would be great to cover both cases:

  • a StrictMode dev remount of <Redoc> terminates the worker of the store that is still shown, so search stops working;
  • a store that StoreBuilder replaces (e.g. inline options={{ ... }} re-created on every parent render) now only gets detach(), so its search worker is never terminated and each such re-render leaves one worker thread behind (same as on main today).

@n0rahh
n0rahh merged commit d524b6d into Redocly:v2.x Nov 10, 2026
2 of 3 checks passed
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.

3 participants