[react profiler bug]Error: "getCommitTree(): Unable to reconstruct tree for root "1" and commit 1"

[Zaynex]

---

Please do not remove the text below this line

DevTools version: 4.0.2-2bcc6c6

Call stack: at d (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:11:5744)
at e.getCommitTree (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:11:8526)
at Ai (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:56:274200)
at Ha (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:43:55890)
at bi (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:43:62939)
at Xl (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:43:99535)
at Hl (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:43:84255)
at Fl (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:43:81285)
at chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:43:25363
at n.unstable_runWithPriority (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:56:4368)

Component stack: in Ai
in div
in div
in div
in Or
in Unknown
in n
in Unknown
in div
in div
in Ua
in le
in ve
in ko
in Fl

[bvaughn]

Could you share repro steps for this please?

[raibima]

Got a similar error. It happens when you click on one of the components in the flame graph and toggle the filter by commit duration.

If you don't click the component first, the filter works as expected 👌

minimal repro: https://codesandbox.io/s/xenodochial-lamarr-q71wt

(Edit: I think it's a different one. I'll file a separate issue)
New issue: #16441

[Zaynex]

Could you share repro steps for this please?

I'am afraid not. That's my company project and I just start the react project and open react-dev-tools.

How about react team send the error stack error to some analyze website like sentry ?
I mean you can get the real error stack (not uglify codebase).

[gaearon]

@Zaynex It’s not the error stack that we’re looking for but the reproduction instructions.

[bvaughn]

Yup, something as detailed as @raibima's instructions above would be super helpful.

[bvaughn]

I've fixed the related issue, but I'm not sure how to repro this one yet so it's likely still broken. Still, might be worth checking 4.0.4 (will be uploading shortly) to see if it's fixed for you.

[Zaynex]

Click the next commit arrow -> cause error

[bvaughn]

Hm. Seems like something related to your project or setup/environment then. Just profiling and clicking the "->" arrow isn't sufficient to repro the error in my experience.

[bvaughn]

I finally saw this locally when profiling a react-window prototype.

It looks like the exported profiling session has only 33 saved mutations in the operations array, but it has 42 snapshots in the commitData array. Not sure how this happened.

Update 1

I'm able to reproduce this if I hammer my test app really hard while profiling. Seems like it may have something to do with a high rendering load.

Update 2

It looks like when I'm able to reproduce the problem- DevTools is reporting the following error the same number of times that the operations Array is off by:

Children cannot be added or removed during a reorder operation.

I'm also seeing "duplicate key warnings" in my main app. (Unsurprising since I've been playing around with key recycling.) Not sure how/if this is related yet though.

Update 3

Anecdotally it seems like fixing the duplicate key warning also fixes the DevTools warning and Profiler runtime error. Unfortunately I can't reproduce the Profiler error with something so simple as a project that intentionally triggers duplicate key warnings. (That isn't sufficient to cause the child reordering warning.)

[bvaughn]

I narrowed this down some more over the weekend. (This might not be the only source of the bug, it is the only reliable trigger I am aware of.) Anyway, when the bug manifests it follows this pattern.

First, (due to a bug in local code I was testing), we encounter a duplicate key warning (node id 197, key "6"):

Then on a subsequent update, during a re-order operation, one of the children is missing (id 197- the first node to use the duplicate key "6").

Digging into this a bit more, it looks like React actually doesn't notify DevTools of the that node being removed. (It doesn't call handleCommitFiberUnmount.) This seems like a React bug at the moment. The DevTools backend isn't stateful; it has no way to know about unmounts in most cases.

[bvaughn]

Knowing the above, I've been able to trap the bug within a small repro now:

function Foo() {
  return <div />
}

function App() {
  const [state, setState] = React.useState(false);
  const handleClick = () => setState(on => !on);
  return (
    <React.Fragment>
      <button onClick={handleClick}>{state ? 'on' : 'off'}</button>
      <div>
        {state
          ? [<Foo key="b" />, <Foo key="a" />, <Foo key="a" />]
          : [<Foo key="a" />, <Foo key="b" />]
        }
      </div>
    </React.Fragment>
  )
}

Clicking the button twice triggers the error. The first click causes the duplicate key warning and the second shows the unmount bug.

And this test case (added to store-test.js) triggers the same error:

it('should not break when duplicate key children are reordered', () => {
  const Child = () => null

  const container = document.createElement('div');

  act(() => ReactDOM.render(
    [<Child key="b" />, <Child key="a" />, <Child key="a" />],
    container,
  ));
  expect(store).toMatchSnapshot('1: mount with duplicate keys');

  act(() => ReactDOM.render(
    [<Child key="a" />, <Child key="b" />],
    container,
  ));
  expect(store).toMatchSnapshot('2: re-order');
});

Even without DevTools, React leaves the DOM in an invalid state (aab) after the following renders:

const Child = ({ label }) => <div>{label}</div>;

ReactDOM.render(
  [
    <Child key="b" label="b" />,
    <Child key="a" label="a" />,
    <Child key="a" label="a" />
  ],
  container
);

ReactDOM.render(
  [<Child key="a" label="a" />, <Child key="b" label="b" />],
  container
);

Here's a reduced test case (no DevTools) that shows the failure:

it("properly handles when duplicate key children are reordered", () => {
  const Child = ({ label }) => label;
  const Parent = ({ children }) => children;

  const container = document.createElement("div");
  expect(() => {
    ReactDOM.render(
      <Parent>
        {[
          <Child key="b" label="b" />,
          <Child key="a" label="a" />,
          <Child key="a" label="a" />
        ]}
      </Parent>,
      container
    );
  }).toWarnDev("Encountered two children with the same key, `a`.");
  expect(container.innerHTML).toBe("baa");

  ReactDOM.render(
    <Parent>
      {[<Child key="a" label="a" />, <Child key="b" label="b" />]}
    </Parent>,
    container
  );
  expect(container.innerHTML).toBe("ab"); // Currently fails with innerHTML "baa"
});

[bvaughn]

I think the bug is here:

react/packages/react-reconciler/src/ReactChildFiber.js

Lines 872 to 873 in 901139c

	// Add all children to a key map for quick lookups.
	const existingChildren = mapRemainingChildren(returnFiber, oldFiber);

(and so here as well):

react/packages/react-reconciler/src/ReactChildFiber.js

Lines 1061 to 1062 in 901139c

	// Add all children to a key map for quick lookups.
	const existingChildren = mapRemainingChildren(returnFiber, oldFiber);

Existing children can't be represented by a map when there are duplicate keys, which causes the logic below to skip over one of the duplicates.

[bvaughn]

After digging into this, I think the case above is not one that we will fix into React, because fixing it would significantly hurt performance for the common case (no duplicate keys, which is technically not supported anyway).

[bvaughn]

@Zaynex Could you confirm whether you see a duplicate key warning (or any other warnings) in the console around the time when you see the Profiler error?

[Zaynex]

@bvaughn I am sure there are no duplicate key warning. But a new warning was found.

backend.js:1 Uncaught RangeError: Invalid array length
    at He (backend.js:1)
    at backend.js:1
    at Set.forEach (<anonymous>)
    at Object.flushInitialOperations (backend.js:1)
    at backend.js:9
    at <anonymous>:1:225
    at Array.map (<anonymous>)
    at Object.emit (<anonymous>:1:202)
    at i (backend.js:9)
    at backend.js:9
    at Map.forEach (<anonymous>)
    at y (backend.js:9)
    at e (backend.js:1)
He @ backend.js:1
(anonymous) @ backend.js:1
flushInitialOperations @ backend.js:1
(anonymous) @ backend.js:9
(anonymous) @ VM1079:1
emit @ VM1079:1
i @ backend.js:9
(anonymous) @ backend.js:9
y @ backend.js:9
e @ backend.js:1
postMessage (async)
r @ contentScript.js:1
(anonymous) @ contentScript.js:1
setInterval (async)
108 @ contentScript.js:1
n @ contentScript.js:1
(anonymous) @ contentScript.js:1
(anonymous) @ contentScript.js:1

[bvaughn]

RE this comment

It looks like the exported profiling session has only 33 saved mutations in the operations array, but it has 42 snapshots in the commitData array. Not sure how this happened.

I believe my changes in PR #17771 should fix this mismatch and so, possible, fix the source of this bug. Let's keep an eye out once that fix has been released and see if we can still trigger this bug.