@samitouri / QOS-React / commits / 0c11baa6ab

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>`

Josh Story committed Oct 22, 2022 at 15:19 UTC 0c11baa6abea41132d0c894264783041ca852548
6 files changed +195 -10
packages/react-dom-bindings/src/client/ReactDOMFloatClient.js
+54 -2
@@ -29,7 +29,11 @@ import {
29 markNodeAsResource,
30 } from './ReactDOMComponentTree';
31 import {HTML_NAMESPACE} from '../shared/DOMNamespaces';
32 -import {getCurrentRootHostContainer} from 'react-reconciler/src/ReactFiberHostContext';
32 +import {
33 + getCurrentRootHostContainer,
34 + getHostContext,
35 +} from 'react-reconciler/src/ReactFiberHostContext';
36 +import {getResourceFormOnly} from './validateDOMNesting';
37
38 // The resource types we support. currently they match the form for the as argument.
39 // In the future this may need to change, especially when modules / scripts are supported
@@ -1331,6 +1335,11 @@ function insertResourceInstanceBefore(
1335 }
1336
1337 export function isHostResourceType(type: string, props: Props): boolean {
1338 + let resourceFormOnly: boolean;
1339 + if (__DEV__) {
1340 + const hostContext = getHostContext();
1341 + resourceFormOnly = getResourceFormOnly(hostContext);
1342 + }
1343 switch (type) {
1344 case 'meta':
1345 case 'title': {
@@ -1339,14 +1348,29 @@ export function isHostResourceType(type: string, props: Props): boolean {
1348 case 'link': {
1349 const {onLoad, onError} = props;
1350 if (onLoad || onError) {
1351 + if (__DEV__) {
1352 + if (resourceFormOnly) {
1353 + console.error(
1354 + 'Cannot render a <link> with onLoad or onError listeners outside the main document.' +
1355 + ' Try removing onLoad={...} and onError={...} or moving it into the root <head> tag or' +
1356 + ' somewhere in the <body>.',
1357 + );
1358 + }
1359 + }
1360 return false;
1361 }
1362 switch (props.rel) {
1363 case 'stylesheet': {
1364 + const {href, precedence, disabled} = props;
1365 if (__DEV__) {
1366 validateLinkPropsForStyleResource(props);
1367 + if (typeof precedence !== 'string' && resourceFormOnly) {
1368 + console.error(
1369 + 'Cannot render a <link rel="stylesheet" /> outside the main document without knowing its precedence.' +
1370 + ' Consider adding precedence="default" or moving it into the root <head> tag.',
1371 + );
1372 + }
1373 }
1349 - const {href, precedence, disabled} = props;
1374 return (
1375 typeof href === 'string' &&
1376 typeof precedence === 'string' &&
@@ -1363,8 +1387,36 @@ export function isHostResourceType(type: string, props: Props): boolean {
1387 // We don't validate because it is valid to use async with onLoad/onError unlike combining
1388 // precedence with these for style resources
1389 const {src, async, onLoad, onError} = props;
1390 + if (__DEV__) {
1391 + if (async !== true && resourceFormOnly) {
1392 + console.error(
1393 + 'Cannot render a sync or defer <script> outside the main document without knowing its order.' +
1394 + ' Try adding async="" or moving it into the root <head> tag.',
1395 + );
1396 + } else if ((onLoad || onError) && resourceFormOnly) {
1397 + console.error(
1398 + 'Cannot render a <script> with onLoad or onError listeners outside the main document.' +
1399 + ' Try removing onLoad={...} and onError={...} or moving it into the root <head> tag or' +
1400 + ' somewhere in the <body>.',
1401 + );
1402 + }
1403 + }
1404 return (async: any) && typeof src === 'string' && !onLoad && !onError;
1405 }
1406 + case 'base':
1407 + case 'template':
1408 + case 'style':
1409 + case 'noscript': {
1410 + if (__DEV__) {
1411 + if (resourceFormOnly) {
1412 + console.error(
1413 + 'Cannot render <%s> outside the main document. Try moving it into the root <head> tag.',
1414 + type,
1415 + );
1416 + }
1417 + }
1418 + return false;
1419 + }
1420 }
1421 return false;
1422 }
packages/react-dom-bindings/src/client/validateDOMNesting.js
+12 -1
@@ -7,6 +7,7 @@
7
8 let validateDOMNesting = () => {};
9 let updatedAncestorInfo = () => {};
10 +let getResourceFormOnly = () => false;
11
12 if (__DEV__) {
13 // This validation code was written based on the HTML5 parsing spec:
@@ -153,6 +154,8 @@ if (__DEV__) {
154
155 listItemTagAutoclosing: null,
156 dlItemTagAutoclosing: null,
157 +
158 + resourceFormOnly: true,
159 };
160
161 updatedAncestorInfo = function(oldInfo, tag) {
@@ -180,6 +183,10 @@ if (__DEV__) {
183 ancestorInfo.dlItemTagAutoclosing = null;
184 }
185
186 + if (tag !== '#document' && tag !== 'html') {
187 + ancestorInfo.resourceFormOnly = false;
188 + }
189 +
190 ancestorInfo.current = info;
191
192 if (tag === 'form') {
@@ -472,6 +479,10 @@ if (__DEV__) {
479 );
480 }
481 };
482 +
483 + getResourceFormOnly = hostContextDev => {
484 + return hostContextDev.ancestorInfo.resourceFormOnly;
485 + };
486 }
487
477 -export {updatedAncestorInfo, validateDOMNesting};
488 +export {updatedAncestorInfo, validateDOMNesting, getResourceFormOnly};
packages/react-dom/src/__tests__/ReactDOMFloat-test.js
+112
@@ -268,6 +268,118 @@ describe('ReactDOMFloat', () => {
268 );
269 });
270
271 + function renderSafelyAndExpect(root, children) {
272 + root.render(children);
273 + return expect(() => {
274 + try {
275 + expect(Scheduler).toFlushWithoutYielding();
276 + } catch (e) {
277 + try {
278 + expect(Scheduler).toFlushWithoutYielding();
279 + } catch (f) {}
280 + }
281 + });
282 + }
283 +
284 + // @gate enableFloat || !__DEV__
285 + it('warns if you render resource-like elements above <head> or <body>', async () => {
286 + const root = ReactDOMClient.createRoot(document);
287 +
288 + renderSafelyAndExpect(
289 + root,
290 + <>
291 + <noscript>foo</noscript>
292 + <html>
293 + <body>foo</body>
294 + </html>
295 + </>,
296 + ).toErrorDev(
297 + [
298 + 'Cannot render <noscript> outside the main document. Try moving it into the root <head> tag.',
299 + 'Warning: validateDOMNesting(...): <noscript> cannot appear as a child of <#document>.',
300 + ],
301 + {withoutStack: 1},
302 + );
303 +
304 + renderSafelyAndExpect(
305 + root,
306 + <html>
307 + <template>foo</template>
308 + <body>foo</body>
309 + </html>,
310 + ).toErrorDev([
311 + 'Cannot render <template> outside the main document. Try moving it into the root <head> tag.',
312 + 'Warning: validateDOMNesting(...): <template> cannot appear as a child of <html>.',
313 + ]);
314 +
315 + renderSafelyAndExpect(
316 + root,
317 + <html>
318 + <body>foo</body>
319 + <style>foo</style>
320 + </html>,
321 + ).toErrorDev([
322 + 'Cannot render <style> outside the main document. Try moving it into the root <head> tag.',
323 + 'Warning: validateDOMNesting(...): <style> cannot appear as a child of <html>.',
324 + ]);
325 +
326 + renderSafelyAndExpect(
327 + root,
328 + <>
329 + <html>
330 + <body>foo</body>
331 + </html>
332 + <link rel="stylesheet" href="foo" />
333 + </>,
334 + ).toErrorDev(
335 + [
336 + '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.',
337 + 'Warning: validateDOMNesting(...): <link> cannot appear as a child of <#document>.',
338 + ],
339 + {withoutStack: 1},
340 + );
341 +
342 + renderSafelyAndExpect(
343 + root,
344 + <>
345 + <html>
346 + <body>foo</body>
347 + <script href="foo" />
348 + </html>
349 + </>,
350 + ).toErrorDev([
351 + '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.',
352 + 'Warning: validateDOMNesting(...): <script> cannot appear as a child of <html>.',
353 + ]);
354 +
355 + renderSafelyAndExpect(
356 + root,
357 + <>
358 + <html>
359 + <script async={true} onLoad={() => {}} href="bar" />
360 + <body>foo</body>
361 + </html>
362 + </>,
363 + ).toErrorDev([
364 + '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>.',
365 + ]);
366 +
367 + renderSafelyAndExpect(
368 + root,
369 + <>
370 + <link rel="foo" onLoad={() => {}} href="bar" />
371 + <html>
372 + <body>foo</body>
373 + </html>
374 + </>,
375 + ).toErrorDev(
376 + [
377 + '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>.',
378 + ],
379 + {withoutStack: 1},
380 + );
381 + });
382 +
383 // @gate enableFloat
384 it('can acquire a resource after releasing it in the same commit', async () => {
385 const root = ReactDOMClient.createRoot(container);
packages/react-reconciler/src/ReactFiberHostContext.js
+13 -3
@@ -7,15 +7,25 @@
7 * @flow
8 */
9
10 -import type {Container} from './ReactFiberHostConfig';
10 +import type {Container, HostContext} from './ReactFiberHostConfig';
11 import {enableNewReconciler} from 'shared/ReactFeatureFlags';
12
13 -import {getCurrentRootHostContainer as getCurrentRootHostContainer_old} from './ReactFiberHostContext.old';
13 +import {
14 + getCurrentRootHostContainer as getCurrentRootHostContainer_old,
15 + getHostContext as getHostContext_old,
16 +} from './ReactFiberHostContext.old';
17
15 -import {getCurrentRootHostContainer as getCurrentRootHostContainer_new} from './ReactFiberHostContext.new';
18 +import {
19 + getCurrentRootHostContainer as getCurrentRootHostContainer_new,
20 + getHostContext as getHostContext_new,
21 +} from './ReactFiberHostContext.new';
22
23 export function getCurrentRootHostContainer(): null | Container {
24 return enableNewReconciler
25 ? getCurrentRootHostContainer_new()
26 : getCurrentRootHostContainer_old();
27 }
28 +
29 +export function getHostContext(): HostContext {
30 + return enableNewReconciler ? getHostContext_new() : getHostContext_old();
31 +}
packages/react-reconciler/src/ReactFiberHostContext.new.js
+2 -2
@@ -40,7 +40,7 @@ function requiredContext<Value>(c: Value | NoContextT): Value {
40
41 function getCurrentRootHostContainer(): null | Container {
42 const container = rootInstanceStackCursor.current;
43 - return container === NO_CONTEXT ? null : (container: any);
43 + return container === NO_CONTEXT ? null : ((container: any): Container);
44 }
45
46 function getRootHostContainer(): Container {
@@ -106,8 +106,8 @@ function popHostContext(fiber: Fiber): void {
106 }
107
108 export {
109 - getCurrentRootHostContainer,
109 getHostContext,
110 + getCurrentRootHostContainer,
111 getRootHostContainer,
112 popHostContainer,
113 popHostContext,
packages/react-reconciler/src/ReactFiberHostContext.old.js
+2 -2
@@ -40,7 +40,7 @@ function requiredContext<Value>(c: Value | NoContextT): Value {
40
41 function getCurrentRootHostContainer(): null | Container {
42 const container = rootInstanceStackCursor.current;
43 - return container === NO_CONTEXT ? null : (container: any);
43 + return container === NO_CONTEXT ? null : ((container: any): Container);
44 }
45
46 function getRootHostContainer(): Container {
@@ -106,8 +106,8 @@ function popHostContext(fiber: Fiber): void {
106 }
107
108 export {
109 - getCurrentRootHostContainer,
109 getHostContext,
110 + getCurrentRootHostContainer,
111 getRootHostContainer,
112 popHostContainer,
113 popHostContext,