add warnings for non-resources rendered outside body or head (#25532)

Adds some clarifying warnings when you render a component that is almost a resource but isn't and the element was rendered outside the main document tree (outside of `<body>` or `<head>`
facebook · Dec 3, 2022 · fbe8d2a · fbe8d2a
1 parent 6b1ace6
commit fbe8d2a
Show file tree

Hide file tree

Showing 6 changed files with 195 additions and 10 deletions.
diff --git a/packages/react-dom-bindings/src/client/ReactDOMFloatClient.js b/packages/react-dom-bindings/src/client/ReactDOMFloatClient.js
@@ -29,7 +29,11 @@ import {
   markNodeAsResource,
 } from './ReactDOMComponentTree';
 import {HTML_NAMESPACE} from '../shared/DOMNamespaces';
-import {getCurrentRootHostContainer} from 'react-reconciler/src/ReactFiberHostContext';
+import {
+  getCurrentRootHostContainer,
+  getHostContext,
+} from 'react-reconciler/src/ReactFiberHostContext';
+import {getResourceFormOnly} from './validateDOMNesting';
 
 // The resource types we support. currently they match the form for the as argument.
 // In the future this may need to change, especially when modules / scripts are supported
@@ -1331,6 +1335,11 @@ function insertResourceInstanceBefore(
 }
 
 export function isHostResourceType(type: string, props: Props): boolean {
+  let resourceFormOnly: boolean;
+  if (__DEV__) {
+    const hostContext = getHostContext();
+    resourceFormOnly = getResourceFormOnly(hostContext);
+  }
   switch (type) {
     case 'meta':
     case 'title': {
@@ -1339,14 +1348,29 @@ export function isHostResourceType(type: string, props: Props): boolean {
     case 'link': {
       const {onLoad, onError} = props;
       if (onLoad || onError) {
+        if (__DEV__) {
+          if (resourceFormOnly) {
+            console.error(
+              'Cannot render a <link> with onLoad or onError listeners outside the main document.' +
+                ' Try removing onLoad={...} and onError={...} or moving it into the root <head> tag or' +
+                ' somewhere in the <body>.',
+            );
+          }
+        }
         return false;
       }
       switch (props.rel) {
         case 'stylesheet': {
+          const {href, precedence, disabled} = props;
           if (__DEV__) {
             validateLinkPropsForStyleResource(props);
+            if (typeof precedence !== 'string' && resourceFormOnly) {
+              console.error(
+                'Cannot render a <link rel="stylesheet" /> outside the main document without knowing its precedence.' +
+                  ' Consider adding precedence="default" or moving it into the root <head> tag.',
+              );
+            }
           }
-          const {href, precedence, disabled} = props;
           return (
             typeof href === 'string' &&
             typeof precedence === 'string' &&
@@ -1363,8 +1387,36 @@ export function isHostResourceType(type: string, props: Props): boolean {
       // We don't validate because it is valid to use async with onLoad/onError unlike combining
       // precedence with these for style resources
       const {src, async, onLoad, onError} = props;
+      if (__DEV__) {
+        if (async !== true && resourceFormOnly) {
+          console.error(
+            'Cannot render a sync or defer <script> outside the main document without knowing its order.' +
+              ' Try adding async="" or moving it into the root <head> tag.',
+          );
+        } else if ((onLoad || onError) && resourceFormOnly) {
+          console.error(
+            'Cannot render a <script> with onLoad or onError listeners outside the main document.' +
+              ' Try removing onLoad={...} and onError={...} or moving it into the root <head> tag or' +
+              ' somewhere in the <body>.',
+          );
+        }
+      }
       return (async: any) && typeof src === 'string' && !onLoad && !onError;
     }
+    case 'base':
+    case 'template':
+    case 'style':
+    case 'noscript': {
+      if (__DEV__) {
+        if (resourceFormOnly) {
+          console.error(
+            'Cannot render <%s> outside the main document. Try moving it into the root <head> tag.',
+            type,
+          );
+        }
+      }
+      return false;
+    }
   }
   return false;
 }

diff --git a/packages/react-dom-bindings/src/client/validateDOMNesting.js b/packages/react-dom-bindings/src/client/validateDOMNesting.js
@@ -7,6 +7,7 @@
 
 let validateDOMNesting = () => {};
 let updatedAncestorInfo = () => {};
+let getResourceFormOnly = () => false;
 
 if (__DEV__) {
   // This validation code was written based on the HTML5 parsing spec:
@@ -153,6 +154,8 @@ if (__DEV__) {
 
     listItemTagAutoclosing: null,
     dlItemTagAutoclosing: null,
+
+    resourceFormOnly: true,
   };
 
   updatedAncestorInfo = function(oldInfo, tag) {
@@ -180,6 +183,10 @@ if (__DEV__) {
       ancestorInfo.dlItemTagAutoclosing = null;
     }
 
+    if (tag !== '#document' && tag !== 'html') {
+      ancestorInfo.resourceFormOnly = false;
+    }
+
     ancestorInfo.current = info;
 
     if (tag === 'form') {
@@ -472,6 +479,10 @@ if (__DEV__) {
       );
     }
   };
+
+  getResourceFormOnly = hostContextDev => {
+    return hostContextDev.ancestorInfo.resourceFormOnly;
+  };
 }
 
-export {updatedAncestorInfo, validateDOMNesting};
+export {updatedAncestorInfo, validateDOMNesting, getResourceFormOnly};
diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js
@@ -268,6 +268,118 @@ describe('ReactDOMFloat', () => {
     );
   });
 
+  function renderSafelyAndExpect(root, children) {
+    root.render(children);
+    return expect(() => {
+      try {
+        expect(Scheduler).toFlushWithoutYielding();
+      } catch (e) {
+        try {
+          expect(Scheduler).toFlushWithoutYielding();
+        } catch (f) {}
+      }
+    });
+  }
+
+  // @gate enableFloat || !__DEV__
+  it('warns if you render resource-like elements above <head> or <body>', async () => {
+    const root = ReactDOMClient.createRoot(document);
+
+    renderSafelyAndExpect(
+      root,
+      <>
+        <noscript>foo</noscript>
+        <html>
+          <body>foo</body>
+        </html>
+      </>,
+    ).toErrorDev(
+      [
+        'Cannot render <noscript> outside the main document. Try moving it into the root <head> tag.',
+        'Warning: validateDOMNesting(...): <noscript> cannot appear as a child of <#document>.',
+      ],
+      {withoutStack: 1},
+    );
+
+    renderSafelyAndExpect(
+      root,
+      <html>
+        <template>foo</template>
+        <body>foo</body>
+      </html>,
+    ).toErrorDev([
+      'Cannot render <template> outside the main document. Try moving it into the root <head> tag.',
+      'Warning: validateDOMNesting(...): <template> cannot appear as a child of <html>.',
+    ]);
+
+    renderSafelyAndExpect(
+      root,
+      <html>
+        <body>foo</body>
+        <style>foo</style>
+      </html>,
+    ).toErrorDev([
+      'Cannot render <style> outside the main document. Try moving it into the root <head> tag.',
+      'Warning: validateDOMNesting(...): <style> cannot appear as a child of <html>.',
+    ]);
+
+    renderSafelyAndExpect(
+      root,
+      <>
+        <html>
+          <body>foo</body>
+        </html>
+        <link rel="stylesheet" href="foo" />
+      </>,
+    ).toErrorDev(
+      [
+        'Cannot render a <link rel="stylesheet" /> outside the main document without knowing its precedence. Consider adding precedence="default" or moving it into the root <head> tag.',
+        'Warning: validateDOMNesting(...): <link> cannot appear as a child of <#document>.',
+      ],
+      {withoutStack: 1},
+    );
+
+    renderSafelyAndExpect(
+      root,
+      <>
+        <html>
+          <body>foo</body>
+          <script href="foo" />
+        </html>
+      </>,
+    ).toErrorDev([
+      'Cannot render a sync or defer <script> outside the main document without knowing its order. Try adding async="" or moving it into the root <head> tag.',
+      'Warning: validateDOMNesting(...): <script> cannot appear as a child of <html>.',
+    ]);
+
+    renderSafelyAndExpect(
+      root,
+      <>
+        <html>
+          <script async={true} onLoad={() => {}} href="bar" />
+          <body>foo</body>
+        </html>
+      </>,
+    ).toErrorDev([
+      'Cannot render a <script> with onLoad or onError listeners outside the main document. Try removing onLoad={...} and onError={...} or moving it into the root <head> tag or somewhere in the <body>.',
+    ]);
+
+    renderSafelyAndExpect(
+      root,
+      <>
+        <link rel="foo" onLoad={() => {}} href="bar" />
+        <html>
+          <body>foo</body>
+        </html>
+      </>,
+    ).toErrorDev(
+      [
+        'Cannot render a <link> with onLoad or onError listeners outside the main document. Try removing onLoad={...} and onError={...} or moving it into the root <head> tag or somewhere in the <body>.',
+      ],
+      {withoutStack: 1},
+    );
+  });
+
   // @gate enableFloat
   it('can acquire a resource after releasing it in the same commit', async () => {
     const root = ReactDOMClient.createRoot(container);

diff --git a/packages/react-reconciler/src/ReactFiberHostContext.js b/packages/react-reconciler/src/ReactFiberHostContext.js
@@ -7,15 +7,25 @@
  * @flow
  */
 
-import type {Container} from './ReactFiberHostConfig';
+import type {Container, HostContext} from './ReactFiberHostConfig';
 import {enableNewReconciler} from 'shared/ReactFeatureFlags';
 
-import {getCurrentRootHostContainer as getCurrentRootHostContainer_old} from './ReactFiberHostContext.old';
+import {
+  getCurrentRootHostContainer as getCurrentRootHostContainer_old,
+  getHostContext as getHostContext_old,
+} from './ReactFiberHostContext.old';
 
-import {getCurrentRootHostContainer as getCurrentRootHostContainer_new} from './ReactFiberHostContext.new';
+import {
+  getCurrentRootHostContainer as getCurrentRootHostContainer_new,
+  getHostContext as getHostContext_new,
+} from './ReactFiberHostContext.new';
 
 export function getCurrentRootHostContainer(): null | Container {
   return enableNewReconciler
     ? getCurrentRootHostContainer_new()
     : getCurrentRootHostContainer_old();
 }
+
+export function getHostContext(): HostContext {
+  return enableNewReconciler ? getHostContext_new() : getHostContext_old();
+}
diff --git a/packages/react-reconciler/src/ReactFiberHostContext.new.js b/packages/react-reconciler/src/ReactFiberHostContext.new.js
@@ -40,7 +40,7 @@ function requiredContext<Value>(c: Value | NoContextT): Value {
 
 function getCurrentRootHostContainer(): null | Container {
   const container = rootInstanceStackCursor.current;
-  return container === NO_CONTEXT ? null : (container: any);
+  return container === NO_CONTEXT ? null : ((container: any): Container);
 }
 
 function getRootHostContainer(): Container {
@@ -106,8 +106,8 @@ function popHostContext(fiber: Fiber): void {
 }
 
 export {
-  getCurrentRootHostContainer,
   getHostContext,
+  getCurrentRootHostContainer,
   getRootHostContainer,
   popHostContainer,
   popHostContext,

diff --git a/packages/react-reconciler/src/ReactFiberHostContext.old.js b/packages/react-reconciler/src/ReactFiberHostContext.old.js
@@ -40,7 +40,7 @@ function requiredContext<Value>(c: Value | NoContextT): Value {
 
 function getCurrentRootHostContainer(): null | Container {
   const container = rootInstanceStackCursor.current;
-  return container === NO_CONTEXT ? null : (container: any);
+  return container === NO_CONTEXT ? null : ((container: any): Container);
 }
 
 function getRootHostContainer(): Container {
@@ -106,8 +106,8 @@ function popHostContext(fiber: Fiber): void {
 }
 
 export {
-  getCurrentRootHostContainer,
   getHostContext,
+  getCurrentRootHostContainer,
   getRootHostContainer,
   popHostContainer,
   popHostContext,