Skip to content
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

docs: prefer useState in createContext example #2995

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jroitgrund
Copy link

@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.

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.
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 Feb 4, 2025 11:22am

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
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
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
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)

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.

3 participants