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

Try useOptimistic with useFormState and progressive enhancement #38

Closed
wants to merge 6 commits into from

Conversation

unstubbable
Copy link
Owner

@unstubbable unstubbable commented Jan 12, 2024

No description provided.

Copy link

vercel bot commented Jan 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
mfng ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 12, 2024 6:57pm

This breaks progressive enhancement, and the `prevState` in the reducer
is always `undefined`. Therefore, only the quantity gets rendered
optimistically, not the total quantity.
The `prevResult` in the `useOptimistic` reducer ist still `undefined` though.

React.startTransition(() => {
setOptimisticResult(parseInt(formData.get(`quantity`) as string, 10));
formAction(formData);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should await this 😄

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to use the hook version so that this transition is attached to the component, and then await the action. Otherwise things like errors from the action won't be surfaced.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can't be awaited, a formAction returns void, not Promise<void>.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good point!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to use the hook version so that this transition is attached to the component, and then await the action. Otherwise things like errors from the action won't be surfaced.

How can I verify this statement? Using React.startTransition, at least the component stack looks fine when forcing a network error:
Screenshot 2024-01-12 at 20 00 54

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tom-sherman Sounds to me like the error behavior you've described here, which I couldn't reproduce, is addressed in facebook/react#28111. Am I interpreting this correctly?

Moving the state update from the reducer to the dispatch function still does not give me the previous state, though.
@@ -15,10 +15,29 @@ export interface ProductProps {
}

export function Product({buy}: ProductProps): JSX.Element {
const [result, formAction] = ReactDOM.useFormState(buy, undefined);
const [formState, formAction] = ReactDOM.useFormState(buy, undefined);
const [result, setOptimisticResult] = React.useOptimistic(formState);
Copy link

@tom-sherman tom-sherman Jan 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird to me to pass the form state to useOptimistic. This isn't actually what you want to optimistically update, that is actually the product quantity.

Instead of returning the quantity from the mutation I think the recommended pattern is to fetch this in the server component and pass that to useOptimistic.

In other words: useOptimistic is for values, useFormState is for tracking the state of the action.

Copy link
Owner Author

@unstubbable unstubbable Jan 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, you might have a point here. In a real-world example I would probably render a summary page based on the backend data (bought products) that I fetch in the server component, and not render the result of the buy action. But imagine this being an add-to-cart action instead of the actual buy action, and I just want to show a success or error toast. Wouldn't you base this on the result of the server action?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That toast use case doesn't need to be both progressively enhanced and updated optimistically, so it's hard to say. Feels very different to what you're going for here.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely want to show a toast optimistically. And I would also appreciate it if the toast was shown even when the user clicked the button before the app was hydrated. Wouldn't you agree?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that depends. In my opinion if you want to update something that you fetch from the server initially - passing formState to useOptimistic can be weird. But something like implementing form with lists of items in PE manner, then passing formState to useOptimistic can make it more responsive. Generally speaking, you can pass anything to useOptimistic 😄

@unstubbable unstubbable changed the title Try useOptimistic Try useOptimistic with useFormState and progressive enhancement Jan 12, 2024
@unstubbable unstubbable closed this Apr 9, 2024
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