DevTools: Fix or silence "cannot find fiber with ID" warnings when hovering

[bvaughn]

Sometimes I see "cannot find fiber with ID" when hovering over the tree. They are sporadic. I'll need to dig into why they happen. But we also need to look at our usage of warnings to make sure they're either actionable or gated by DEV/DEBUG.

---

Originally reported by @gaearon via bvaughn/react-devtools-experimental#166

[bvaughn]

Comment from @bvauhgn bvaughn/react-devtools-experimental#166 (comment)

---

I agree this is probably not ideal currently. What are "expected" edge cases and what are legit errors? I've tried to replace legit error cases with throw statements recently, but I understand that it's still not the clearest.

Some (most?) of the the remaining warnings are expected potential edge cases. A little more context...

For example, if an element is selected in the DevTools frontend, then it gets unmounted on the page/backend, we may temporarily see this warning when we query for its inspected props:
https://github.com/bvaughn/react-devtools-experimental/blob/20193f03c04971a14ee8a0b6755bdfee08b94ba3/src/backend/renderer.js#L1432-L1438

Similarly, since the Store is mutable, it's possible that a node will be removed while a render batch is in progress:
https://github.com/bvaughn/react-devtools-experimental/blob/85179fe4a7e7cb2e9d79733eec4839443393e7ba/src/devtools/views/Components/Element.js#L122-L128

[capaj]

I get quite a lot of them:

I have react "react": "^16.13.1"
and React Developer Tools
4.8.2 (7/15/2020)

Also it seems that quite big parts of my react component tree are not visible in the tree. I think this is certainly a valid bug.

[bvaughn]

Repro info would be helpful if you can provide it.

[ManzerHa-Kore]

I got this warning too after componentWillMount warning.
https://stackoverflow.com/questions/63576010/react-devtools-backend-js2273-could-not-find-fiber-with-id-55-reactjs

Using "react": "16.13.1"

[Amoodaa]

@bvaughn i can provide a repro, but it also may be part of my bad?
but I seriously dunno why its happening

[bvaughn]

@Amoodaa Share the repro and we'll see?

[Amoodaa]

im suspecting the lazy loading im using, maybe I'm overdoing it as well
will zip the repo and we'll talk it out

[Amoodaa]

i've sent an email with the repo and the env bc in happens to need to login

[bvaughn]

Great, thank you. It's Sunday here so I'm not going to dig into this until tomorrow at the earliest, but I will let you know what I find.

[bvaughn]

Still on my plate for this week but it's been a busy one so far. Probably going to be a few days. Thanks for your patience.

Tried to look into the repro today but the login info you sent me does not work.

[Amoodaa]

@briankung i sent you new credentials and an updated version of the repo

[bvaughn]

Ok thank you. I'll try to look again today.

[bvaughn]

I can repro with the latest example you've sent. First thing I tried doing was stripping things out of the app to make the repro case smaller, and I noticed that removing react-helmet-async "fixed" the problem. (Not sure why yet, still digging in.) Seems specific to the Helmet component and entering or leaving the Settings route.

This may have been a false positive. I think there's something about the timing here. If I wait too long (for the page to settle) in my reduced test case, then I can't repro.

[bvaughn]

Seems like I can further narrow it down to say that if I remove all of the Material components, my reduced repro stops failing as well. Leaving in e.g. ListItem is enough to repro the bug though.

[bvaughn]

Does indeed seem related to the Loadable approach you're using. (Edit: Specifically to the Lazy component.) If I remove all of the loadable wrappers, I can't repro the issue.

[Amoodaa]

tbh, this is my first time using lazy loading, and i can see now why its still not as "widely used", ill probably do a pass on the app, bc its having a real overhead on reloading/bootstrap time
and when i updated to react-scripts.4.0.0 and react 17.0.1, it started appearing, so, can you try downgrading to
4.0.0.next-98 and 16.13.1? and see what happens, this is info i should have passed earlier

[Amoodaa]

@bvaughn So this seems to becoming stale again until more repros appear,
Can i get a hint on how to fix my codebase then?

[bvaughn]

I was on PTO last week.

I don't have a suggestion for how to fix the issue yet. When I do, I'll share it. (Not sure when that will be though.)

[bvaughn]

I think I found the underlying cause of this bug. I think it's caused by a combination of the "legacy" ReactDOM.render() method and React.Lazy. In legacy render mode, when a Lazy component renders and suspends (because it hasn't loaded the inner component yet) the React tree gets committed in an inconsistent state (everything down to the suspended boundary, but not below). If the Lazy component (or the tree containing it) get removed quickly, before the inner component is loaded and committed, React will tell DevTools to unmount the Lazy component (that never actually mounted) and DevTools will thrown an error.

I think this bug can be caught in the following test.

Edit This test is actually not sufficient to catch the bug after closer examination, but I think it's still something along these lines.

async function fakeImport(result) {
  return { default: result };
}

const App = ({ renderChildren }) => {
  if (renderChildren) {
    return (
      <React.Suspense fallback="Loading...">
        <LazyComponent />
      </React.Suspense>
    );
  } else {
    return null;
  }
};

const LazyInnerComponent = () => null;
const LazyComponent = React.lazy(() => fakeImport(LazyInnerComponent));

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

// Render once to start fetching the lazy component
act(() => ReactDOM.render(<App renderChildren={true} />, container));

// Render again to unmount it before it finishes loading
act(() => ReactDOM.render(<App renderChildren={false} />, container));

[bvaughn]

I believe this issue was fixed via PR #20362. It has now been published as v4.10.1 and should be available shortly. Thank you!