Skip to content

Conversation

@jroitgrund
Copy link
Contributor

@jroitgrund jroitgrund commented Feb 4, 2025

Related Bug Reports or Discussions

N/A

Summary

This PR edits the 'context usage' example from the docs to use state, rather than a ref, to hold the zustand store.
This PR edits the 'context usage' example from the docs to use a strict mull check, rather than !ref.current, so that the react compiler recognizes the ref is immutable and optimizes the component.

Both useState and useRef are correct in practice: useState will only render once since we never set the state after initialization, and useRef is safe to use since the component never needs to rerender when the ref value changes.

However, lint rules will flag useRef as a mistake, because we are passing a ref value to a child component, which in general can cause stale components since changing refs doesn't trigger re-renders.

Check List

  • pnpm run fix:format for formatting code and docs

Both useState and useRef are correct in practice: useState will only render once since we never set the state after initializion, and useRef is safe to use since the component never needs to rerender when the ref value changes.

However, lint rules will flag useRef as a mistake, because we are passing a ref value to a child component, which in general can cause stale components since changing refs doesn't trigger re-renders.
@vercel
Copy link

vercel bot commented Feb 4, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
zustand-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 20, 2025 0:08am

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 4, 2025

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.

@dai-shi
Copy link
Member

dai-shi commented Feb 4, 2025

Is it eslint-plugin-react-hooks or eslint-plugin-react-compiler?
I heard that useRef for lazy initialization will be supported by the compiler.

@jroitgrund
Copy link
Contributor Author

It's react-compiler

│   62:39  error  Ref values (the `current` property) may not be accessed during render.
│ (https://react.dev/reference/react/useRef)  react-compiler/react-compiler

@dai-shi
Copy link
Member

dai-shi commented Feb 4, 2025

Can you look for official info from the compiler team that recommends useState over useRef for lazy initialization?

@jroitgrund
Copy link
Contributor Author

jroitgrund commented Feb 4, 2025

I'd say it falls under https://react.dev/learn/referencing-values-with-refs#differences-between-refs-and-state

You shouldn’t read (or write) the current value during rendering.

I think they didn't want to special case 'useRef but you never update .current', and they would rather recommend 'useState without ever callling the setter' for immutable data which is used during render.

@dai-shi
Copy link
Member

dai-shi commented Feb 4, 2025

I would like to see if it's officially unsupported with the compiler. Maybe we should wait for the compiler to be out of the experimental phase.

@jroitgrund
Copy link
Contributor Author

Looks like it's being discussed, I asked a question here: facebook/react#30843 (comment)

@josephsavona
Copy link

The compiler does support this pattern, but you have to specifically check that against null, ie if (ref.current == null) { /* lazy init */ }

@dai-shi
Copy link
Member

dai-shi commented Feb 5, 2025

The compiler does support this pattern, but you have to specifically check that against null, ie if (ref.current == null) { /* lazy init */ }

Cool. Does it work with === undefined too? (Or, === null if it's initialized with null)

@404-ahmad-404
Copy link

Yes, that's right

@dai-shi
Copy link
Member

dai-shi commented Mar 20, 2025

Does it work with === undefined too? (Or, === null if it's initialized with null)

As I tried in the playground, == null and === null works, but === undefined does not.

@jroitgrund
Copy link
Contributor Author

jroitgrund commented Mar 20, 2025

Happy to make this change, but curious as to the rationale. Do refs provide a benefit over state? state provides:

  • a built-in API for lazy initialisation, rather than a manual check
  • the 'correct' way to consume values at render-time (see docs: strict null check in createContext example #2995 (comment)). Concretely, this means that it's clearly supported as per the docs, rather than, it's recommended not to do this, but a dev told is it was OK (as long as we use a very specific incantation!)

@dai-shi
Copy link
Member

dai-shi commented Mar 20, 2025

The rationale is that useRef should be more lightweight than useState. We don't need "state" that changes over time, but just a stable reference.

Having that said, I'm fine to recommend useState as best practice with React Compiler. I think there should be a dedicated guide, not in docs/previous-versions/zustand-v3-create-context.md.
This document is about the migration from v3, so I would like to avoid changes. So, for now, I would like to change it as #2995 (comment). Please note that I may change my mind in the future.

@jroitgrund jroitgrund changed the title docs: prefer useState in createContext example docs: strict null check in createContext example Mar 20, 2025
@jroitgrund
Copy link
Contributor Author

jroitgrund commented Mar 20, 2025

Makes sense! Pushed, edited description + title.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 20, 2025

Open in Stackblitzdemostarter

npm i https://pkg.pr.new/zustand@2995

commit: 3a258df

Copy link
Member

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dai-shi dai-shi merged commit 17e281f into pmndrs:main Mar 20, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants