From 92238b79edf3c18de19bd3dcf2e44f435278aee0 Mon Sep 17 00:00:00 2001 From: Arthurk12 Date: Tue, 1 Sep 2026 20:56:35 -0300 Subject: [PATCH] fix(sidekick-area): avoid rendering into an unmounted root on re-registration Changing the client's display language while the "Pick random user" sidekick panel is open crashes the client with "Cannot update an unmounted root.", thrown from React's ReactDOMRoot.render (issue #135). contentFunction caches the {element, root} pair it creates and reuses the cached root whenever the client calls it again for the same container, to avoid detaching the previously rendered tree. However, the client also owns that root's lifecycle: it can unmount the root it was handed and then invoke contentFunction again for the same container, which happens while processing the re-registration this plugin triggers whenever its effect reruns for a new intl (i.e. a language switch). The cache had no way to notice the root had already been unmounted, so it kept calling render on a dead root. Wrap the created root so unmount() also clears the cached entry, forcing the next contentFunction call for that element to create a fresh root instead of reusing the stale one. --- .../component.tsx | 28 +++++++-- .../unit/sidekick-area-registration.test.tsx | 61 ++++++++++++++++++- 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/src/components/extensible-areas/generic-content-sidekick-area/component.tsx b/src/components/extensible-areas/generic-content-sidekick-area/component.tsx index 78beb7f..3adbbbb 100644 --- a/src/components/extensible-areas/generic-content-sidekick-area/component.tsx +++ b/src/components/extensible-areas/generic-content-sidekick-area/component.tsx @@ -42,9 +42,17 @@ function GenericContentSidekickAreaManager( const genericContentId = useRef(''); // The client may call contentFunction more than once for the same container (for - // instance when the item is re-registered). Calling createRoot twice on one element - // detaches the previously rendered tree and leaves the panel blank, so the root is - // kept and reused for as long as the container is the same node. + // instance when the item is re-registered, which happens whenever this effect below + // reruns for a new intl - e.g. a language change). Calling createRoot twice on one + // element detaches the previously rendered tree and leaves the panel blank, so the root + // is kept and reused for as long as the container is the same node. + // + // The client also owns the returned root's lifecycle: while the panel is open, a + // re-registration can make it unmount the root it was previously handed for this same + // container before calling contentFunction again. That unmount happens from outside this + // closure, so the cached entry below is wrapped to notice it and drop itself - otherwise + // `render` is called on an already-unmounted root and the client crashes with + // "Cannot update an unmounted root." (issue #135). const panelRoot = useRef<{ element: HTMLElement; root: ReactDOM.Root } | null>(null); const sidekickAreaName = intl.formatMessage(intlMessages.sidekickAreaTitle); @@ -57,7 +65,19 @@ function GenericContentSidekickAreaManager( new GenericContentSidekickArea({ contentFunction: (element: HTMLElement) => { if (!panelRoot.current || panelRoot.current.element !== element) { - panelRoot.current = { element, root: ReactDOM.createRoot(element) }; + const root = ReactDOM.createRoot(element); + panelRoot.current = { + element, + root: { + render: (node) => root.render(node), + unmount: () => { + root.unmount(); + if (panelRoot.current?.element === element) { + panelRoot.current = null; + } + }, + }, + }; } panelRoot.current.root.render( { }); }); +// Regression test for https://github.com/bigbluebutton/bbb-plugin-pick-random-user/issues/135: +// switching the client's display language while the sidekick panel is open crashed the +// client with "Cannot update an unmounted root.". The client re-registers the item's +// contentFunction whenever this plugin's effect re-runs for a new intl/locale, and it may +// unmount the root it was previously handed before calling contentFunction again for the +// very same container — that sequence is what the reused id from the second test above +// makes possible. +function makePluginApiForPanel(setGenericContentItems: ReturnType) { + return { + setGenericContentItems, + useCurrentUser: () => ({ data: { userId: 'presenter-1', presenter: true } }), + useDataChannel: () => ({ + data: { loading: false, data: [] }, + pushEntry: vi.fn(), + deleteEntry: vi.fn(), + }), + } as never; +} + +describe('sidekick area contentFunction (issue #135)', () => { + it('recovers instead of crashing when the client re-invokes contentFunction on a container whose root it already unmounted', () => { + const setGenericContentItems = vi.fn(() => ['generated-id-1']); + const pluginApi = makePluginApiForPanel(setGenericContentItems); + + render( + , + ); + + const { contentFunction } = setGenericContentItems.mock.calls[0][0][0]; + const container = document.createElement('div'); + document.body.appendChild(container); + + let firstRoot: { unmount: () => void }; + act(() => { + firstRoot = contentFunction(container); + }); + + // The client tears down the root it was handed for the previously registered content + // (e.g. while processing a re-registration triggered by a locale change) but keeps + // reusing the same container element. + act(() => { + firstRoot.unmount(); + }); + + expect(() => act(() => { + contentFunction(container); + })).not.toThrow(); + + expect(container.querySelector('[data-test="pickRandomUserPanel"]')).not.toBeNull(); + }); +}); + describe('useGetInternationalization', () => { it('keeps the same intl instance across renders', () => { const messages = { 'pickRandomUserPlugin.modal.title': 'Pick random user' };