[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); }); ```
mofeiZ committed
Mar 4, 2023 at 11:26 UTC
03462cfc7a96f399d79010f1986c0abdbe0bbd15
1 file changed
+29
-27
packages/react-dom-bindings/src/server/ReactDOMServerExternalRuntime.js
+29
-27
@@ -21,35 +21,37 @@ if (!window.$RC) {
21
window.$RM = new Map();
22
}
23
24
-if (document.readyState === 'loading') {
25
- if (document.body != null) {
24
+if (document.body != null) {
25
+ if (document.readyState === 'loading') {
26
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});
27
}
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 */));
42
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
+}
52
51
-function handleExistingNodes() {
52
- const existingNodes = document.getElementsByTagName('template');
53
+function handleExistingNodes(target /*: HTMLElement */) {
54
+ const existingNodes = target.querySelectorAll('template');
55
for (let i = 0; i < existingNodes.length; i++) {
56
handleNode(existingNodes[i]);
57
}
@@ -60,8 +62,8 @@ function installFizzInstrObserver(target /*: Node */) {
62
for (let i = 0; i < mutations.length; i++) {
63
const addedNodes = mutations[i].addedNodes;
64
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]);
67
}
68
}
69
}