Skip to content

Commit 9236abd

Browse files
gnoffgnoff
authored
when float is enabled only push title and script as a single unit (#25536)
replaces: #25535 This takes a more huerstic based approach with no new conditionals on the hot path of fizz rendering. If float is enabled * title and script can only have simple children * if non-simple children are found they will be ignored * title and script are pushed in a single unit during pushStartInstance including their children and closing tags If float is not enabled * the original pushing behaviors are in place and you can have complex children but you will get warnings
1 parent dd5c208 commit 9236abd

File tree

5 files changed

+293
-39
lines changed

5 files changed

+293
-39
lines changed

‎packages/react-dom-bindings/src/server/ReactDOMFloatServer.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -645,9 +645,8 @@ export function resourcesFromElement(type: string, props: Props): boolean {
645645
resources.headsMap.set(key, resource);
646646
resources.headResources.add(resource);
647647
}
648-
return true;
649648
}
650-
return false;
649+
return true;
651650
}
652651
case 'meta': {
653652
let key, propertyPath;

‎packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js‎

Lines changed: 169 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1294,21 +1294,102 @@ function pushStartMenuItem(
12941294
return null;
12951295
}
12961296

1297-
function pushStartTitle(
1297+
function pushTitle(
12981298
target: Array<Chunk | PrecomputedChunk>,
12991299
props: Object,
13001300
responseState: ResponseState,
13011301
): ReactNodeList {
1302+
if (__DEV__) {
1303+
const children = props.children;
1304+
const childForValidation =
1305+
Array.isArray(children) && children.length < 2
1306+
? children[0] || null
1307+
: children;
1308+
if (Array.isArray(children) && children.length > 1) {
1309+
console.error(
1310+
'A title element received an array with more than 1 element as children. ' +
1311+
'In browsers title Elements can only have Text Nodes as children. If ' +
1312+
'the children being rendered output more than a single text node in aggregate the browser ' +
1313+
'will display markup and comments as text in the title and hydration will likely fail and ' +
1314+
'fall back to client rendering',
1315+
);
1316+
} else if (
1317+
childForValidation != null &&
1318+
childForValidation.$$typeof != null
1319+
) {
1320+
console.error(
1321+
'A title element received a React element for children. ' +
1322+
'In the browser title Elements can only have Text Nodes as children. If ' +
1323+
'the children being rendered output more than a single text node in aggregate the browser ' +
1324+
'will display markup and comments as text in the title and hydration will likely fail and ' +
1325+
'fall back to client rendering',
1326+
);
1327+
} else if (
1328+
childForValidation != null &&
1329+
typeof childForValidation !== 'string' &&
1330+
typeof childForValidation !== 'number'
1331+
) {
1332+
console.error(
1333+
'A title element received a value that was not a string or number for children. ' +
1334+
'In the browser title Elements can only have Text Nodes as children. If ' +
1335+
'the children being rendered output more than a single text node in aggregate the browser ' +
1336+
'will display markup and comments as text in the title and hydration will likely fail and ' +
1337+
'fall back to client rendering',
1338+
);
1339+
}
1340+
}
1341+
13021342
if (enableFloat && resourcesFromElement('title', props)) {
13031343
// We have converted this link exclusively to a resource and no longer
13041344
// need to emit it
13051345
return null;
13061346
}
1347+
return pushTitleImpl(target, props, responseState);
1348+
}
13071349

1308-
return pushStartTitleImpl(target, props, responseState);
1350+
function pushTitleImpl(
1351+
target: Array<Chunk | PrecomputedChunk>,
1352+
props: Object,
1353+
responseState: ResponseState,
1354+
): null {
1355+
target.push(startChunkForTag('title'));
1356+
1357+
let children = null;
1358+
for (const propKey in props) {
1359+
if (hasOwnProperty.call(props, propKey)) {
1360+
const propValue = props[propKey];
1361+
if (propValue == null) {
1362+
continue;
1363+
}
1364+
switch (propKey) {
1365+
case 'children':
1366+
children = propValue;
1367+
break;
1368+
case 'dangerouslySetInnerHTML':
1369+
throw new Error(
1370+
'`dangerouslySetInnerHTML` does not make sense on <title>.',
1371+
);
1372+
// eslint-disable-next-line-no-fallthrough
1373+
default:
1374+
pushAttribute(target, responseState, propKey, propValue);
1375+
break;
1376+
}
1377+
}
1378+
}
1379+
target.push(endOfStartTag);
1380+
1381+
const child =
1382+
Array.isArray(children) && children.length < 2
1383+
? children[0] || null
1384+
: children;
1385+
if (typeof child === 'string' || typeof child === 'number') {
1386+
target.push(stringToChunk(escapeTextForBrowser(child)));
1387+
}
1388+
target.push(endTag1, stringToChunk('title'), endTag2);
1389+
return null;
13091390
}
13101391

1311-
function pushStartTitleImpl(
1392+
function pushStartTitle(
13121393
target: Array<Chunk | PrecomputedChunk>,
13131394
props: Object,
13141395
responseState: ResponseState,
@@ -1340,7 +1421,7 @@ function pushStartTitleImpl(
13401421
target.push(endOfStartTag);
13411422

13421423
if (__DEV__) {
1343-
const child =
1424+
const childForValidation =
13441425
Array.isArray(children) && children.length < 2
13451426
? children[0] || null
13461427
: children;
@@ -1352,7 +1433,10 @@ function pushStartTitleImpl(
13521433
'will display markup and comments as text in the title and hydration will likely fail and ' +
13531434
'fall back to client rendering',
13541435
);
1355-
} else if (child != null && child.$$typeof != null) {
1436+
} else if (
1437+
childForValidation != null &&
1438+
childForValidation.$$typeof != null
1439+
) {
13561440
console.error(
13571441
'A title element received a React element for children. ' +
13581442
'In the browser title Elements can only have Text Nodes as children. If ' +
@@ -1361,9 +1445,9 @@ function pushStartTitleImpl(
13611445
'fall back to client rendering',
13621446
);
13631447
} else if (
1364-
child != null &&
1365-
typeof child !== 'string' &&
1366-
typeof child !== 'number'
1448+
childForValidation != null &&
1449+
typeof childForValidation !== 'string' &&
1450+
typeof childForValidation !== 'number'
13671451
) {
13681452
console.error(
13691453
'A title element received a value that was not a string or number for children. ' +
@@ -1374,6 +1458,7 @@ function pushStartTitleImpl(
13741458
);
13751459
}
13761460
}
1461+
13771462
return children;
13781463
}
13791464

@@ -1410,12 +1495,12 @@ function pushStartHtml(
14101495
return pushStartGenericElement(target, props, tag, responseState);
14111496
}
14121497

1413-
function pushStartScript(
1498+
function pushScript(
14141499
target: Array<Chunk | PrecomputedChunk>,
14151500
props: Object,
14161501
responseState: ResponseState,
14171502
textEmbedded: boolean,
1418-
): ReactNodeList {
1503+
): null {
14191504
if (enableFloat && resourcesFromScript(props)) {
14201505
if (textEmbedded) {
14211506
// This link follows text but we aren't writing a tag. while not as efficient as possible we need
@@ -1427,7 +1512,61 @@ function pushStartScript(
14271512
return null;
14281513
}
14291514

1430-
return pushStartGenericElement(target, props, 'script', responseState);
1515+
return pushScriptImpl(target, props, responseState);
1516+
}
1517+
1518+
function pushScriptImpl(
1519+
target: Array<Chunk | PrecomputedChunk>,
1520+
props: Object,
1521+
responseState: ResponseState,
1522+
): null {
1523+
target.push(startChunkForTag('script'));
1524+
1525+
let children = null;
1526+
let innerHTML = null;
1527+
for (const propKey in props) {
1528+
if (hasOwnProperty.call(props, propKey)) {
1529+
const propValue = props[propKey];
1530+
if (propValue == null) {
1531+
continue;
1532+
}
1533+
switch (propKey) {
1534+
case 'children':
1535+
children = propValue;
1536+
break;
1537+
case 'dangerouslySetInnerHTML':
1538+
innerHTML = propValue;
1539+
break;
1540+
default:
1541+
pushAttribute(target, responseState, propKey, propValue);
1542+
break;
1543+
}
1544+
}
1545+
}
1546+
target.push(endOfStartTag);
1547+
1548+
if (__DEV__) {
1549+
if (children != null && typeof children !== 'string') {
1550+
const descriptiveStatement =
1551+
typeof children === 'number'
1552+
? 'a number for children'
1553+
: Array.isArray(children)
1554+
? 'an array for children'
1555+
: 'something unexpected for children';
1556+
console.error(
1557+
'A script element was rendered with %s. If script element has children it must be a single string.' +
1558+
' Consider using dangerouslySetInnerHTML or passing a plain string as children.',
1559+
descriptiveStatement,
1560+
);
1561+
}
1562+
}
1563+
1564+
pushInnerHTML(target, innerHTML, children);
1565+
if (typeof children === 'string') {
1566+
target.push(stringToChunk(encodeHTMLTextNode(children)));
1567+
}
1568+
target.push(endTag1, stringToChunk('script'), endTag2);
1569+
return null;
14311570
}
14321571

14331572
function pushStartGenericElement(
@@ -1703,11 +1842,15 @@ export function pushStartInstance(
17031842
case 'menuitem':
17041843
return pushStartMenuItem(target, props, responseState);
17051844
case 'title':
1706-
return pushStartTitle(target, props, responseState);
1845+
return enableFloat
1846+
? pushTitle(target, props, responseState)
1847+
: pushStartTitle(target, props, responseState);
17071848
case 'link':
17081849
return pushLink(target, props, responseState, textEmbedded);
17091850
case 'script':
1710-
return pushStartScript(target, props, responseState, textEmbedded);
1851+
return enableFloat
1852+
? pushScript(target, props, responseState, textEmbedded)
1853+
: pushStartGenericElement(target, props, type, responseState);
17111854
case 'meta':
17121855
return pushMeta(target, props, responseState, textEmbedded);
17131856
// Newline eating tags
@@ -1777,9 +1920,19 @@ export function pushEndInstance(
17771920
props: Object,
17781921
): void {
17791922
switch (type) {
1923+
// When float is on we expect title and script tags to always be pushed in
1924+
// a unit and never return children. when we end up pushing the end tag we
1925+
// want to ensure there is no extra closing tag pushed
1926+
case 'title':
1927+
case 'script': {
1928+
if (!enableFloat) {
1929+
break;
1930+
}
1931+
}
17801932
// Omitted close tags
17811933
// TODO: Instead of repeating this switch we could try to pass a flag from above.
17821934
// That would require returning a tuple. Which might be ok if it gets inlined.
1935+
// eslint-disable-next-line-no-fallthrough
17831936
case 'area':
17841937
case 'base':
17851938
case 'br':
@@ -2396,8 +2549,7 @@ export function writeInitialResources(
23962549

23972550
scripts.forEach(r => {
23982551
// should never be flushed already
2399-
pushStartGenericElement(target, r.props, 'script', responseState);
2400-
pushEndInstance(target, target, 'script', r.props);
2552+
pushScriptImpl(target, r.props, responseState);
24012553
r.flushed = true;
24022554
r.hint.flushed = true;
24032555
});
@@ -2415,11 +2567,7 @@ export function writeInitialResources(
24152567
headResources.forEach(r => {
24162568
switch (r.type) {
24172569
case 'title': {
2418-
pushStartTitleImpl(target, r.props, responseState);
2419-
if (typeof r.props.children === 'string') {
2420-
target.push(stringToChunk(escapeTextForBrowser(r.props.children)));
2421-
}
2422-
pushEndInstance(target, target, 'title', r.props);
2570+
pushTitleImpl(target, r.props, responseState);
24232571
break;
24242572
}
24252573
case 'meta': {
@@ -2516,11 +2664,7 @@ export function writeImmediateResources(
25162664
headResources.forEach(r => {
25172665
switch (r.type) {
25182666
case 'title': {
2519-
pushStartTitleImpl(target, r.props, responseState);
2520-
if (typeof r.props.children === 'string') {
2521-
target.push(stringToChunk(escapeTextForBrowser(r.props.children)));
2522-
}
2523-
pushEndInstance(target, target, 'title', r.props);
2667+
pushTitleImpl(target, r.props, responseState);
25242668
break;
25252669
}
25262670
case 'meta': {

0 commit comments

Comments
 (0)