Skip to content

Commit 0c11baa

Browse files
gnoffgnoff
authored
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>`
1 parent 9236abd commit 0c11baa

File tree

6 files changed

+195
-10
lines changed

6 files changed

+195
-10
lines changed

‎packages/react-dom-bindings/src/client/ReactDOMFloatClient.js

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@ import {
2929
markNodeAsResource,
3030
} from './ReactDOMComponentTree';
3131
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';
3337

3438
// The resource types we support. currently they match the form for the as argument.
3539
// In the future this may need to change, especially when modules / scripts are supported
@@ -1331,6 +1335,11 @@ function insertResourceInstanceBefore(
13311335
}
13321336

13331337
export function isHostResourceType(type: string, props: Props): boolean {
1338+
let resourceFormOnly: boolean;
1339+
if (__DEV__) {
1340+
const hostContext = getHostContext();
1341+
resourceFormOnly = getResourceFormOnly(hostContext);
1342+
}
13341343
switch (type) {
13351344
case 'meta':
13361345
case 'title': {
@@ -1339,14 +1348,29 @@ export function isHostResourceType(type: string, props: Props): boolean {
13391348
case 'link': {
13401349
const {onLoad, onError} = props;
13411350
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+
}
13421360
return false;
13431361
}
13441362
switch (props.rel) {
13451363
case 'stylesheet': {
1364+
const {href, precedence, disabled} = props;
13461365
if (__DEV__) {
13471366
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+
}
13481373
}
1349-
const {href, precedence, disabled} = props;
13501374
return (
13511375
typeof href === 'string' &&
13521376
typeof precedence === 'string' &&
@@ -1363,8 +1387,36 @@ export function isHostResourceType(type: string, props: Props): boolean {
13631387
// We don't validate because it is valid to use async with onLoad/onError unlike combining
13641388
// precedence with these for style resources
13651389
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+
}
13661404
return (async: any) && typeof src === 'string' && !onLoad && !onError;
13671405
}
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+
}
13681420
}
13691421
return false;
13701422
}

‎packages/react-dom-bindings/src/client/validateDOMNesting.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
let validateDOMNesting = () => {};
99
let updatedAncestorInfo = () => {};
10+
let getResourceFormOnly = () => false;
1011

1112
if (__DEV__) {
1213
// This validation code was written based on the HTML5 parsing spec:
@@ -153,6 +154,8 @@ if (__DEV__) {
153154

154155
listItemTagAutoclosing: null,
155156
dlItemTagAutoclosing: null,
157+
158+
resourceFormOnly: true,
156159
};
157160

158161
updatedAncestorInfo = function(oldInfo, tag) {
@@ -180,6 +183,10 @@ if (__DEV__) {
180183
ancestorInfo.dlItemTagAutoclosing = null;
181184
}
182185

186+
if (tag !== '#document' && tag !== 'html') {
187+
ancestorInfo.resourceFormOnly = false;
188+
}
189+
183190
ancestorInfo.current = info;
184191

185192
if (tag === 'form') {
@@ -472,6 +479,10 @@ if (__DEV__) {
472479
);
473480
}
474481
};
482+
483+
getResourceFormOnly = hostContextDev => {
484+
return hostContextDev.ancestorInfo.resourceFormOnly;
485+
};
475486
}
476487

477-
export {updatedAncestorInfo, validateDOMNesting};
488+
export {updatedAncestorInfo, validateDOMNesting, getResourceFormOnly};

‎packages/react-dom/src/__tests__/ReactDOMFloat-test.js

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,118 @@ describe('ReactDOMFloat', () => {
268268
);
269269
});
270270

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+
271383
// @gate enableFloat
272384
it('can acquire a resource after releasing it in the same commit', async () => {
273385
const root = ReactDOMClient.createRoot(container);

‎packages/react-reconciler/src/ReactFiberHostContext.js

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,25 @@
77
* @flow
88
*/
99

10-
import type {Container} from './ReactFiberHostConfig';
10+
import type {Container, HostContext} from './ReactFiberHostConfig';
1111
import {enableNewReconciler} from 'shared/ReactFeatureFlags';
1212

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';
1417

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';
1622

1723
export function getCurrentRootHostContainer(): null | Container {
1824
return enableNewReconciler
1925
? getCurrentRootHostContainer_new()
2026
: getCurrentRootHostContainer_old();
2127
}
28+
29+
export function getHostContext(): HostContext {
30+
return enableNewReconciler ? getHostContext_new() : getHostContext_old();
31+
}

‎packages/react-reconciler/src/ReactFiberHostContext.new.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ function requiredContext<Value>(c: Value | NoContextT): Value {
4040

4141
function getCurrentRootHostContainer(): null | Container {
4242
const container = rootInstanceStackCursor.current;
43-
return container === NO_CONTEXT ? null : (container: any);
43+
return container === NO_CONTEXT ? null : ((container: any): Container);
4444
}
4545

4646
function getRootHostContainer(): Container {
@@ -106,8 +106,8 @@ function popHostContext(fiber: Fiber): void {
106106
}
107107

108108
export {
109-
getCurrentRootHostContainer,
110109
getHostContext,
110+
getCurrentRootHostContainer,
111111
getRootHostContainer,
112112
popHostContainer,
113113
popHostContext,

‎packages/react-reconciler/src/ReactFiberHostContext.old.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ function requiredContext<Value>(c: Value | NoContextT): Value {
4040

4141
function getCurrentRootHostContainer(): null | Container {
4242
const container = rootInstanceStackCursor.current;
43-
return container === NO_CONTEXT ? null : (container: any);
43+
return container === NO_CONTEXT ? null : ((container: any): Container);
4444
}
4545

4646
function getRootHostContainer(): Container {
@@ -106,8 +106,8 @@ function popHostContext(fiber: Fiber): void {
106106
}
107107

108108
export {
109-
getCurrentRootHostContainer,
110109
getHostContext,
110+
getCurrentRootHostContainer,
111111
getRootHostContainer,
112112
popHostContainer,
113113
popHostContext,

0 commit comments

Comments
 (0)