-
-
Notifications
You must be signed in to change notification settings - Fork 183
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
Optimize usePrevious #115
Comments
Thank you very much for pointing this out. This is an interesting question, and I quickly watched the video and it seems to be related to the concurrent mode of react18. I know about concurrent mode, but I am not very familiar with this mode. |
It's late at night in my timezone right now, you can submit a PR or I'll change it tomorrow. |
So the traditional usePrevious hook worked because while useRef was updated, React didn't re-render - it just appeared to work correctly since the ref update wasn't triggering a new render cycle. I found this implementation quite elegant too. It demonstrates that you can achieve the same result without setState by using useRef: https://github.com/alibaba/hooks/blob/master/packages/hooks/src/usePrevious/index.ts |
Interesting
While this looks nice I think it violates the rule outlined in the pitfall section too, right? |
btw. alibaba has the same discussion 😂: alibaba/hooks#2162 |
You're right. I'll migrate to a pattern that doesn't rely on refs👍👍👍. |
Edit: nvm, it's only allowed for initialization. |
Yes. This is typically performancfe improve method. Often used with Intersetion Observer initialization or other thing |
I'm thinking about the implementation of useLatest - it writes to the ref during render. However, it seems difficult to simply migrate this operation to useEffect instead?: https://stackoverflow.com/questions/68512910/concurrent-safe-version-of-uselatest-in-react |
facebook/react#16956 (comment) also a good case. I decide to migrate to useLayoutEffect. |
So how would your other solution look like? |
just wrap with useIsomorphicLayoutEffect。 export const useLatest: UseLatest = <T>(value: T): MutableRefObject<T> => {
const ref = useRef(value)
useIsomorphicLayoutEffect(() => {
ref.current = value
}, [value])
return ref
} |
facebook/react#16956 (comment) As long as you don't pass the value to child components and use it in the child component's useLayoutEffect, there won't be any issues. I believe the chances of encountering this scenario are quite rare, and it's more acceptable compared to potential concurrent mode issues. |
Following this YT video: https://www.youtube.com/watch?v=B-Xb_8n5wRg the usePrevious hook probably should not use
useRef
but instead something closer to that implementation: https://github.com/uidotdev/usehooks/blob/90fbbb4cc085e74e50c36a62a5759a40c62bb98e/index.js#L1017-L1027Other libraries discuss this too as you can see here: streamich/react-use#2605
The text was updated successfully, but these errors were encountered: