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

6 files changed

Lines changed: 195 additions & 10 deletions

File tree

‎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
exportfunctionisHostResourceType(type: string,props: Props): boolean{
1338+
letresourceFormOnly: boolean;
1339+
if(__DEV__){
1340+
consthostContext=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
returnfalse;
13431361
}
13441362
switch(props.rel){
13451363
case 'stylesheet': {
1364+
const{href,precedence,disabled}=props;
13461365
if(__DEV__){
13471366
validateLinkPropsForStyleResource(props);
1367+
if(typeofprecedence!=='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
typeofhref==='string'&&
13521376
typeofprecedence==='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+
}elseif((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)&&typeofsrc==='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+
returnfalse;
1419+
}
13681420
}
13691421
returnfalse;
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
letvalidateDOMNesting=()=>{};
99
letupdatedAncestorInfo=()=>{};
10+
letgetResourceFormOnly=()=>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+
returnhostContextDev.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+
functionrenderSafelyAndExpect(root,children){
272+
root.render(children);
273+
returnexpect(()=>{
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+
constroot=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+
<linkrel="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+
<scripthref="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+
<scriptasync={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+
<linkrel="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
constroot=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-
importtype{Container}from'./ReactFiberHostConfig';
10+
importtype{Container,HostContext}from'./ReactFiberHostConfig';
1111
import{enableNewReconciler}from'shared/ReactFeatureFlags';
1212

13-
import{getCurrentRootHostContainerasgetCurrentRootHostContainer_old}from'./ReactFiberHostContext.old';
13+
import{
14+
getCurrentRootHostContainerasgetCurrentRootHostContainer_old,
15+
getHostContextasgetHostContext_old,
16+
}from'./ReactFiberHostContext.old';
1417

15-
import{getCurrentRootHostContainerasgetCurrentRootHostContainer_new}from'./ReactFiberHostContext.new';
18+
import{
19+
getCurrentRootHostContainerasgetCurrentRootHostContainer_new,
20+
getHostContextasgetHostContext_new,
21+
}from'./ReactFiberHostContext.new';
1622

1723
export functiongetCurrentRootHostContainer(): null|Container{
1824
returnenableNewReconciler
1925
? getCurrentRootHostContainer_new()
2026
: getCurrentRootHostContainer_old();
2127
}
28+
29+
exportfunctiongetHostContext(): HostContext{
30+
returnenableNewReconciler ? 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
functiongetCurrentRootHostContainer(): null|Container{
4242
constcontainer=rootInstanceStackCursor.current;
43-
returncontainer===NO_CONTEXT ? null : (container: any);
43+
returncontainer===NO_CONTEXT ? null : ((container: any): Container);
4444
}
4545

4646
functiongetRootHostContainer(): 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
functiongetCurrentRootHostContainer(): null|Container{
4242
constcontainer=rootInstanceStackCursor.current;
43-
returncontainer===NO_CONTEXT ? null : (container: any);
43+
returncontainer===NO_CONTEXT ? null : ((container: any): Container);
4444
}
4545

4646
functiongetRootHostContainer(): 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)