-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[v5]: follow React "standard" way with breaking behavioral change #2395
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
Size Change: -90 B (-1%) Total Size: 6.19 kB
ℹ️ View Unchanged
|
const { bears, increasePopulation } = useBearStore( | ||
({ bears, increasePopulation }) => ({ | ||
bears, | ||
increasePopulation, | ||
}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say it worked before by chance. This selector creates a new object every time, which isn't supported in v5. We should use a memoized selector, zutand/traditional
or 👇 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or, was this test intentional to check the behavior with such selectors? @TkDodo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I think I copied this way if creating a store from somewhere. Definitely not intentional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for confirming. That was the reason I introduced useMemoSelector
, without looking at the test code. I believe it is an unnecessary hack.
@@ -474,7 +474,7 @@ it('can set the store without merging', () => { | |||
|
|||
it('only calls selectors when necessary', async () => { | |||
type State = { a: number; b: number } | |||
const useBoundStore = create<State>(() => ({ a: 0, b: 0 })) | |||
const useBoundStore = createWithEqualityFn<State>(() => ({ a: 0, b: 0 })) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This behavior (inline selector optimization) isn't supported in v5. Use zustand/traditional
.
While this change may surprise users, v5 should live long with React 19+, so I believe this is a good move. And, major bump is only the chance to make this kind of changes. |
For #2138
I think useMemoSelector is a hack that should be avoided. We should generally follow how React works. If we need previous behavior, we should use
zustand/traditional
which is based on a library provided by React, instead of adding our own hack.