-
Notifications
You must be signed in to change notification settings - Fork 563
RFC: useKey
for resetting state as an alternative to the key
prop
#265
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?
Conversation
Hi @vezaynk! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
```tsx | ||
export default function ProfilePage({ userId }) { | ||
const [comment, setComment] = useState(''); | ||
// 🔴 Bad: call `useKey` with dynamic internal state is inappropriate |
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.
IMHO, new hook with new rule to use is not make any sense. We should avoid human error rather than create new potential problem.
Someone gonna write an article to recommend a best practice when working with useKey
like:
function ProfilePage({ userId }) {
useKey(userId)
return <Proflile userId={userId} />
}
It's just use key
prop 🤔
|
||
# Drawbacks | ||
|
||
- `useKey` is technically possible to call within other hooks. This could lead to library hooks hijacking the behavior of components and unintentionally resetting all state. |
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.
Yes, how can we debug when "something" reset my hooks/components state?
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.
Ideally, "something" should never do that. I say it's technically possible because you can't prevent functions from calling one another.
However, I think if React strictly enforces useKey
being above any other hook, this issue largely disappears.
|
||
The impact of not implementating the RFC would be that developers would continue to use the `key` prop in the parent component to reset the state of child components. This would lead to the same issues described in the motivation section, where the parent is responsible for managing the child's state reset behavior. In other words, the tail will keep wagging the dog. | ||
|
||
An alternative to this RFC, which can already be implemented in userland are HOCs or wrapper components that manage the `key` prop behavior. They can look like this: |
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 the ProfilePage take the userId as key as well.
function ParentOfProfilePage() {
return <ProfilePage userId={userId} orgId={orgId} key={[userId, orgId].join('-')} />
}
function ProfilePage({ userId, orgId }) {
const [comment, setComment] = useState('');
// ...
}
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.
Yes, but this still retains the main issue I perceive with the key
prop. I think it's backwards for ParentOfProfilePage
to tell ProfilePage
when it should construct a new instance instead of ProfilePage
directly owning it.
To me, this feels akin to if SQL databases inverted the concepts of primary keys and foreign keys.
|
||
1. It requires additional boilerplate code to create the HOC or wrapper component. | ||
2. There are a dozen subtly different ways to implement this behavior, which can lead to inconsistencies across codebases. | ||
3. It does not resolve the fundemental issue that the component itself should be responsible for managing its own re-rendering behavior. |
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.
The child always have ability to re-rendering itself.
When the key change, the component will be unmounted, then remounted as a fresh new. Its not re-rendering.
useEffect(() => {
return () => {} // <-- Unmount will trigger cleanup, re-render may not.
}, [])
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.
By "re-rendering behavior" I mean whether it goes through a rerender or a full remount. Both are a kind of "rerender", but the component itself is currently unable to dictate which one it should be.
Summary
React has a special prop called
key
that is used to distinguish between rendered instances of a component within the same parent. It's essential for mapping a data array to a component array.The
key
prop also has an application outside of mapped components. It can be used to force a component to re-render by changing thekey
prop value. This is useful when you want to reset the state of a component or trigger a re-render without changing the component's props.This technique is described in You Might Not Need an Effect.
However, writing code where the parent is responsible for the state reset is somewhat of a "tail wagging the dog" situation, as often the component itself is the one that knows best when it needs to reset its state. This RFC proposes to allow components to use the
key
prop behavior in a more flexible way, enabling them to control their own re-rendering behavior by introducing a new hook calleduseKey
.Link to full formatted RFC