From d9a4715a81cfc094db661dfd0ddc180af8c6bed4 Mon Sep 17 00:00:00 2001 From: Roman Claudiu Date: Sat, 7 Nov 2026 22:58:42 +0200 Subject: [PATCH 1/2] fix: stop leaking scroll listeners under React StrictMode 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 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. 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 #2842 --- src/components/Redoc/Redoc.tsx | 7 ++ src/components/__tests__/Redoc.test.tsx | 116 ++++++++++++++++++++++++ src/services/AppStore.ts | 20 +++- src/services/MenuStore.ts | 23 +++-- src/services/ScrollService.ts | 9 +- 5 files changed, 163 insertions(+), 12 deletions(-) create mode 100644 src/components/__tests__/Redoc.test.tsx diff --git a/src/components/Redoc/Redoc.tsx b/src/components/Redoc/Redoc.tsx index a3d0eef7cd..b90ac3bec5 100644 --- a/src/components/Redoc/Redoc.tsx +++ b/src/components/Redoc/Redoc.tsx @@ -28,6 +28,13 @@ export class Redoc extends React.Component { this.props.store.onDidMount(); } + componentDidUpdate(prevProps: RedocProps) { + if (prevProps.store !== this.props.store) { + prevProps.store.dispose(); + this.props.store.onDidMount(); + } + } + componentWillUnmount() { this.props.store.dispose(); } diff --git a/src/components/__tests__/Redoc.test.tsx b/src/components/__tests__/Redoc.test.tsx new file mode 100644 index 0000000000..6b845dae07 --- /dev/null +++ b/src/components/__tests__/Redoc.test.tsx @@ -0,0 +1,116 @@ +/* tslint:disable:no-implicit-dependencies */ +import * as React from 'react'; +import { createRoot, Root } from 'react-dom/client'; +import { act } from 'react-dom/test-utils'; + +import { Redoc, RedocStandalone } from '../'; +import { AppStore } from '../../services'; + +// jest falls back to the in-process search worker, which has no terminate() +const options = { disableSearch: true }; + +const spec = { + openapi: '3.0.0', + info: { title: 'Test', version: '1.0.0' }, + paths: { + '/pets': { + get: { operationId: 'listPets', responses: { '200': { description: 'OK' } } }, + }, + }, +}; + +describe('Redoc scroll listeners', () => { + const scrollListeners = new Set(); + let container: HTMLDivElement; + let root: Root; + + beforeAll(() => { + (globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + }); + + beforeEach(() => { + scrollListeners.clear(); + const addEventListener = window.addEventListener; + const removeEventListener = window.removeEventListener; + jest.spyOn(window, 'addEventListener').mockImplementation((type, listener, options) => { + if (type === 'scroll') scrollListeners.add(listener); + addEventListener.call(window, type, listener, options); + }); + jest.spyOn(window, 'removeEventListener').mockImplementation((type, listener, options) => { + if (type === 'scroll') scrollListeners.delete(listener); + removeEventListener.call(window, type, listener, options); + }); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(() => { + jest.restoreAllMocks(); + container.remove(); + }); + + const scrollSubscribers = (store: AppStore) => + (store as any).scroll._emiter.listenerCount('scroll') as number; + + test('RedocStandalone removes every scroll listener on unmount in StrictMode', async () => { + let loaded!: () => void; + const onLoaded = new Promise(resolve => (loaded = resolve)); + + await act(async () => { + root.render( + + loaded()} + /> + , + ); + }); + await act(async () => { + await onLoaded; + }); + + expect(scrollListeners.size).toBe(1); + + act(() => root.unmount()); + + expect(scrollListeners.size).toBe(0); + }); + + test('Redoc keeps the scroll-spy of its store after a StrictMode remount', () => { + const store = new AppStore(spec as any, undefined, options); + + act(() => { + root.render( + + + , + ); + }); + + expect(scrollListeners.size).toBe(1); + expect(scrollSubscribers(store)).toBe(1); + + act(() => root.unmount()); + + expect(scrollListeners.size).toBe(0); + }); + + test('Redoc disposes the previous store when it receives a new one', () => { + const first = new AppStore(spec as any, undefined, options); + const second = new AppStore(spec as any, undefined, options); + + act(() => root.render()); + act(() => root.render()); + + expect(scrollListeners.size).toBe(1); + expect(scrollSubscribers(first)).toBe(0); + expect(scrollSubscribers(second)).toBe(1); + + act(() => root.unmount()); + + expect(scrollListeners.size).toBe(0); + }); +}); diff --git a/src/services/AppStore.ts b/src/services/AppStore.ts index 082557f391..a3b8609540 100644 --- a/src/services/AppStore.ts +++ b/src/services/AppStore.ts @@ -79,13 +79,19 @@ export class AppStore { this.search.indexItems(this.menu.items); } - this.disposer = observe(this.menu, 'activeItemIdx', change => { - this.updateMarkOnMenu(change.newValue as number); - }); + this.observeActiveItem(); } } onDidMount() { + // listeners are attached on mount (not in the constructor), so a store that is never + // mounted, e.g. one built twice by React StrictMode, doesn't leak them, and a store that + // is disposed and mounted again (StrictMode remount) gets them back + this.scroll.bind(); + this.menu.subscribe(); + if (this.search) { + this.observeActiveItem(); + } this.menu.updateOnHistory(); this.updateMarkOnMenu(this.menu.activeItemIdx); } @@ -98,9 +104,17 @@ export class AppStore { } if (this.disposer != null) { this.disposer(); + this.disposer = null; } } + private observeActiveItem() { + if (this.disposer != null) return; + this.disposer = observe(this.menu, 'activeItemIdx', change => { + this.updateMarkOnMenu(change.newValue as number); + }); + } + /** * serializes store * **SUPER HACKY AND NOT OPTIMAL IMPLEMENTATION** diff --git a/src/services/MenuStore.ts b/src/services/MenuStore.ts index 34fb5e083d..06b7893c52 100644 --- a/src/services/MenuStore.ts +++ b/src/services/MenuStore.ts @@ -46,8 +46,8 @@ export class MenuStore { /** * cached flattened menu items to support absolute indexing */ - private _unsubscribe: () => void; - private _hashUnsubscribe: () => void; + private _unsubscribe?: () => void; + private _hashUnsubscribe?: () => void; /** * @@ -61,13 +61,18 @@ export class MenuStore { this.flatItems = flattenByProp(this.items || [], 'items'); this.flatItems.forEach((item, idx) => (item.absoluteIdx = idx)); - - this.subscribe(); } + /** + * Subscribes to scroll and history updates. Called when the store is mounted. + */ subscribe() { - this._unsubscribe = this.scroll.subscribe(this.updateOnScroll); - this._hashUnsubscribe = this.history.subscribe(this.updateOnHistory); + if (!this._unsubscribe) { + this._unsubscribe = this.scroll.subscribe(this.updateOnScroll); + } + if (!this._hashUnsubscribe) { + this._hashUnsubscribe = this.history.subscribe(this.updateOnHistory); + } } @action @@ -252,7 +257,9 @@ export class MenuStore { } dispose() { - this._unsubscribe(); - this._hashUnsubscribe(); + this._unsubscribe?.(); + this._hashUnsubscribe?.(); + this._unsubscribe = undefined; + this._hashUnsubscribe = undefined; } } diff --git a/src/services/ScrollService.ts b/src/services/ScrollService.ts index 54a5a781a6..2426fd2c90 100644 --- a/src/services/ScrollService.ts +++ b/src/services/ScrollService.ts @@ -10,13 +10,19 @@ export class ScrollService { private _scrollParent: Window | HTMLElement | undefined; private _emiter: EventEmitter; private _prevOffsetY = 0; + private _bound = false; constructor(private options: RedocNormalizedOptions) { this._scrollParent = IS_BROWSER ? window : undefined; this._emiter = new EventEmitter(); - this.bind(); } + /** + * Starts listening to scroll events. Called when the store is mounted, so a store + * that is never mounted does not leave a listener behind. + */ bind() { + if (this._bound) return; + this._bound = true; this._prevOffsetY = this.scrollY(); if (this._scrollParent) { this._scrollParent.addEventListener('scroll', this.handleScroll); @@ -24,6 +30,7 @@ export class ScrollService { } dispose() { + this._bound = false; if (this._scrollParent) { this._scrollParent.removeEventListener('scroll', this.handleScroll); } From 2f539352e959d36ad2cc89c8a04bc3866defc5b2 Mon Sep 17 00:00:00 2001 From: Roman Claudiu Date: Tue, 10 Nov 2026 14:41:32 +0200 Subject: [PATCH 2/2] fix: only detach the previous store when Redoc receives a new one --- src/components/Redoc/Redoc.tsx | 3 ++- src/components/__tests__/Redoc.test.tsx | 24 +++++++++++++++++++++++- src/services/AppStore.ts | 16 ++++++++++++---- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/components/Redoc/Redoc.tsx b/src/components/Redoc/Redoc.tsx index b90ac3bec5..97040f6062 100644 --- a/src/components/Redoc/Redoc.tsx +++ b/src/components/Redoc/Redoc.tsx @@ -30,7 +30,8 @@ export class Redoc extends React.Component { componentDidUpdate(prevProps: RedocProps) { if (prevProps.store !== this.props.store) { - prevProps.store.dispose(); + // only detach: the previous store may be passed in again, so its search worker must keep running + prevProps.store.detach(); this.props.store.onDidMount(); } } diff --git a/src/components/__tests__/Redoc.test.tsx b/src/components/__tests__/Redoc.test.tsx index 6b845dae07..de14082840 100644 --- a/src/components/__tests__/Redoc.test.tsx +++ b/src/components/__tests__/Redoc.test.tsx @@ -98,7 +98,7 @@ describe('Redoc scroll listeners', () => { expect(scrollListeners.size).toBe(0); }); - test('Redoc disposes the previous store when it receives a new one', () => { + test('Redoc detaches the previous store when it receives a new one', () => { const first = new AppStore(spec as any, undefined, options); const second = new AppStore(spec as any, undefined, options); @@ -113,4 +113,26 @@ describe('Redoc scroll listeners', () => { expect(scrollListeners.size).toBe(0); }); + + test('Redoc keeps the search worker of a store that is passed in again', () => { + const first = new AppStore(spec as any); + const second = new AppStore(spec as any); + // the in-process worker used by jest has no terminate() + const terminateFirst = jest.fn(); + (first.search!.searchWorker as any).terminate = terminateFirst; + (second.search!.searchWorker as any).terminate = jest.fn(); + + act(() => root.render()); + act(() => root.render()); + act(() => root.render()); + + expect(terminateFirst).not.toHaveBeenCalled(); + expect(scrollSubscribers(first)).toBe(1); + expect(scrollSubscribers(second)).toBe(0); + + act(() => root.unmount()); + + expect(terminateFirst).toHaveBeenCalledTimes(1); + expect(scrollListeners.size).toBe(0); + }); }); diff --git a/src/services/AppStore.ts b/src/services/AppStore.ts index a3b8609540..37e8f2241f 100644 --- a/src/services/AppStore.ts +++ b/src/services/AppStore.ts @@ -96,18 +96,26 @@ export class AppStore { this.updateMarkOnMenu(this.menu.activeItemIdx); } - dispose() { + /** + * Removes the scroll, history and menu listeners attached by `onDidMount`. + * The store stays usable and can be mounted again; the search worker keeps running. + */ + detach() { this.scroll.dispose(); this.menu.dispose(); - if (this.search) { - this.search.dispose(); - } if (this.disposer != null) { this.disposer(); this.disposer = null; } } + dispose() { + this.detach(); + if (this.search) { + this.search.dispose(); + } + } + private observeActiveItem() { if (this.disposer != null) return; this.disposer = observe(this.menu, 'activeItemIdx', change => {