Cache inspected element data until it is updated #289
Conversation
e0ed2ee to
efc2f82
Compare
| return id; | ||
| } | ||
|
|
||
| mostRecentlyInspectedElementID = id; |
There was a problem hiding this comment.
This mostRecentlyInspectedElementID assumes that there's only one most recently inspected element, but (in my experience) during most debugging sessions more than one element gets inspected, the person who does the debugging jumps from element to element to find the issue, so this optimization, although helpful when sitting on one element, is invalidated for the usual debugging sessions. Of course I've got no data to prove that.
There was a problem hiding this comment.
I don't think I agree with the idea that it's "invalid" for a typical debugging session. The problem this PR was addressing was that we polled a component once a second, which felt like an infinite loop.
We could cache data for the most recent N elements, although then our invalidation check would become more expensive. I suspect this fix is sufficient but we can revisit if not.
There was a problem hiding this comment.
I guess "invalidated" was, again, a too-strong word (but I don't know other words for this thought, recommendations?).
It's great that it solves/replaces polling, so please do not consider my comments as de-valueing the work done here.
As we do not have real data, we might only guess how this will perform. I had no chance to test this yet in the same scenario where I struggled with breakpoints hit by polling (not every day gives problems that require such debugging), so I expressed general concerns about the approach.
There was a problem hiding this comment.
Cool, cool. Let's talk again when you've had a chance to evaluate the change in the context of debugging with a breakpoint, etc. (also keeping in mind that #15726 may improve the experience further).
I think this change is not sufficient by itself, but it does get us incrementally closer.
This way we can avoid unnecessarily re-rendering function components with hooks (which can be annoying if there are breakpoints or console logs in those functions).
This is meant to help with issues like facebook/react-devtools#1215 (comment) and hopefully also (to a lesser extent) #195 .