Skip to content

Commit

Permalink
[Fizz] Fix children rendering in custom elements with `enableCustomEl…
Browse files Browse the repository at this point in the history
…ementPropertySupport` (#27511)

The `enableCustomElementPropertySupport` flag changes React's handling
of custom elements in a way that is more useful that just treating every
prop as an attribute. However when server rendering we have no choice
but to serialize props as attributes. When this flag is on and React
supports more prop types on the client like functions and objects the
server implementation needs to be a bit more naunced in how it renders
these components. With this flag on `false`, function, and object props
are omitted entirely and `true` is normalized to `""`. There was a bug
however in the implementation which caused children more complex than a
single string to be omitted because it matched the object type filter.
This change reorganizes the code a bit to put these filters in the
default prop handline case, leaving children, style, and innerHTML to be
handled via normal logic.

fixes: facebook/react#27286

DiffTrain build for [bb778528d1ca22b44dad832f0258aaa4c0e6d4a4](facebook/react@bb77852)
  • Loading branch information
jerrydev0927 committed Oct 12, 2023
1 parent be04453 commit 6b8275d
Show file tree
Hide file tree
Showing 9 changed files with 161 additions and 182 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1fc58281af73ca4507c41d53a3e08dc2038b0c1f
bb778528d1ca22b44dad832f0258aaa4c0e6d4a4
2 changes: 1 addition & 1 deletion compiled/facebook-www/ReactART-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
return self;
}

var ReactVersion = "18.3.0-www-modern-5fb559f6";
var ReactVersion = "18.3.0-www-modern-70e823a6";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down
4 changes: 2 additions & 2 deletions compiled/facebook-www/ReactART-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -9780,7 +9780,7 @@ var slice = Array.prototype.slice,
return null;
},
bundleType: 0,
version: "18.3.0-www-modern-21a11200",
version: "18.3.0-www-modern-7f660f7a",
rendererPackageName: "react-art"
};
var internals$jscomp$inline_1286 = {
Expand Down Expand Up @@ -9811,7 +9811,7 @@ var internals$jscomp$inline_1286 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-www-modern-21a11200"
reconcilerVersion: "18.3.0-www-modern-7f660f7a"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1287 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
47 changes: 22 additions & 25 deletions compiled/facebook-www/ReactDOMServer-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ if (__DEV__) {
var React = require("react");
var ReactDOM = require("react-dom");

var ReactVersion = "18.3.0-www-classic-0d5846ce";
var ReactVersion = "18.3.0-www-classic-c2645119";

// This refers to a WWW module.
var warningWWW = require("warning");
Expand Down Expand Up @@ -4715,29 +4715,7 @@ function pushStartCustomElement(target, props, tag) {
continue;
}

if (
enableCustomElementPropertySupport &&
(typeof propValue === "function" || typeof propValue === "object")
) {
// It is normal to render functions and objects on custom elements when
// client rendering, but when server rendering the output isn't useful,
// so skip it.
continue;
}

if (enableCustomElementPropertySupport && propValue === false) {
continue;
}

if (enableCustomElementPropertySupport && propValue === true) {
propValue = "";
}

if (enableCustomElementPropertySupport && propKey === "className") {
// className gets rendered as class on the client, so it should be
// rendered as class on the server.
propKey = "class";
}
var attributeName = propKey;

switch (propKey) {
case "children":
Expand All @@ -4757,15 +4735,34 @@ function pushStartCustomElement(target, props, tag) {
// Ignored. These are built-in to React on the client.
break;

case "className":
if (enableCustomElementPropertySupport) {
// className gets rendered as class on the client, so it should be
// rendered as class on the server.
attributeName = "class";
}

// intentional fallthrough

default:
if (
isAttributeNameSafe(propKey) &&
typeof propValue !== "function" &&
typeof propValue !== "symbol"
) {
if (enableCustomElementPropertySupport) {
if (propValue === false) {
continue;
} else if (propValue === true) {
propValue = "";
} else if (typeof propValue === "object") {
continue;
}
}

target.push(
attributeSeparator,
stringToChunk(propKey),
stringToChunk(attributeName),
attributeAssign,
stringToChunk(escapeTextForBrowser(propValue)),
attributeEnd
Expand Down
47 changes: 22 additions & 25 deletions compiled/facebook-www/ReactDOMServer-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ if (__DEV__) {
var React = require("react");
var ReactDOM = require("react-dom");

var ReactVersion = "18.3.0-www-modern-5fb559f6";
var ReactVersion = "18.3.0-www-modern-70e823a6";

// This refers to a WWW module.
var warningWWW = require("warning");
Expand Down Expand Up @@ -4715,29 +4715,7 @@ function pushStartCustomElement(target, props, tag) {
continue;
}

if (
enableCustomElementPropertySupport &&
(typeof propValue === "function" || typeof propValue === "object")
) {
// It is normal to render functions and objects on custom elements when
// client rendering, but when server rendering the output isn't useful,
// so skip it.
continue;
}

if (enableCustomElementPropertySupport && propValue === false) {
continue;
}

if (enableCustomElementPropertySupport && propValue === true) {
propValue = "";
}

if (enableCustomElementPropertySupport && propKey === "className") {
// className gets rendered as class on the client, so it should be
// rendered as class on the server.
propKey = "class";
}
var attributeName = propKey;

switch (propKey) {
case "children":
Expand All @@ -4757,15 +4735,34 @@ function pushStartCustomElement(target, props, tag) {
// Ignored. These are built-in to React on the client.
break;

case "className":
if (enableCustomElementPropertySupport) {
// className gets rendered as class on the client, so it should be
// rendered as class on the server.
attributeName = "class";
}

// intentional fallthrough

default:
if (
isAttributeNameSafe(propKey) &&
typeof propValue !== "function" &&
typeof propValue !== "symbol"
) {
if (enableCustomElementPropertySupport) {
if (propValue === false) {
continue;
} else if (propValue === true) {
propValue = "";
} else if (typeof propValue === "object") {
continue;
}
}

target.push(
attributeSeparator,
stringToChunk(propKey),
stringToChunk(attributeName),
attributeAssign,
stringToChunk(escapeTextForBrowser(propValue)),
attributeEnd
Expand Down
78 changes: 37 additions & 41 deletions compiled/facebook-www/ReactDOMServer-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -1527,25 +1527,9 @@ function pushStartInstance(
for (propKey$jscomp$9 in props)
if (hasOwnProperty.call(props, propKey$jscomp$9)) {
var propValue$jscomp$9 = props[propKey$jscomp$9];
if (
!(
null == propValue$jscomp$9 ||
(enableCustomElementPropertySupport &&
("function" === typeof propValue$jscomp$9 ||
"object" === typeof propValue$jscomp$9)) ||
(enableCustomElementPropertySupport &&
!1 === propValue$jscomp$9)
)
)
switch (
(enableCustomElementPropertySupport &&
!0 === propValue$jscomp$9 &&
(propValue$jscomp$9 = ""),
enableCustomElementPropertySupport &&
"className" === propKey$jscomp$9 &&
(propKey$jscomp$9 = "class"),
propKey$jscomp$9)
) {
if (null != propValue$jscomp$9) {
var attributeName = propKey$jscomp$9;
switch (propKey$jscomp$9) {
case "children":
children$jscomp$7 = propValue$jscomp$9;
break;
Expand All @@ -1558,18 +1542,30 @@ function pushStartInstance(
case "suppressContentEditableWarning":
case "suppressHydrationWarning":
break;
case "className":
enableCustomElementPropertySupport &&
(attributeName = "class");
default:
isAttributeNameSafe(propKey$jscomp$9) &&
if (
isAttributeNameSafe(propKey$jscomp$9) &&
"function" !== typeof propValue$jscomp$9 &&
"symbol" !== typeof propValue$jscomp$9 &&
"symbol" !== typeof propValue$jscomp$9
) {
if (enableCustomElementPropertySupport)
if (!1 === propValue$jscomp$9) continue;
else if (!0 === propValue$jscomp$9)
propValue$jscomp$9 = "";
else if ("object" === typeof propValue$jscomp$9) continue;
target$jscomp$0.push(
" ",
propKey$jscomp$9,
attributeName,
'="',
escapeTextForBrowser(propValue$jscomp$9),
'"'
);
}
}
}
}
target$jscomp$0.push(">");
pushInnerHTML(target$jscomp$0, innerHTML$jscomp$6, children$jscomp$7);
Expand Down Expand Up @@ -2291,24 +2287,24 @@ function hoistStylesheetDependency(stylesheet) {
function createRenderState(resumableState, generateStaticMarkup) {
var idPrefix = resumableState.idPrefix;
resumableState = idPrefix + "P:";
var JSCompiler_object_inline_segmentPrefix_1568 = idPrefix + "S:";
var JSCompiler_object_inline_segmentPrefix_1569 = idPrefix + "S:";
idPrefix += "B:";
var JSCompiler_object_inline_preconnects_1580 = new Set(),
JSCompiler_object_inline_fontPreloads_1581 = new Set(),
JSCompiler_object_inline_highImagePreloads_1582 = new Set(),
JSCompiler_object_inline_styles_1583 = new Map(),
JSCompiler_object_inline_bootstrapScripts_1584 = new Set(),
JSCompiler_object_inline_scripts_1585 = new Set(),
JSCompiler_object_inline_bulkPreloads_1586 = new Set(),
JSCompiler_object_inline_preloads_1587 = {
var JSCompiler_object_inline_preconnects_1581 = new Set(),
JSCompiler_object_inline_fontPreloads_1582 = new Set(),
JSCompiler_object_inline_highImagePreloads_1583 = new Set(),
JSCompiler_object_inline_styles_1584 = new Map(),
JSCompiler_object_inline_bootstrapScripts_1585 = new Set(),
JSCompiler_object_inline_scripts_1586 = new Set(),
JSCompiler_object_inline_bulkPreloads_1587 = new Set(),
JSCompiler_object_inline_preloads_1588 = {
images: new Map(),
stylesheets: new Map(),
scripts: new Map(),
moduleScripts: new Map()
};
return {
placeholderPrefix: resumableState,
segmentPrefix: JSCompiler_object_inline_segmentPrefix_1568,
segmentPrefix: JSCompiler_object_inline_segmentPrefix_1569,
boundaryPrefix: idPrefix,
startInlineScript: "<script>",
htmlChunks: null,
Expand All @@ -2320,14 +2316,14 @@ function createRenderState(resumableState, generateStaticMarkup) {
importMapChunks: [],
preloadChunks: [],
hoistableChunks: [],
preconnects: JSCompiler_object_inline_preconnects_1580,
fontPreloads: JSCompiler_object_inline_fontPreloads_1581,
highImagePreloads: JSCompiler_object_inline_highImagePreloads_1582,
styles: JSCompiler_object_inline_styles_1583,
bootstrapScripts: JSCompiler_object_inline_bootstrapScripts_1584,
scripts: JSCompiler_object_inline_scripts_1585,
bulkPreloads: JSCompiler_object_inline_bulkPreloads_1586,
preloads: JSCompiler_object_inline_preloads_1587,
preconnects: JSCompiler_object_inline_preconnects_1581,
fontPreloads: JSCompiler_object_inline_fontPreloads_1582,
highImagePreloads: JSCompiler_object_inline_highImagePreloads_1583,
styles: JSCompiler_object_inline_styles_1584,
bootstrapScripts: JSCompiler_object_inline_bootstrapScripts_1585,
scripts: JSCompiler_object_inline_scripts_1586,
bulkPreloads: JSCompiler_object_inline_bulkPreloads_1587,
preloads: JSCompiler_object_inline_preloads_1588,
boundaryResources: null,
stylesToHoist: !1,
generateStaticMarkup: generateStaticMarkup
Expand Down Expand Up @@ -5050,4 +5046,4 @@ exports.renderToString = function (children, options) {
'The server used "renderToString" which does not support Suspense. If you intended for this Suspense boundary to render the fallback content on the server consider throwing an Error somewhere within the Suspense boundary. If you intended to have the server wait for the suspended component please switch to "renderToReadableStream" which supports Suspense on the server'
);
};
exports.version = "18.3.0-www-classic-959d060f";
exports.version = "18.3.0-www-classic-d1253dd4";
Loading

0 comments on commit 6b8275d

Please sign in to comment.