Skip to content

Commit 03462cf

Browse files
authored
[Fizz] External runtime: fix bug in processing existing elements (#26303)
## Summary Fix bug in how the Fizz external runtime processes existing template elements. Bug: - getElementsByTagName returns a HTMLCollection, which is live. - while iterating over an HTMLCollection, we call handleNode which removes nodes Fix: - Call Array.from to copy children of `document.body` before processing. - We could use `querySelectorAll` instead, but that is likely slower due to reading more nodes. ## How did you test this change? Did ad-hoc testing on Facebook home page by commenting out the mutation observer and adding the following. ```javascript window.addEventListener('DOMContentLoaded', function () { handleExistingNodes(document.body); }); ```
1 parent faacefb commit 03462cf

File tree

1 file changed

+29
-27
lines changed

1 file changed

+29
-27
lines changed

Diff for: packages/react-dom-bindings/src/server/ReactDOMServerExternalRuntime.js

+29-27
Original file line numberDiff line numberDiff line change
@@ -21,35 +21,37 @@ if (!window.$RC) {
2121
window.$RM = new Map();
2222
}
2323

24-
if (document.readyState === 'loading') {
25-
if (document.body != null) {
24+
if (document.body != null) {
25+
if (document.readyState === 'loading') {
2626
installFizzInstrObserver(document.body);
27-
} else {
28-
// body may not exist yet if the fizz runtime is sent in <head>
29-
// (e.g. as a preinit resource)
30-
// $FlowFixMe[recursive-definition]
31-
const domBodyObserver = new MutationObserver(() => {
32-
// We expect the body node to be stable once parsed / created
33-
if (document.body) {
34-
if (document.readyState === 'loading') {
35-
installFizzInstrObserver(document.body);
36-
}
37-
handleExistingNodes();
38-
// We can call disconnect without takeRecord here,
39-
// since we only expect a single document.body
40-
domBodyObserver.disconnect();
41-
}
42-
});
43-
// documentElement must already exist at this point
44-
// $FlowFixMe[incompatible-call]
45-
domBodyObserver.observe(document.documentElement, {childList: true});
4627
}
47-
}
28+
// $FlowFixMe[incompatible-cast]
29+
handleExistingNodes((document.body /*: HTMLElement */));
30+
} else {
31+
// Document must be loading -- body may not exist yet if the fizz external
32+
// runtime is sent in <head> (e.g. as a preinit resource)
33+
// $FlowFixMe[recursive-definition]
34+
const domBodyObserver = new MutationObserver(() => {
35+
// We expect the body node to be stable once parsed / created
36+
if (document.body != null) {
37+
if (document.readyState === 'loading') {
38+
installFizzInstrObserver(document.body);
39+
}
40+
// $FlowFixMe[incompatible-cast]
41+
handleExistingNodes((document.body /*: HTMLElement */));
4842

49-
handleExistingNodes();
43+
// We can call disconnect without takeRecord here,
44+
// since we only expect a single document.body
45+
domBodyObserver.disconnect();
46+
}
47+
});
48+
// documentElement must already exist at this point
49+
// $FlowFixMe[incompatible-call]
50+
domBodyObserver.observe(document.documentElement, {childList: true});
51+
}
5052

51-
function handleExistingNodes() {
52-
const existingNodes = document.getElementsByTagName('template');
53+
function handleExistingNodes(target /*: HTMLElement */) {
54+
const existingNodes = target.querySelectorAll('template');
5355
for (let i = 0; i < existingNodes.length; i++) {
5456
handleNode(existingNodes[i]);
5557
}
@@ -60,8 +62,8 @@ function installFizzInstrObserver(target /*: Node */) {
6062
for (let i = 0; i < mutations.length; i++) {
6163
const addedNodes = mutations[i].addedNodes;
6264
for (let j = 0; j < addedNodes.length; j++) {
63-
if (addedNodes.item(j).parentNode) {
64-
handleNode(addedNodes.item(j));
65+
if (addedNodes[j].parentNode) {
66+
handleNode(addedNodes[j]);
6567
}
6668
}
6769
}

0 commit comments

Comments
 (0)