-
Notifications
You must be signed in to change notification settings - Fork 7.9k
useEffectEvent revamp #8279
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
base: main
Are you sure you want to change the base?
useEffectEvent revamp #8279
Conversation
Size changesDetails📦 Next.js Bundle Analysis for react-devThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
|
I do not know if it is this page or the You Might Not Need an Effect page, but there should be somewhere that explains where you would and would not use useEffectEvent more explicitly. |
|
Here’s the original doc that spells out the motivation more clearly: https://react.dev/learn/separating-events-from-effects#declaring-an-effect-event I agree that this reference page should ideally convey this (but shorter). Reading it now as it is, I don’t have a clear sense of what problem it solves. I do get this sense from the original doc so the challenge is how to compress it. |
stephan-noel
left a comment
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.
Looks great. Just left 2 small things.
|
|
||
| #### Parameters {/*parameters*/} | ||
|
|
||
| * `callback`: A function containing the logic for your Effect Event. The function can accept any number of arguments and return any value. When you call the returned Effect Event function, the `callback` always accesses the latest values from props and state at the time of the call. |
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.
nit: I guess it's not only props and state, but anything from the surrounding closure? At least that's what I understood by looking at packages\react-reconciler\src\ReactFiberHooks.js and packages\react-reconciler\src\ReactFiberCommitWork.js in the main repo. Maybe most people won't read into it that much.
Preview
Used the new claude skills to revamp the useEffectEvent docs, then did a human pass.
If you're interested, the first commit is claude's attempt, and the second commit is my edits.
Here's the PR with the claude improvements this used: #8280