More accurate 'why did this render' wording #17068
Conversation
| @@ -167,7 +167,7 @@ function WhatChanged({ | |||
| if (changeDescription.didHooksChange) { | |||
| changes.push( | |||
| <div key="hooks" className={styles.WhatChangedItem}> | |||
| • Hooks changed | |||
| • Internal state changed | |||
There was a problem hiding this comment.
Slight preference for "hooks" since people know what that means at least. "Internal state" sounds like it might refer to React internals or something else.
There was a problem hiding this comment.
What about just "State changed"? What I found confusing was that changes due to useContext won't show up as "Hooks changed".
There was a problem hiding this comment.
Yeah, that's understandable. Most hooks aren't stateful in a way that triggers re-renders.
I'm not sure if "state changed" is better than "hooks changed" to be honest.
| @@ -207,7 +207,7 @@ function WhatChanged({ | |||
| if (changes.length === 0) { | |||
| changes.push( | |||
| <div key="nothing" className={styles.WhatChangedItem}> | |||
| The parent component rendered. | |||
| The parent component rendered or context changed | |||
There was a problem hiding this comment.
FWIW, we can detect a context change for class components, so we probably only want to show this additional warning if we're inspecting a function component.
There was a problem hiding this comment.
Sure, how do I detect that situation?
BTW, can't we also detect a context change for function components, simply by the process of elimination? If it's not due to state or prop changes and it's memoized, then it must be due to context changes, right?
There was a problem hiding this comment.
Sure, how do I detect that situation?
You can use the type attribute to differentiate between a class component (ElementTypeClass) and a function component (ElementTypeFunction, ElementTypeMemo, or ElementTypeForwardRef).
BTW, can't we also detect a context change for function components, simply by the process of elimination? If it's not due to state or prop changes and it's memoized, then it must be due to context changes, right?
I think that may be a shaky assumption.
Currently the only two hooks that directly trigger a re-render are useReducer and useState (which is built on top of useReducer). The rest of them are either reactive or at least indirect (like useContext). I'm not sure if this will always be the case though.
So I guess we could show a more meaningful message if memoized props and state were shallowly equal and element type was ElementTypeMemo but this feels fragile and wouldn't cover some more ambiguous cases. If we show a more specific message in some cases, but not in others- that alone could cause confusion. ("It's not a context change, because if it would DevTools would say so like it did before.")
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution. |
|
Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you! |
After using the new devtools profiler (which is awesome btw!) I discovered that one of my components was rerendering because "the parent component rendered", despite being
React.memoed and taking no props, which seemed like an impossibility (see twitter thread documenting my confusion). I eventually discovered that it was in fact updating due to context changes, and @bvaughn helpfully pointed out that the message was a fallback used when the component rerendered due to reasons other than state and props changing.This PR attempts to make the wording a little more accurate: in addition to adding the "or context changed" caveat, it seems like "Internal state changed" is a more accurate description than "Hooks changed", because updates due to the
useContexthook don't fall in that category. Ideally in the special case where the component is memoized the message could definitively say the update was due to a context change, but I'm not sure how to detect that situation 🙂