Data: Warn if the 'useSelect' hook returns different values when called with the same state and parameters #53666
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Mamaduka
requested review from
gziolo,
jsnajdr,
noahtallen,
ntsekouras,
tyxla and
youknowriad
Mamaduka
added
[Type] Enhancement
|
Size Change: +354 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
This was referenced Aug 15, 2023
Mamaduka
force-pushed
the
try/use-select-stable-reference-check
branch
from
0600677 to
39b92c6
Compare
Member
Author
|
Okay, I think this is ready for initial review and testing. |
jsnajdr
reviewed
Aug 15, 2023
Member
jsnajdr
left a comment
jsnajdr
left a comment
There was a problem hiding this comment.
Yes, this looks good. Let me know what you think about the feedback, then we can merge.
Mamaduka
changed the title
Member
|
Also changelog entry would be nice. |
Member
Author
|
I'll add the changelog update. Should I keep PR open while fixing remaining core "violations"? The good news is that I've spotted at most ten. More general question: Would this enhancement be beneficial for third-party consumers? |
Member
|
I'd prefer merging immediately. It's OK to have console warnings for some time while violations are fixed. Third party consumers should benefit. Just like everybody benefits from React warnings that are very similar in spirit to this one. |
Member
Author
|
Sorry forgot to add a changelog entry before merging this 😓 - #53685. |
Contributor
|
Really cool addition 👍 |
3 tasks
7 tasks
Member
Author
|
I wonder if providing both Simple solution patch:diff --git packages/data/src/components/use-select/index.js packages/data/src/components/use-select/index.js
index 9afb4a6cfbd..3ff13229c47 100644
--- packages/data/src/components/use-select/index.js
+++ packages/data/src/components/use-select/index.js
@@ -161,7 +161,11 @@ function Store( registry, suspense ) {
if ( ! isShallowEqual( mapResult, secondMapResult ) ) {
// eslint-disable-next-line no-console
console.warn(
- `The 'useSelect' hook returns different values when called with the same state and parameters. This can lead to unnecessary rerenders.`
+ `The 'useSelect' hook returns different values when called with the same state and parameters. This can lead to unnecessary rerenders.`,
+ {
+ prev: mapResult,
+ next: secondMapResult,
+ }
);
didWarnUnstableReference = true;
}Screenshot
|
Member
Logging which keys actually fail the shallow equality check would me much more useful. When there are two non-identical objects that contain identical data (e.g., from two calls of a non-memoized selector), looking at them in the console doesn't help, because they look the same. If I want to find out what the problem really is, I need to create some custom logging code like: console.log( 'non-equal keys:', Object.keys( a ).filter( ( k ) => a[ k ] !== b[ k ] ) );Would be nice if this was built-in. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
See #53596 (comment).
PR adds a warning for
useSelecthooks that return a new object/array reference on eachmapSelecetcall. The warning will only be displayed in dev mode.Why?
This will help us to catch possible bugs like #53596 early in development.
How?
Call the
mapSelectfunction again and compare the returned results with the previous call.Testing Instructions
Example plugin
Screenshots or screencast