-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Revamp useOptimistic docs #8264
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
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! 🙌 |
|
added some initial nits. examples add a lot of clarity, so great work on those. i'd just slightly trim everything and focus it down so each part explains one aspect |
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 like a really nice improvement. Just some small nits and questions, but didn't get to the Usage section yet. Will have more in depth look at that later if it's still open to try to give more meaningful feedback.
| #### Caveats {/*setoptimistic-caveats*/} | ||
| * The `set` function must be called inside a [Transition](/reference/react/useTransition) using [`startTransition`](/reference/react/startTransition) or inside an [Action](/reference/react/useTransition#perform-non-blocking-updates-with-actions). If you call the setter outside an Action or Transition, [React will show a warning](#an-optimistic-state-update-occurred-outside-a-transition-or-action) and the optimistic value will briefly render. |
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.
Similar to an above comment I made, the explicit mention of "Action or Transition" makes me feel as if I'm missing something, the argument to startTransition is named "action". Is there an some way of calling a setter in a transition that isn't an "action"?
nit:
An "of" might read a bit better in "If you call the setter outside an Action or Transition"
outside of an Action
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.
You're right - I'm including both because I worry that people are not familiar with the idea that Actions are just the callback inside Transitions, so it can either be an action prop, or wrapped in startTransition. Not sure how to clarify it enough for devs with less experience with Transitions/Actions.
| * `state`: The value returned when there are no pending Actions or Transitions. | ||
| * **optional** `reducer(currentState, optimisticValue)`: The reducer function that specifies how the optimistic state gets updated. It must be pure, should take the current state and the optimistic value as arguments, and should return the next optimistic 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.
When I read "state" the first thing that comes to my mind is either a prop or the value from useState. Then I think of the more abstract concept of "state".
II wonder if it would make more sense to name this parameter value instead of state. The same would apply to the first parameter of the reducer function.
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.
Not sure. State makes it clear that it re-renders. Fun fact, useOptimistic is short for useOptimisticState.
9e57f8c to
fbdc328
Compare
| <Note> | ||
| When using [Action props](/reference/react/useTransition#exposing-action-props-from-components), you can call the set function without `startTransition`: |
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.
This separation is nice, it clarifies the confusion about "Action vs Transition". Maybe it can be explained more like this in other parts of the docs.
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.
Where are you thinking?
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 other sections that are saying "Action or Transition" where there were some comments about confusion. Related to #8264 (comment)
| ); | ||
|
|
||
| function handleClick() { | ||
| const newFollowState = !optimisticState.isFollowing; |
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.
Can this logic also be handled inside the reducer function?
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.
You want to pass it in here because you're posting the value to the followAction endpoint. If you did it in the reducer, then if there was an update while this was in progress, it would toggle it back, causing it to flicker when the post finishes.
| #### Parameters {/*setoptimistic-parameters*/} | ||
| * `nextState`: The value that you want the optimistic state to be. If you provided a `reducer` to `useOptimistic`, this value will be passed as the second argument to your reducer. It can be a value of any type. |
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.
Above and elsewhere it's referred to as optimisticValue, but here it's named nextState. Is it better to stick to one? Maybe optimisticValue?
Same applies to below for the argument as function explainer.
| #### Parameters {/*setoptimistic-parameters*/} | ||
| * `nextState`: The value that you want the optimistic state to be. If you provided a `reducer` to `useOptimistic`, this value will be passed as the second argument to your reducer. It can be a value of any type. | ||
| * If you pass a function as `nextState`, it will be treated as an _updater function_. It must be pure, should take the pending state as its only argument, and should return the next state. React will put your updater function in a queue and re-render your component. During the next render, React will calculate the next state by applying the queued updaters to the previous state similar to [`useState` updaters](/reference/react/useState#setstate-parameters). |
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: Besides the naming of optimisticValue instead of nextState, when mentioning "next state" as separate words, is it more appropriate to say "optimistic state" instead?
For example, "should return the next state" makes sense for the useState updater function, but does "should return the optimistic state" make more sense here? Not sure if the fact of transitions being interruptible has any bearing here. Maybe that train of thought would get in the weeds with "next render" though.
| startTransition(async () => { | ||
| setOptimistic('b'); | ||
| setValue('b'); | ||
| }); |
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.
I think the simplicity does help digesting here. It just feels like something is missing with the transition being async without any corresponding await, especially with regards to point 4 below.
| 1. The <CodeStep step={2}>current state</CodeStep> of the optimistic value, returning the <CodeStep step={1}>state</CodeStep> provided. |
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 current state of the optimistic value" felt a bit hard to parse. It feels like saying the optimistic value is changing state, whereas before optimisticValue referred to the parameter of the set function.
Is this portion of the section like a summary of the signature so people don't have to scroll back up to the top?
| const newName = formData.get('name'); | ||
| setOptimisticName(newName); | ||
| const updatedName = await updateName(newName); | ||
| onSubmit(updatedName); |
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.
Just sharing how I was thinking about this example and questions I had.
I guess technically onSubmit could also be an action prop since it's also called inside of an action? I was trying to understand if leaving it as onSubmit was intentional to communicate that following the action prop naming convention is optional. On the other hand, if you don't communicate somehow that it's inside an action, I thought consumers could be confused about why the updates from their set functions get entangled? In this case, maybe it's fine to diverge since we usually expect submits to be async?
Update 1 hour later:
Just realized it's called inside of an action but because of the lack of async context thing, it's not really in a transition anymore, for now at least. That was pretty confusing 😄 . "Half-action prop".
Maybe worth calling this out explicitly? Feels like an action prop gotcha. Could also help keep track of what needs to change once async context lands.
| To show a pending state while an Action is running, use `useOptimistic` with a boolean. | ||
| Here's a button that shows "Submitting..." while the Action is pending using the [Action prop pattern](/reference/react/useTransition#exposing-action-props-from-components): |
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.
I feel like it would be even clearer to explicitly mention that useTransition could also be used here instead and that this example is just illustrating that useOptimistic could also be used for this purpose and it's just as "blessed".
A question that pops into my head is when to use which approach. Makes me thing that an example which is harder to replace with useTransition might help address both of these. If there is no such situation in which one is preferable, maybe mentioning that could be helpful.
| **Use reducers** when you need to pass data to the update (like which item to add) or when handling multiple types of updates with a single hook. | ||
| Both patterns ensure React can update your optimistic value if the state changes while your Transition is pending. | ||
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.
Maybe this deep dive is better placed after the later examples that illustrate when to use a reducer? Maybe it's even is better as a kind of summary of heuristics based on the previous examples.
At least when reading top to bottom, I started reading the deep dive and couldn't make sense of it until the later examples.
| </Sandpack> | ||
| The `reducer` receives the current list of todos and the new todo to add. This is important because if the `todos` prop changes while your add is pending (for example, another user added a todo), React will update your optimistic value by re-running the reducer with the updated list. This ensures your new todo is added to the latest list, not an outdated copy. | ||
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.
I feel like here I finally understood why you need to use a reducer, because the updater function, or set function with optimisticValue would not be re-run when the state argument changes. It feels like it's buried being here a bit. Maybe including this explanation in the deep dive/summary as well would be useful.
As a side note, it does make me wonder about highlighting that reducer is optional, supporting only reducers which always work correctly instead of a kind of hybrid set function/reducer as well for convenience does feel like it's a wider/heavier/more error-prone api surface area for little gain. That ship has probably sailed though.
| --- | ||
| ### Optimistic delete with error recovery {/*optimistic-delete-with-error-recovery*/} |
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.
Taking from the WG comment you made:
Handling errors / mismatched: If an action errors, or the value resolves to a different value than the optimistic one (like the value from the server has changed from a different user or tab) the optimistic value will automatically commit the newest actual value passed in.
The reducer part is more for updates to the state parameter during a transition, but in this comment you meant a mismatch in when the server resolves to a different value right? Did you still want to include the mismatch case in here similar to how the error case is covered?
Preview
Fixes #7737
Fixes #7379
Fixes #7098
Fixes #6926