@samitouri / QOS-React-2 / commits / b685b40d87

[Fiber] only remove properties from singletons on release (#37112)

Removing attributes does not actually reset all property state on the singleton instance. It also has the side effect of wiping any 3rd party set attributes and properties that 3rd party scripts and extensions. Reimplements the singleton release to clear properties on the instance based on the last committed props for the singleton fiber. Notably there is an unfixed path with preamble contribution markers that still just clears based on attributes which covers SSR'd attributes that need to be wiped before client recovery on hydration can continue. There are gated failing tests for this as a TODO.

Josh Story committed Jul 24, 2026 at 14:38 UTC b685b40d870b90a975da28c8d22ecf0ba910b1a1
5 files changed +344 -9
packages/react-dom-bindings/src/client/ReactDOMComponent.js
+28 -2
@@ -379,12 +379,18 @@ export function trapClickOnNonInteractiveElement(node: HTMLElement) {
379 // listener on the target node.
380 // https://www.quirksmode.org/blog/archives/2010/09/click_event_del.html
381 // Just set it using the onclick property so that we don't have to manage any
382 - // bookkeeping for it. Not sure if we need to clear it when the listener is
383 - // removed.
382 + // bookkeeping for it. HostSingleton release clears the property only if it
383 + // still points to this noop.
384 // TODO: Only do this for the relevant Safaris maybe?
385 node.onclick = noop;
386 }
387
388 +export function clearClickListener(node: HTMLElement) {
389 + if (node.onclick === noop) {
390 + node.onclick = null;
391 + }
392 +}
393 +
394 const xlinkNamespace = 'http://www.w3.org/1999/xlink';
395 const xmlNamespace = 'http://www.w3.org/XML/1998/namespace';
396
@@ -1489,6 +1495,26 @@ export function setInitialProperties(
1495 }
1496 }
1497
1498 +export type SingletonType = 'html' | 'head' | 'body';
1499 +
1500 +const emptyProps = {};
1501 +
1502 +export function clearSingletonProperties(
1503 + domElement: Element,
1504 + tag: SingletonType,
1505 + props: Object,
1506 +): void {
1507 + // This is equivalent to updating to empty props for tags without
1508 + // tag-specific update logic. Host singletons are limited to html, head, and
1509 + // body, so they always use this generic path.
1510 + for (const propKey in props) {
1511 + const propValue = props[propKey];
1512 + if (props.hasOwnProperty(propKey) && propValue != null) {
1513 + setProp(domElement, tag, propKey, null, emptyProps, propValue);
1514 + }
1515 + }
1516 +}
1517 +
1518 export function updateProperties(
1519 domElement: Element,
1520 tag: string,
packages/react-dom-bindings/src/client/ReactDOMComponentTree.js
+3 -2
@@ -69,10 +69,12 @@ const internalPropsMap:
69 | Map<InstanceUnion, Props> = new PossiblyWeakMap();
70
71 export function detachDeletedInstance(node: Instance): void {
72 + // Don't delete the event listener set. The native event listeners it tracks
73 + // remain attached to the node, so this bookkeeping needs to last for the
74 + // lifetime of the node to prevent duplicate listeners if it is reused.
75 if (enableInternalInstanceMap) {
76 internalInstanceMap.delete(node);
77 internalPropsMap.delete(node);
75 - delete (node as any)[internalEventHandlersKey];
78 delete (node as any)[internalEventHandlerListenersKey];
79 delete (node as any)[internalEventHandlesSetKey];
80 delete (node as any)[internalRootNodeResourcesKey];
@@ -85,7 +87,6 @@ export function detachDeletedInstance(node: Instance): void {
87 // these fields are relevant.
88 delete (node as any)[internalInstanceKey];
89 delete (node as any)[internalPropsKey];
88 - delete (node as any)[internalEventHandlersKey];
90 delete (node as any)[internalEventHandlerListenersKey];
91 delete (node as any)[internalEventHandlesSetKey];
92 }
packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
+40 -4
@@ -77,15 +77,18 @@ import {compareDocumentPositionForEmptyFragment} from 'shared/ReactDOMFragmentRe
77
78 export {detachDeletedInstance};
79 import {hasRole} from './DOMAccessibilityRoles';
80 +import type {SingletonType} from './ReactDOMComponent';
81 import {
82 setInitialProperties,
83 updateProperties,
84 + clearSingletonProperties,
85 hydrateProperties,
86 hydrateText,
87 diffHydratedProperties,
88 getPropsFromElement,
89 diffHydratedText,
90 trapClickOnNonInteractiveElement,
91 + clearClickListener,
92 } from './ReactDOMComponent';
93 import {hydrateInput} from './ReactDOMInput';
94 import {hydrateTextarea} from './ReactDOMTextarea';
@@ -1269,18 +1272,18 @@ function clearHydrationBoundary(
1272 // then it contributed to the html tag and we need to reset it.
1273 const ownerDocument = parentInstance.ownerDocument;
1274 const documentElement: Element = ownerDocument.documentElement as any;
1272 - releaseSingletonInstance(documentElement);
1275 + clearSingletonPreambleContribution(documentElement);
1276 } else if (data === PREAMBLE_CONTRIBUTION_HEAD) {
1277 const ownerDocument = parentInstance.ownerDocument;
1278 const head: Element = ownerDocument.head as any;
1276 - releaseSingletonInstance(head);
1279 + clearSingletonPreambleContribution(head);
1280 // We need to clear the head because this is the only singleton that can have children that
1281 // were part of this boundary but are not inside this boundary.
1282 clearHead(head);
1283 } else if (data === PREAMBLE_CONTRIBUTION_BODY) {
1284 const ownerDocument = parentInstance.ownerDocument;
1285 const body: Element = ownerDocument.body as any;
1283 - releaseSingletonInstance(body);
1286 + clearSingletonPreambleContribution(body);
1287 }
1288 }
1289 // $FlowFixMe[incompatible-type] we bail out when we get a null
@@ -4844,7 +4847,40 @@ export function acquireSingletonInstance(
4847 updateFiberProps(instance, props);
4848 }
4849
4847 -export function releaseSingletonInstance(instance: Instance): void {
4850 +export function releaseSingletonInstance(
4851 + instance: Instance,
4852 + type: SingletonType,
4853 + props: Props,
4854 +): void {
4855 + // Remove the attributes and property-backed state owned by this Fiber.
4856 + clearSingletonProperties(instance, type, props);
4857 +
4858 + // These properties aren't cleared by updateProperties when their next
4859 + // value is null. Normally that is handled by replacing/removing the host
4860 + // instance, but a singleton cannot be removed.
4861 + // TODO: HostSingleton updates do not currently schedule ContentReset when
4862 + // dangerouslySetInnerHTML becomes undefined, so an ordinary update can leave
4863 + // the previous HTML in place. This only handles the release path.
4864 + if (props.dangerouslySetInnerHTML != null) {
4865 + instance.textContent = '';
4866 + }
4867 + clearClickListener(instance as any as HTMLElement);
4868 +
4869 + // Only remove state that was represented by this Fiber's props. Attributes
4870 + // added imperatively while React owned the singleton must be preserved.
4871 + detachDeletedInstance(instance);
4872 +}
4873 +
4874 +function clearSingletonPreambleContribution(instance: Instance): void {
4875 + // This path is only used when clearing a dehydrated boundary that contains a
4876 + // Fizz preamble contribution marker. The marker tells us which singleton the
4877 + // boundary contributed to, but it does not include the contributed props and
4878 + // there is no HostSingleton Fiber to provide them. We therefore cannot tell
4879 + // which attributes came from React and which were added imperatively by a
4880 + // script or third party. For now, clearing every attribute is an accepted
4881 + // edge case.
4882 + // TODO: Include the contributed properties in the marker so this cleanup can
4883 + // remove only the attributes owned by the boundary.
4884 const attributes = instance.attributes;
4885 while (attributes.length) {
4886 instance.removeAttributeNode(attributes[0]);
packages/react-dom/src/__tests__/ReactDOMSingletonComponents-test.js
+266
@@ -225,6 +225,272 @@ describe('ReactDOM HostSingleton', () => {
225 );
226 });
227
228 + it('resets property-backed state when a singleton is released', async () => {
229 + const root = ReactDOMClient.createRoot(document);
230 + const head = document.head;
231 + const body = document.body;
232 +
233 + root.render(
234 + <html>
235 + <head onClick={() => {}} />
236 + <body
237 + data-react-owned="true"
238 + onClick={() => {}}
239 + style={{color: 'red'}}
240 + dangerouslySetInnerHTML={{__html: '<div>managed content</div>'}}
241 + />
242 + </html>,
243 + );
244 + await waitForAll([]);
245 +
246 + expect(document.body).toBe(body);
247 + expect(head.onclick).not.toBe(null);
248 + expect(body.onclick).not.toBe(null);
249 + expect(body.textContent).toBe('managed content');
250 + expect(body.getAttribute('data-react-owned')).toBe('true');
251 + expect(body.style.color).toBe('red');
252 +
253 + // Simulate an inline script or third-party code adding its own attribute,
254 + // style, and click listener while React owns the singleton.
255 + const externalClickHandler = jest.fn();
256 + body.setAttribute('data-external', 'true');
257 + body.style.backgroundColor = 'blue';
258 + body.onclick = externalClickHandler;
259 +
260 + root.render(<html />);
261 + await waitForAll([]);
262 +
263 + expect(document.head).toBe(head);
264 + expect(document.body).toBe(body);
265 + expect(head.onclick).toBe(null);
266 + expect(body.onclick).toBe(externalClickHandler);
267 + expect(body.textContent).toBe('');
268 + expect(body.hasAttribute('data-react-owned')).toBe(false);
269 + expect(body.getAttribute('data-external')).toBe('true');
270 + expect(body.style.color).toBe('');
271 + expect(body.style.backgroundColor).toBe('blue');
272 + });
273 +
274 + // @gate TODO
275 + it('clears dangerouslySetInnerHTML when it becomes undefined', async () => {
276 + const root = ReactDOMClient.createRoot(document);
277 + const body = document.body;
278 + const undefinedHTML = undefined;
279 +
280 + root.render(
281 + <html>
282 + <head />
283 + <body
284 + dangerouslySetInnerHTML={{__html: '<div>managed content</div>'}}
285 + />
286 + </html>,
287 + );
288 + await waitForAll([]);
289 + expect(body.textContent).toBe('managed content');
290 +
291 + root.render(
292 + <html>
293 + <head />
294 + <body dangerouslySetInnerHTML={undefinedHTML} />
295 + </html>,
296 + );
297 + await waitForAll([]);
298 +
299 + expect(body.textContent).toBe('');
300 + });
301 +
302 + // @gate TODO
303 + it('clears dangerouslySetInnerHTML when __html becomes undefined', async () => {
304 + const root = ReactDOMClient.createRoot(document);
305 + const body = document.body;
306 +
307 + root.render(
308 + <html>
309 + <head />
310 + <body
311 + dangerouslySetInnerHTML={{__html: '<div>managed content</div>'}}
312 + />
313 + </html>,
314 + );
315 + await waitForAll([]);
316 + expect(body.textContent).toBe('managed content');
317 +
318 + root.render(
319 + <html>
320 + <head />
321 + <body dangerouslySetInnerHTML={{__html: undefined}} />
322 + </html>,
323 + );
324 + await waitForAll([]);
325 +
326 + expect(body.textContent).toBe('');
327 + });
328 +
329 + it('updates dangerouslySetInnerHTML on a singleton', async () => {
330 + const root = ReactDOMClient.createRoot(document);
331 + const body = document.body;
332 +
333 + root.render(
334 + <html>
335 + <head />
336 + <body dangerouslySetInnerHTML={{__html: '<div>first</div>'}} />
337 + </html>,
338 + );
339 + await waitForAll([]);
340 + expect(body.innerHTML).toBe('<div>first</div>');
341 +
342 + root.render(
343 + <html>
344 + <head />
345 + <body dangerouslySetInnerHTML={{__html: '<span>second</span>'}} />
346 + </html>,
347 + );
348 + await waitForAll([]);
349 +
350 + expect(body.innerHTML).toBe('<span>second</span>');
351 + });
352 +
353 + it('replaces singleton children with dangerouslySetInnerHTML', async () => {
354 + const root = ReactDOMClient.createRoot(document);
355 + const body = document.body;
356 +
357 + root.render(
358 + <html>
359 + <head />
360 + <body>
361 + <div>managed child</div>
362 + </body>
363 + </html>,
364 + );
365 + await waitForAll([]);
366 + expect(body.innerHTML).toBe('<div>managed child</div>');
367 +
368 + root.render(
369 + <html>
370 + <head />
371 + <body dangerouslySetInnerHTML={{__html: '<span>managed HTML</span>'}} />
372 + </html>,
373 + );
374 + await waitForAll([]);
375 +
376 + expect(body.innerHTML).toBe('<span>managed HTML</span>');
377 + });
378 +
379 + // @gate TODO
380 + it('replaces dangerouslySetInnerHTML with singleton children', async () => {
381 + const root = ReactDOMClient.createRoot(document);
382 + const body = document.body;
383 +
384 + root.render(
385 + <html>
386 + <head />
387 + <body
388 + dangerouslySetInnerHTML={{__html: '<div>managed content</div>'}}
389 + />
390 + </html>,
391 + );
392 + await waitForAll([]);
393 + expect(body.innerHTML).toBe('<div>managed content</div>');
394 +
395 + root.render(
396 + <html>
397 + <head />
398 + <body>
399 + <span>managed child</span>
400 + </body>
401 + </html>,
402 + );
403 + await waitForAll([]);
404 +
405 + expect(body.innerHTML).toBe('<span>managed child</span>');
406 + });
407 +
408 + // @gate TODO
409 + it('preserves imperative attributes when acquiring a singleton', async () => {
410 + const body = document.body;
411 + body.setAttribute('data-external', 'true');
412 +
413 + const root = ReactDOMClient.createRoot(document);
414 + root.render(
415 + <html>
416 + <head />
417 + <body />
418 + </html>,
419 + );
420 + await waitForAll([]);
421 +
422 + expect(document.body).toBe(body);
423 + expect(body.getAttribute('data-external')).toBe('true');
424 + });
425 +
426 + // @gate TODO
427 + it('preserves imperative attributes when clearing a preamble contribution', async () => {
428 + const body = document.body;
429 + body.setAttribute('data-react-owned', 'true');
430 + body.setAttribute('data-external', 'true');
431 + // This is the shape Fizz emits when a completed Suspense boundary
432 + // contributes props to the body singleton.
433 + body.innerHTML = '<!--$--><!--body--><div>server</div><!--/$-->';
434 +
435 + ReactDOMClient.hydrateRoot(
436 + document,
437 + <html>
438 + <head />
439 + <body data-react-owned="true" suppressHydrationWarning={true}>
440 + <React.Suspense fallback={null}>
441 + <span>client</span>
442 + </React.Suspense>
443 + </body>
444 + </html>,
445 + {
446 + onRecoverableError() {},
447 + },
448 + );
449 + await waitForAll([]);
450 +
451 + expect(body.textContent).toBe('client');
452 + expect(body.getAttribute('data-external')).toBe('true');
453 + });
454 +
455 + it('does not duplicate native listeners when a singleton is reacquired', async () => {
456 + const root = ReactDOMClient.createRoot(document);
457 + const body = document.body;
458 + const onScroll = jest.fn();
459 +
460 + root.render(
461 + <html>
462 + <head />
463 + <body onScroll={onScroll} />
464 + </html>,
465 + );
466 + await waitForAll([]);
467 +
468 + body.dispatchEvent(new document.defaultView.Event('scroll'));
469 + expect(onScroll).toHaveBeenCalledTimes(1);
470 +
471 + root.render(
472 + <html>
473 + <head />
474 + </html>,
475 + );
476 + await waitForAll([]);
477 +
478 + body.dispatchEvent(new document.defaultView.Event('scroll'));
479 + expect(onScroll).toHaveBeenCalledTimes(1);
480 +
481 + root.render(
482 + <html>
483 + <head />
484 + <body onScroll={onScroll} />
485 + </html>,
486 + );
487 + await waitForAll([]);
488 +
489 + expect(document.body).toBe(body);
490 + body.dispatchEvent(new document.defaultView.Event('scroll'));
491 + expect(onScroll).toHaveBeenCalledTimes(2);
492 + });
493 +
494 it('renders into html, head, and body persistently so the node identities never change and extraneous styles are retained', async () => {
495 // Server render some html that will get replaced with a client render
496 await actIntoEmptyDocument(() => {
packages/react-reconciler/src/ReactFiberCommitHostEffects.js
+7 -1
@@ -826,8 +826,14 @@ export function commitHostSingletonRelease(releasingWork: Fiber) {
826 releasingWork,
827 releaseSingletonInstance,
828 releasingWork.stateNode,
829 + releasingWork.type,
830 + releasingWork.memoizedProps,
831 );
832 } else {
831 - releaseSingletonInstance(releasingWork.stateNode);
833 + releaseSingletonInstance(
834 + releasingWork.stateNode,
835 + releasingWork.type,
836 + releasingWork.memoizedProps,
837 + );
838 }
839 }