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

Rethrow errors from form actions #26689

Merged
merged 2 commits into from
Apr 21, 2023

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Apr 21, 2023

This is the next step toward full support for async form actions.

Errors thrown inside form actions should cause the form to re-render and throw the error so it can be captured by an error boundary. The behavior is the same if the <form /> had an internal useTransition hook, which is pretty much exactly how we implement it, too.

The first time an action is called, the form's HostComponent is "upgraded" to become stateful, by lazily mounting a list of hooks. The rest of the implementation for function components can be shared.

Because the error handling behavior added in this commit is just using useTransition under-the-hood, it also handles pending states, too. However, this pending state can't be observed until we add a new hook for that purpose. I'll add this next.

This sets up the initial writing for form actions. The idea is that
form actions are implicitly wrapped with `startTransition`.

This does not handle async actions yet, so error and pending states
don't work. That will be added in later steps.
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Apr 21, 2023
@react-sizebot
Copy link

react-sizebot commented Apr 21, 2023

Comparing: cc93a85...87a2258

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 = 163.95 kB 163.96 kB = 51.67 kB 51.67 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js +0.36% 167.70 kB 168.30 kB +0.22% 52.81 kB 52.93 kB
facebook-www/ReactDOM-prod.classic.js +0.30% 566.07 kB 567.77 kB +0.32% 100.25 kB 100.57 kB
facebook-www/ReactDOM-prod.modern.js +0.31% 549.80 kB 551.50 kB +0.33% 97.57 kB 97.90 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-reconciler/cjs/react-reconciler.production.min.js +0.55% 113.12 kB 113.74 kB +0.50% 34.63 kB 34.80 kB
oss-experimental/react-reconciler/cjs/react-reconciler.profiling.min.js +0.51% 122.13 kB 122.75 kB +0.49% 36.83 kB 37.01 kB
oss-experimental/react-reconciler/cjs/react-reconciler.development.js +0.41% 903.47 kB 907.19 kB +0.53% 192.81 kB 193.83 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js +0.36% 167.70 kB 168.30 kB +0.22% 52.81 kB 52.93 kB
oss-experimental/react-dom/umd/react-dom.production.min.js +0.36% 167.61 kB 168.21 kB +0.28% 53.17 kB 53.31 kB
oss-experimental/react-dom/cjs/react-dom-unstable_testing.production.min.js +0.34% 173.92 kB 174.51 kB +0.27% 55.12 kB 55.27 kB
oss-experimental/react-dom/umd/react-dom.profiling.min.js +0.34% 176.60 kB 177.19 kB +0.28% 55.51 kB 55.67 kB
oss-experimental/react-dom/cjs/react-dom.profiling.min.js +0.34% 177.33 kB 177.93 kB +0.30% 55.22 kB 55.39 kB
facebook-www/ReactDOM-prod.modern.js +0.31% 549.80 kB 551.50 kB +0.33% 97.57 kB 97.90 kB
facebook-www/ReactDOM-prod.classic.js +0.30% 566.07 kB 567.77 kB +0.32% 100.25 kB 100.57 kB
facebook-www/ReactDOMTesting-prod.modern.js +0.30% 566.34 kB 568.04 kB +0.31% 101.67 kB 101.99 kB
facebook-www/ReactDOM-dev.modern.js +0.29% 1,402.24 kB 1,406.35 kB +0.36% 303.14 kB 304.22 kB
facebook-www/ReactDOM-profiling.modern.js +0.29% 580.22 kB 581.92 kB +0.33% 102.05 kB 102.38 kB
facebook-www/ReactDOMTesting-prod.classic.js +0.29% 580.88 kB 582.58 kB +0.32% 103.96 kB 104.29 kB
oss-experimental/react-dom/cjs/react-dom.development.js +0.29% 1,274.50 kB 1,278.20 kB +0.36% 280.81 kB 281.82 kB
oss-experimental/react-dom/umd/react-dom.development.js +0.29% 1,336.38 kB 1,340.26 kB +0.36% 283.55 kB 284.57 kB
facebook-www/ReactDOMTesting-dev.modern.js +0.29% 1,420.63 kB 1,424.74 kB +0.35% 307.53 kB 308.62 kB
facebook-www/ReactDOM-dev.classic.js +0.29% 1,430.13 kB 1,434.25 kB +0.36% 308.72 kB 309.84 kB
oss-experimental/react-dom/cjs/react-dom-unstable_testing.development.js +0.29% 1,292.60 kB 1,296.30 kB +0.36% 285.12 kB 286.14 kB
facebook-www/ReactDOM-profiling.classic.js +0.28% 596.55 kB 598.25 kB +0.32% 104.75 kB 105.08 kB
facebook-www/ReactDOMTesting-dev.classic.js +0.28% 1,448.52 kB 1,452.63 kB +0.35% 313.17 kB 314.28 kB

Generated by 🚫 dangerJS against 87a2258

@acdlite acdlite force-pushed the form-action-error-handling branch 2 times, most recently from aeb3e94 to 34003e2 Compare April 21, 2023 00:57
@acdlite acdlite marked this pull request as ready for review April 21, 2023 01:19
next: null,
};

stateHook.queue = queue;
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch, it's superfluous

Copy link
Collaborator

@sebmarkbage sebmarkbage left a comment

Choose a reason for hiding this comment

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

The flag should probably check both enableAsyncActions and enableFormActions where it's relevant.

In terms of the naming on the react-reconciler side. Maybe this should be named something about the HostComponent being Transition aware rather than about Forms specifically.

startFormAction -> startHostTransition

StatefulHostComponent -> TransitionAwareHostComponent

Want to update the Flight fixture to use this instead of console.error for error handling?

if (!enableFormActions) {
return false;
}
return renderWithHooks(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This overrides updateQueue to null. That field is fine to reuse after diffInCommitPhase and outside of legacy mode hydration I think.

But just setting it to null here is probably also fine, since it's only used to carry data from complete phase to commit phase.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder what the specialized form of this looks like for this use case when we can ignore all the user space stuff like multiple render passes, DEV warnings. We don't even have to set the ReactCurrentDispatcher.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I explored during that in this diff but it started getting out of hand so I punted on that for now. Maybe once there's 2 cases.

This is the next step toward full support for async form actions.

Errors thrown inside form actions should cause the form to re-render and
throw the error so it can be captured by an error boundary. The behavior
is the same if the <form /> had an internal useTransition hook, which is
pretty much exactly how we implement it, too.

The first time an action is called, the form's HostComponent is
"upgraded" to become stateful, by lazily mounting a list of hooks. The
rest of the implementation for function components can be shared.

Because the error handling behavior added in this commit is just using
useTransition under-the-hood, it also handles pending states, too.
However, this pending state can't be observed until we add a new hook
for that purpose. I'll add this next.
@acdlite acdlite merged commit fd3fb8e into facebook:main Apr 21, 2023
2 checks passed
acdlite added a commit to acdlite/react that referenced this pull request Apr 21, 2023
Errors in form actions are now rethrown during render (facebook#26689), so we
can handle the using an error boundary.
acdlite added a commit to acdlite/react that referenced this pull request Apr 21, 2023
Errors in form actions are now rethrown during render (facebook#26689), so we
can handle the using an error boundary.
acdlite added a commit to acdlite/react that referenced this pull request Apr 21, 2023
Errors in form actions are now rethrown during render (facebook#26689), so we
can handle the using an error boundary.
acdlite added a commit that referenced this pull request Apr 21, 2023
Errors in form actions are now rethrown during render (#26689), so we
can handle them using an error boundary.
kassens pushed a commit that referenced this pull request Apr 21, 2023
This is the next step toward full support for async form actions.

Errors thrown inside form actions should cause the form to re-render and
throw the error so it can be captured by an error boundary. The behavior
is the same if the `<form />` had an internal useTransition hook, which
is pretty much exactly how we implement it, too.

The first time an action is called, the form's HostComponent is
"upgraded" to become stateful, by lazily mounting a list of hooks. The
rest of the implementation for function components can be shared.

Because the error handling behavior added in this commit is just using
useTransition under-the-hood, it also handles pending states, too.
However, this pending state can't be observed until we add a new hook
for that purpose. I'll add this next.
kassens pushed a commit that referenced this pull request Apr 21, 2023
Errors in form actions are now rethrown during render (#26689), so we
can handle them using an error boundary.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
This is the next step toward full support for async form actions.

Errors thrown inside form actions should cause the form to re-render and
throw the error so it can be captured by an error boundary. The behavior
is the same if the `<form />` had an internal useTransition hook, which
is pretty much exactly how we implement it, too.

The first time an action is called, the form's HostComponent is
"upgraded" to become stateful, by lazily mounting a list of hooks. The
rest of the implementation for function components can be shared.

Because the error handling behavior added in this commit is just using
useTransition under-the-hood, it also handles pending states, too.
However, this pending state can't be observed until we add a new hook
for that purpose. I'll add this next.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
Errors in form actions are now rethrown during render (facebook#26689), so we
can handle them using an error boundary.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
This is the next step toward full support for async form actions.

Errors thrown inside form actions should cause the form to re-render and
throw the error so it can be captured by an error boundary. The behavior
is the same if the `<form />` had an internal useTransition hook, which
is pretty much exactly how we implement it, too.

The first time an action is called, the form's HostComponent is
"upgraded" to become stateful, by lazily mounting a list of hooks. The
rest of the implementation for function components can be shared.

Because the error handling behavior added in this commit is just using
useTransition under-the-hood, it also handles pending states, too.
However, this pending state can't be observed until we add a new hook
for that purpose. I'll add this next.

DiffTrain build for commit fd3fb8e.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
Errors in form actions are now rethrown during render (#26689), so we
can handle them using an error boundary.

DiffTrain build for commit 967d46c.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants