handle multiple copies of singletons with appropriate warnings

gnoff · gnoff · commit c3ed9358d507 · 2022-10-07T17:55:46.000-07:00
diff --git a/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js b/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js
@@ -25,7 +25,8 @@ import {
   getInstanceFromNode as getInstanceFromNodeDOMTree,
   isContainerMarkedAsRoot,
 } from './ReactDOMComponentTree';
-export {detachDeletedInstance} from './ReactDOMComponentTree';
+import {detachDeletedInstance} from './ReactDOMComponentTree';
+export {detachDeletedInstance};
 import {hasRole} from './DOMAccessibilityRoles';
 import {
   createElement,
@@ -710,43 +711,55 @@ export function unhideTextInstance(
 
 export function clearContainer(container: Container): void {
   if (enableHostSingletons) {
-    if (container.nodeType === ELEMENT_NODE) {
-      // We have refined the container to Element type
-      const element: Element = (container: any);
-      switch (element.tagName) {
+    // We have refined the container to Element type
+    const nodeType = container.nodeType;
+    if (nodeType === DOCUMENT_NODE || nodeType === ELEMENT_NODE) {
+      switch (container.nodeName) {
+        case '#document':
         case 'HTML':
         case 'HEAD':
         case 'BODY': {
-          let node = element.firstChild;
+          let node = container.firstChild;
           while (node) {
             const nextNode = node.nextSibling;
             const nodeName = node.nodeName;
-            if (getInstanceFromNodeDOMTree(node)) {
-              // retain nodes owned by React
-            } else if (
-              nodeName === 'HEAD' ||
-              nodeName === 'BODY' ||
-              nodeName === 'STYLE' ||
-              (nodeName === 'LINK' &&
-                ((node: any): HTMLLinkElement).rel.toLowerCase() ===
-                  'stylesheet')
-            ) {
-              // retain these nodes
-            } else {
-              element.removeChild(node);
+            switch (nodeName) {
+              case 'HTML':
+              case 'HEAD':
+              case 'BODY': {
+                clearContainer((node: any));
+                // If these singleton instances had previously been rendered with React they
+                // may still hold on to references to the previous fiber tree. We detatch them
+                // prospectiveyl to reset them to a baseline starting state since we cannot create
+                // new instances.
+                detachDeletedInstance((node: any));
+                break;
+              }
+              case 'STYLE': {
+                break;
+              }
+              case 'LINK': {
+                if (
+                  ((node: any): HTMLLinkElement).rel.toLowerCase() ===
+                  'stylesheet'
+                ) {
+                  break;
+                }
+              }
+              // eslint-disable-next-line no-fallthrough
+              default: {
+                container.removeChild(node);
+              }
             }
             node = nextNode;
           }
           return;
         }
         default: {
-          element.textContent = '';
+          container.textContent = '';
         }
       }
     }
-    // Implicitly if the container is of type Document we rely on the HostSingleton
-    // semantics to clear these nodes appropriately when being placed so no ahead of time
-    // clearing is necessary
   } else {
     if (container.nodeType === ELEMENT_NODE) {
       // We have refined the container to Element type
@@ -762,6 +775,19 @@ export function clearContainer(container: Container): void {
   }
 }
 
+// Making this so we can eventually move all of the instance caching to the commit phase.
+// Currently this is only used to associate fiber and props to instances for hydrating
+// HostSingletons. The reason we need it here is we only want to make this binding on commit
+// because only one fiber can own the instance at a time and render can fail/restart
+export function bindInstance(
+  instance: Instance,
+  props: Props,
+  internalInstanceHandle: mixed,
+) {
+  precacheFiberNode((internalInstanceHandle: any), instance);
+  updateFiberProps(instance, props);
+}
+
 // -------------------
 //     Hydration
 // -------------------
@@ -1551,7 +1577,7 @@ export function acquireSingletonInstance(
         'You are mounting a new %s component when a previous one has not first unmounted. It is an' +
           ' error to render more than one %s component at a time and attributes and children of these' +
           ' components will likely fail in unpredictable ways. Please only render a single instance of' +
-          ' <%s> and if you need to mount a new one, ensure any previous ones have unmounted first',
+          ' <%s> and if you need to mount a new one, ensure any previous ones have unmounted first.',
         tagName,
         tagName,
         tagName,
@@ -1586,4 +1612,29 @@ export function releaseSingletonInstance(instance: Instance): void {
   while (attributes.length) {
     instance.removeAttributeNode(attributes[0]);
   }
+  detachDeletedInstance(instance);
+}
+
+export function clearSingleton(instance: Instance): void {
+  const element: Element = (instance: any);
+  let node = element.firstChild;
+  while (node) {
+    const nextNode = node.nextSibling;
+    const nodeName = node.nodeName;
+    if (getInstanceFromNodeDOMTree(node)) {
+      // retain nodes owned by React
+    } else if (
+      nodeName === 'HEAD' ||
+      nodeName === 'BODY' ||
+      nodeName === 'STYLE' ||
+      (nodeName === 'LINK' &&
+        ((node: any): HTMLLinkElement).rel.toLowerCase() === 'stylesheet')
+    ) {
+      // retain these nodes
+    } else {
+      element.removeChild(node);
+    }
+    node = nextNode;
+  }
+  return;
 }
diff --git a/packages/react-dom/src/__tests__/validateDOMNesting-test.js b/packages/react-dom/src/__tests__/validateDOMNesting-test.js
@@ -102,22 +102,49 @@ describe('validateDOMNesting', () => {
           '    in html (at **)',
       ],
     );
-    expectWarnings(
-      ['body', 'body'],
-      [
-        'render(): Rendering components directly into document.body is discouraged',
-        'validateDOMNesting(...): <body> cannot appear as a child of <body>.\n' +
-          '    in body (at **)',
-      ],
-      1,
-    );
-    expectWarnings(
-      ['svg', 'foreignObject', 'body', 'p'],
-      [
-        'validateDOMNesting(...): <body> cannot appear as a child of <foreignObject>.\n' +
-          '    in body (at **)\n' +
-          '    in foreignObject (at **)',
-      ],
-    );
+    if (gate(flags => flags.enableHostSingletons)) {
+      expectWarnings(
+        ['body', 'body'],
+        [
+          'render(): Rendering components directly into document.body is discouraged',
+          'validateDOMNesting(...): <body> cannot appear as a child of <body>.\n' +
+            '    in body (at **)',
+          'Warning: You are mounting a new body component when a previous one has not first unmounted. It is an error to render more than one body component at a time and attributes and children of these components will likely fail in unpredictable ways. Please only render a single instance of <body> and if you need to mount a new one, ensure any previous ones have unmounted first.\n' +
+            '    in body (at **)',
+        ],
+        1,
+      );
+    } else {
+      expectWarnings(
+        ['body', 'body'],
+        [
+          'render(): Rendering components directly into document.body is discouraged',
+          'validateDOMNesting(...): <body> cannot appear as a child of <body>.\n' +
+            '    in body (at **)',
+        ],
+        1,
+      );
+    }
+    if (gate(flags => flags.enableHostSingletons)) {
+      expectWarnings(
+        ['svg', 'foreignObject', 'body', 'p'],
+        [
+          'validateDOMNesting(...): <body> cannot appear as a child of <foreignObject>.\n' +
+            '    in body (at **)\n' +
+            '    in foreignObject (at **)',
+          'Warning: You are mounting a new body component when a previous one has not first unmounted. It is an error to render more than one body component at a time and attributes and children of these components will likely fail in unpredictable ways. Please only render a single instance of <body> and if you need to mount a new one, ensure any previous ones have unmounted first.\n' +
+            '    in body (at **)',
+        ],
+      );
+    } else {
+      expectWarnings(
+        ['svg', 'foreignObject', 'body', 'p'],
+        [
+          'validateDOMNesting(...): <body> cannot appear as a child of <foreignObject>.\n' +
+            '    in body (at **)\n' +
+            '    in foreignObject (at **)',
+        ],
+      );
+    }
   });
 });
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -150,6 +150,7 @@ import {
   detachDeletedInstance,
   acquireResource,
   releaseResource,
+  clearSingleton,
   acquireSingletonInstance,
   releaseSingletonInstance,
 } from './ReactFiberHostConfig';
@@ -2096,7 +2097,6 @@ function commitDeletionEffectsOnFiber(
         // a different fiber. To increase our chances of avoiding this, specifically
         // if you keyed a HostSingleton so there will be a delete followed by a Placement
         // we treat detach eagerly here
-        detachDeletedInstance(deletedFiber.stateNode);
         releaseSingletonInstance(deletedFiber.stateNode);
 
         hostParent = prevHostParent;
@@ -2624,10 +2624,12 @@ function commitMutationEffectsOnFiber(
           const previousWork = finishedWork.alternate;
           if (previousWork === null) {
             const singleton = finishedWork.stateNode;
-            clearContainer(singleton);
+            const props = finishedWork.memoizedProps;
+            // This was a new mount, we need to clear and set initial properties
+            clearSingleton(singleton);
             acquireSingletonInstance(
               finishedWork.type,
-              finishedWork.memoizedProps,
+              props,
               singleton,
               finishedWork,
             );
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
@@ -150,6 +150,7 @@ import {
   detachDeletedInstance,
   acquireResource,
   releaseResource,
+  clearSingleton,
   acquireSingletonInstance,
   releaseSingletonInstance,
 } from './ReactFiberHostConfig';
@@ -2096,7 +2097,6 @@ function commitDeletionEffectsOnFiber(
         // a different fiber. To increase our chances of avoiding this, specifically
         // if you keyed a HostSingleton so there will be a delete followed by a Placement
         // we treat detach eagerly here
-        detachDeletedInstance(deletedFiber.stateNode);
         releaseSingletonInstance(deletedFiber.stateNode);
 
         hostParent = prevHostParent;
@@ -2624,10 +2624,12 @@ function commitMutationEffectsOnFiber(
           const previousWork = finishedWork.alternate;
           if (previousWork === null) {
             const singleton = finishedWork.stateNode;
-            clearContainer(singleton);
+            const props = finishedWork.memoizedProps;
+            // This was a new mount, we need to clear and set initial properties
+            clearSingleton(singleton);
             acquireSingletonInstance(
               finishedWork.type,
-              finishedWork.memoizedProps,
+              props,
               singleton,
               finishedWork,
             );
diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js
@@ -1022,9 +1022,9 @@ function completeWork(
           const currentHostContext = getHostContext();
           const wasHydrated = popHydrationState(workInProgress);
           if (wasHydrated) {
-            // This method returns an updateQueue. we intentionally ignore the queue
-            // because HostSingletons mount with child fibers even if they have a single
-            // Text only child and we do not update attributes on hydrated nodes
+            // We ignore the boolean indicating there is an updateQueue because
+            // it is used only to set text children and HostSingletons do not
+            // use them.
             prepareToHydrateHostInstance(workInProgress, currentHostContext);
           } else {
             workInProgress.stateNode = resolveSingletonInstance(
diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js
@@ -1022,9 +1022,9 @@ function completeWork(
           const currentHostContext = getHostContext();
           const wasHydrated = popHydrationState(workInProgress);
           if (wasHydrated) {
-            // This method returns an updateQueue. we intentionally ignore the queue
-            // because HostSingletons mount with child fibers even if they have a single
-            // Text only child and we do not update attributes on hydrated nodes
+            // We ignore the boolean indicating there is an updateQueue because
+            // it is used only to set text children and HostSingletons do not
+            // use them.
             prepareToHydrateHostInstance(workInProgress, currentHostContext);
           } else {
             workInProgress.stateNode = resolveSingletonInstance(
