React DevTools recording commit without any component re-render

[kentcdodds]

I'm struggling to make an isolated example of this, but the app where I found this is pretty simple so hopefully it's not too challenging to track down.

So I was profiling https://the-react-bookshelf.netlify.com (locally) and got this when I clicked on the "login" button:

The fact that there was no profile data for a commit is interesting. Each commit should be associated to a state update somewhere in the tree, and wherever that happened should trigger at least one component to re-render, but that didn't appear to happen here.

I also verified that I don't have any components filtered out:

And I didn't filter any commits either:

Here's the exported profile data:

https://gist.github.com/kentcdodds/dbff66043653333cd22cb9261a08550b

And here's the repo where you can pull it down and reproduce yourself: https://github.com/kentcdodds/bookshelf. The component we're looking at is here: https://github.com/kentcdodds/bookshelf/blob/master/src/unauthenticated-app.js

Sorry I can't give a more direct reproduction.

[kentcdodds]

Made a smaller reproduction. I'm thinking that it has something to do with @reach/dialog: https://github.com/kentcdodds/devtools-renderless-profile

That has a profile in it as well. Basically the same thing, just a lot less code :)

[marvinhagemeister]

Noticed a similar thing past week in a project at work with react-dnd. There the ghosting element that follows the cursor while dragging renders on each dragover event. But because nothing has changed (or maybe everything is memoized, haven't looked in depth) the devtools gray out all components.

[kentcdodds]

I just noticed this same behavior in the redux Todo app example: https://codesandbox.io/s/github/reduxjs/redux/tree/master/examples/todos

[bvaughn]

I think the reason the linked sandbox shows a cascading commit is that discrete DOM events (like "submit") flush updates in two passes. I was mistaken. See this comment instead.

It just so happens that one of these passes bails out without doing any actual work, but React still calls the DevTools hook and the Profiler isn't "smart" enough to ignore the no-work one. Maybe it should be?

[kentcdodds]

Thanks for digging into that Brian!

Maybe it should be?

I think that it should, for a few reasons:

1. It's confusing
2. It could cause people to try to optimize something that may not need optimizing.
3. It's extra noise and isn't very useful because it's not actionable.

I think that it should exclude those kinds of commits. Maybe making it configurable would work, though I'm not sure I can think of a situation where seeing a commit was bailed would be helpful.

[bvaughn]

I think that it should exclude those kinds of commits. Maybe making it configurable would work, though I'm not sure I can think of a situation where seeing a commit was bailed would be helpful.

It is already configurable, FWIW, and maybe I should lean on this a little harder with a default configuration (would need more thought). Profiler lets you hide commits below a certain threshold:

In general, let me think about this a little more. I'm not really familiar with Redux / react-redux, and I think it's doing something here to trigger this issue. (It wouldn't happen from a "vanilla" React form+state.)

[kentcdodds]

I've noticed it come up in smaller examples in my workshops. Next time I come across it, I'll try to make an even smaller repro. Thanks!

[bvaughn]

That would be helpful, thanks.

[bvaughn]

I may have misspoke earlier by saying that the "submit" event was the cause of this. Looking again, it looks like the second update is being triggered by another event type (e.g. "blur", "focus", "keydown" - whatever happens to dispatch next after the "submit")

Not sure why this is. Nothing seems to be listening for that event type in this project, so it's maybe something React DOM is doing itself?

cc @trueadm @necolas since you two have a lot more insight into the event system than I do 😄

[trueadm]

Looking into this and it seems this might be occurring because ReactDOM's event plugins, that provide functionality to things like onChange, over-register (intentionally) to events such as blur, focus and keydown even if you don't use those events directly in your code.

Furthermore, these events are all "discrete" events, which means they flush any prior work – likely causing multiple updates here:

https://github.com/facebook/react/blob/master/packages/react-dom/src/events/ReactDOMEventListener.js#L288-L291

However, we do have a slightly altered change with the React Flare flag enabled, specifically flushing doesn't occur for multiple discrete events of the same timeStamp:

https://github.com/facebook/react/blob/master/packages/legacy-events/ReactGenericBatching.js#L106-L127

@bvaughn this might fix this issue – please could you test with a build with the React Flare enabled?

If you don't use onChange, or onBeforeInput then maybe the event replaying logic might be responsible here? cc @sebmarkbage

[bvaughn]

However, we do have a slightly altered change with the React Flare flag enabled, specifically flushing doesn't occur for multiple discrete events of the same timeStamp:

I don't think that would apply since it's always the next event (e.g. "blur", "focus", "keydown") rather than the one that scheduled work originally ("submit"). So I think the timestamps wouldn't match.

If you don't use onChange, or onBeforeInput then maybe the event replaying logic might be responsible here?

FWIW the repro project I'm looking at is only using onSubmit:
https://codesandbox.io/s/xenodochial-worker-sh0ou

[trueadm]

@bvaughn > So I think the timestamps wouldn't match.

You'd be surprised. Most events all trigger at the same time for interactions, for example, when you click something there's about 10 events that fire – all with the same timeStamp. In this case, as it's a submit, they are likely to be different though. Worth trying out still?

[bvaughn]

I added a log of the event timeStamp and they are different, fwiw.