Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error: "Commit tree already contains fiber "19587". This is a bug in React DevTools." #18859

Closed
phoenisx opened this issue May 7, 2020 · 12 comments · Fixed by #21432
Closed

Comments

@phoenisx
Copy link

phoenisx commented May 7, 2020

Describe what you were doing when the bug occurred:

  1. I did profiling on a list, that gets updated on each pagination api call.
  2. Once the profiling was done, I moved around in the Profiler to view the Flamegraph
  3. Moving to second capture, the Profiler crashed.

DevTools version: 4.6.0-6cceaeb67

Component stack: in ec
    in div
    in div
    in div
    in So
    in Unknown
    in n
    in Unknown
    in div
    in div
    in rl
    in Ze
    in fn
    in Ga
    in _s
@bvaughn
Copy link
Contributor

bvaughn commented May 7, 2020

What does this mean?

Moving to second capture, the Profiler crashed.

Can you repro this bug? And if so, could you share steps for repro with us?

@phoenisx
Copy link
Author

phoenisx commented May 8, 2020

Sorry for not providing enough details, but even I don't have enough details. This error just popped up when I was profiling my app in React Devtools.

I was profiling a normal (ul) list, and it broke when I clicked next button on DevTools, to see the recorded profile.

Screenshot 2020-05-08 at 11 12 50 AM

About having a repro, I couldn't replicate it myself after that one occurrence, again.

Anyways, my assumption is (it could be wrong), one of the list item in the flame graph is getting duplicated somehow, either due to the app it was profiling or due to something else.

@bvaughn
Copy link
Contributor

bvaughn commented May 8, 2020

Okay. Thanks for clarifying.

@irohitb
Copy link

irohitb commented May 14, 2020

@bvaughn If no-one is working on this, I would like to assign myself to investigate and fix the issue (since multiple people have referenced it). Also, If you assign me, Can you please also help me with where to look?

@bvaughn
Copy link
Contributor

bvaughn commented May 14, 2020

@irohitb If you'd like to dig into this, we'd be happy to have the help. I don't really have the bandwidth to guide you on it though.

Some places you'd want to look at for starters:

And for a general overview of how DevTools works, you can check out this: https://github.com/facebook/react/blob/master/packages/react-devtools/OVERVIEW.md

@bvaughn
Copy link
Contributor

bvaughn commented Apr 28, 2021

@phoenisx This error remains the highest reported DevTools bug but we still don't have a repro case so that I can fix it. Do you remember how to repro the error you saw above? Can you share the steps with me if so? It would be very helpful! Thank you 🙇🏼

@phoenisx
Copy link
Author

Hi @bvaughn

Will try to replicate it again... It's been a long time since I used the Profiler, so I don't remember much. 👍

@bvaughn
Copy link
Contributor

bvaughn commented Apr 29, 2021

Okay, understood! Either way, thanks for checking.

@bvaughn
Copy link
Contributor

bvaughn commented May 1, 2021

I've got a repro for this over on #21402: https://github.com/web-reprex/prettier-list-different

When I run this project, I see an error in DevTools:

Error: Cannot remove node "25" because no matching node was found in the Store.

This is very promising!

Looks like there's a problem with Suspense and LegacyHidden.

@bvaughn
Copy link
Contributor

bvaughn commented May 4, 2021

I think the case I'm seeing is this:

  1. During initial render, a Suspense component is mounted.
  2. It has an Offscreen child, b'c of how Suspense is implemented internally, but DevTools hides this implementation detail to avoid confusion. (What this actually means is that the DevTools backend never even tells the frontend that the Offscreen component exists.)
  3. During login, stuff is updated and for some reason, the backend thinks it needs to unmount the Offscreen component (even though the frontend never knew about it). This causes an error to be thrown.

Unfortunately this thrown error is not surfaced in the UI so things go on until later, when it causes a downstream error in the Profiler.

First step is making that first error visible (done via #21426). Second step is to figure out why it's happening.

Interestingly, if I trim down the repro a https://github.com/web-reprex/prettier-list-different to the minimal repro case, then copy that into the React DevTools test harness (with a bleeding edge build of react and react-dom) the bug no longer happens. 🤔


Edit 1: If I copy it instead into a project created by create-react-app – with the same major versions of react and react-dom, I can repro the bug. So that's good. Seems like something may have changed on the React side between 17.0.2 and HEAD?


Edit 2: One (possibly) significant difference between the two versions is that 17 uses the LegacyHidden component beneath Suspense but HEAD uses the newer Offscreen component. I can also repro the same error in the DevTools test shell by swapping Offscreen with LegacyHidden so... 👍🏼

@bvaughn
Copy link
Contributor

bvaughn commented May 4, 2021

Oooh looks like this may have just been an oversight in our filtering logic. (We filter Offscreen but not LegacyHidden components.

Eventually we'll not want to filter Offscreen but for now, that seems an easy fix.

@bvaughn
Copy link
Contributor

bvaughn commented May 5, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@bvaughn @phoenisx @irohitb and others