Warn if optimistic state is updated outside of a transition #27454

acdlite · 2023-10-03T18:47:37Z

Based on #27453

If optimistic state is updated, and there's no startTransition on the stack, there are two likely scenarios.

One possibility is that the optimistic update is triggered by a regular event handler (e.g. onSubmit) instead of an action. This is a mistake and we will warn.

The other possibility is the optimistic update is inside an async action, but after an await. In this case, we can make it "just work" by associating the optimistic update with the pending async action.

Technically it's possible that the optimistic update is unrelated to the pending action, but we don't have a way of knowing this for sure because browsers currently do not provide a way to track async scope. (The AsyncContext proposal, if it lands, will solve this in the future.) However, this is no different than the problem of unrelated transitions being grouped together — it's not wrong per se, but it's not ideal.

Once AsyncContext starts landing in browsers, we will provide better warnings in development for these cases.

react-sizebot · 2023-10-03T18:51:31Z

Comparing: 85c2b51...d4c4b14

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	167.58 kB	167.58 kB	=	52.14 kB	52.14 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	176.22 kB	176.22 kB	=	54.84 kB	54.83 kB
facebook-www/ReactDOM-prod.classic.js	+0.02%	564.39 kB	564.51 kB	+0.01%	99.37 kB	99.38 kB
facebook-www/ReactDOM-prod.modern.js	+0.02%	548.12 kB	548.23 kB	+0.02%	96.45 kB	96.46 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against d4c4b14

sophiebits · 2023-10-03T19:04:08Z

A warning is good but does this mean if you call set outside a transition then that update will stick around forever? Can we not make it so that it's never applied / immediately reverted? I worry people might build the wrong mental model otherwise.

acdlite · 2023-10-03T19:07:12Z

A warning is good but does this mean if you call set outside a transition then that update will stick around forever? Can we not make it so that it's never applied / immediately reverted? I worry people might build the wrong mental model otherwise.

Yeah it does get immediately reverted... now that #27453 landed :D That's how I noticed it

react/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js

Lines 1223 to 1224 in ba826ab

    
           assertLog(['Loading... (25%)', 'A', 'B']); 
        
           expect(root).toMatchRenderedOutput(<div>B</div>);

acdlite · 2023-10-03T19:08:36Z

there is a failing test related to when enableAsyncActions is disabled but it's caused by something else, I'll fix after this meeting

sophiebits · 2023-10-03T19:13:00Z

packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js

+
+      return (
+        <>
+          {loadingProgress !== 0 ? (


Maybe make this unconditional so it’s easier to witness that the state gets set back? Right now the test logs sorta make it look like it gets stuck at 25.

Wait never mind I'm silly. The toMatchRenderedOutput is plenty.

If optimistic state is updated, and there's no startTransition on the stack, there are two likely scenarios. One possibility is that the optimistic update is triggered by a regular event handler (e.g. `onSubmit`) instead of an action. This is a mistake and we will warn. The other possibility is the optimistic update is inside an async action, but after an `await`. In this case, we can make it "just work" by associating the optimistic update with the pending async action. Technically it's possible that the optimistic update is unrelated to the pending action, but we don't have a way of knowing this for sure because browsers currently do not provide a way to track async scope. (The AsyncContext proposal, if it lands, will solve this in the future.) However, this is no different than the problem of unrelated transitions being grouped together — it's not wrong per se, but it's not ideal. Once AsyncContext starts landing in browsers, we will provide better warnings in development for these cases.

### Based on #27453 If optimistic state is updated, and there's no startTransition on the stack, there are two likely scenarios. One possibility is that the optimistic update is triggered by a regular event handler (e.g. `onSubmit`) instead of an action. This is a mistake and we will warn. The other possibility is the optimistic update is inside an async action, but after an `await`. In this case, we can make it "just work" by associating the optimistic update with the pending async action. Technically it's possible that the optimistic update is unrelated to the pending action, but we don't have a way of knowing this for sure because browsers currently do not provide a way to track async scope. (The AsyncContext proposal, if it lands, will solve this in the future.) However, this is no different than the problem of unrelated transitions being grouped together — it's not wrong per se, but it's not ideal. Once AsyncContext starts landing in browsers, we will provide better warnings in development for these cases. DiffTrain build for [88d56b8](88d56b8)

…#27454) ### Based on facebook#27453 If optimistic state is updated, and there's no startTransition on the stack, there are two likely scenarios. One possibility is that the optimistic update is triggered by a regular event handler (e.g. `onSubmit`) instead of an action. This is a mistake and we will warn. The other possibility is the optimistic update is inside an async action, but after an `await`. In this case, we can make it "just work" by associating the optimistic update with the pending async action. Technically it's possible that the optimistic update is unrelated to the pending action, but we don't have a way of knowing this for sure because browsers currently do not provide a way to track async scope. (The AsyncContext proposal, if it lands, will solve this in the future.) However, this is no different than the problem of unrelated transitions being grouped together — it's not wrong per se, but it's not ideal. Once AsyncContext starts landing in browsers, we will provide better warnings in development for these cases.

…experimental prefix for server action APIs (#56809) The latest React canary builds have a few changes that need to be adopted for compatability. 1. the `useFormState` and `useFormStatus` hooks in `react-dom` and the `formData` opiont in `react-dom/server` are no longer prefixed with `experimental_` 2. server content (an undocumented React feature) has been removed. Next only had trivial intenral use of this API and did not expose a coherent feature to Next users (no ability to seed context on refetches). It is still possible that some users used the React server context APIs which is why this should go into Next 14. ### React upstream changes - facebook/react#27513 - facebook/react#27514 - facebook/react#27511 - facebook/react#27508 - facebook/react#27502 - facebook/react#27474 - facebook/react#26789 - facebook/react#27500 - facebook/react#27488 - facebook/react#27458 - facebook/react#27471 - facebook/react#27470 - facebook/react#27464 - facebook/react#27456 - facebook/react#27462 - facebook/react#27461 - facebook/react#27460 - facebook/react#27459 - facebook/react#27454 - facebook/react#27457 - facebook/react#27453 - facebook/react#27401 - facebook/react#27443 - facebook/react#27445 - facebook/react#27364 - facebook/react#27440 - facebook/react#27436 --------- Co-authored-by: Zack Tanner <zacktanner@gmail.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: Jiachi Liu <inbox@huozhi.im>

…#27454) ### Based on facebook#27453 If optimistic state is updated, and there's no startTransition on the stack, there are two likely scenarios. One possibility is that the optimistic update is triggered by a regular event handler (e.g. `onSubmit`) instead of an action. This is a mistake and we will warn. The other possibility is the optimistic update is inside an async action, but after an `await`. In this case, we can make it "just work" by associating the optimistic update with the pending async action. Technically it's possible that the optimistic update is unrelated to the pending action, but we don't have a way of knowing this for sure because browsers currently do not provide a way to track async scope. (The AsyncContext proposal, if it lands, will solve this in the future.) However, this is no different than the problem of unrelated transitions being grouped together — it's not wrong per se, but it's not ideal. Once AsyncContext starts landing in browsers, we will provide better warnings in development for these cases.

### Based on #27453 If optimistic state is updated, and there's no startTransition on the stack, there are two likely scenarios. One possibility is that the optimistic update is triggered by a regular event handler (e.g. `onSubmit`) instead of an action. This is a mistake and we will warn. The other possibility is the optimistic update is inside an async action, but after an `await`. In this case, we can make it "just work" by associating the optimistic update with the pending async action. Technically it's possible that the optimistic update is unrelated to the pending action, but we don't have a way of knowing this for sure because browsers currently do not provide a way to track async scope. (The AsyncContext proposal, if it lands, will solve this in the future.) However, this is no different than the problem of unrelated transitions being grouped together — it's not wrong per se, but it's not ideal. Once AsyncContext starts landing in browsers, we will provide better warnings in development for these cases. DiffTrain build for commit 88d56b8.

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Oct 3, 2023

acdlite requested review from sophiebits and sebmarkbage October 3, 2023 18:48

acdlite changed the title ~~Warn optimistic state outside transition~~ Warn if optimistic state is updated outside of a transition Oct 3, 2023

acdlite force-pushed the warn-optimistic-state-outside-transition branch from 3e5f86e to 5a5fbe6 Compare October 3, 2023 19:01

sebmarkbage approved these changes Oct 3, 2023

View reviewed changes

acdlite force-pushed the warn-optimistic-state-outside-transition branch from 5a5fbe6 to ba826ab Compare October 3, 2023 19:02

sophiebits reviewed Oct 3, 2023

View reviewed changes

sophiebits mentioned this pull request Oct 3, 2023

Add <form> Component reference documentation reactjs/react.dev#6312

Merged

acdlite force-pushed the warn-optimistic-state-outside-transition branch from ba826ab to d4c4b14 Compare October 4, 2023 14:52

acdlite merged commit 88d56b8 into facebook:main Oct 4, 2023
2 checks passed

gnoff mentioned this pull request Oct 4, 2023

Update React from d900fadbf to 6f1324395 vercel/next.js#56220

Closed

shuding mentioned this pull request Oct 11, 2023

Upgrade vendored React vercel/next.js#56705

Closed

gnoff mentioned this pull request Oct 13, 2023

Update React from d900fadbf to 09fbee89d. Removes server context and experimental prefix for server action APIs vercel/next.js#56809

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Warn if optimistic state is updated outside of a transition #27454

Warn if optimistic state is updated outside of a transition #27454

acdlite commented Oct 3, 2023

react-sizebot commented Oct 3, 2023 •

edited

Loading

sophiebits commented Oct 3, 2023

acdlite commented Oct 3, 2023

acdlite commented Oct 3, 2023

sophiebits Oct 3, 2023

sophiebits Oct 3, 2023

Warn if optimistic state is updated outside of a transition #27454

Warn if optimistic state is updated outside of a transition #27454

Conversation

acdlite commented Oct 3, 2023

Based on #27453

react-sizebot commented Oct 3, 2023 • edited Loading

Critical size changes

Significant size changes

sophiebits commented Oct 3, 2023

acdlite commented Oct 3, 2023

acdlite commented Oct 3, 2023

sophiebits Oct 3, 2023

Choose a reason for hiding this comment

sophiebits Oct 3, 2023

Choose a reason for hiding this comment

react-sizebot commented Oct 3, 2023 •

edited

Loading