Skip to content

Add error boundary to Flight fixture#26695

Merged
acdlite merged 1 commit intofacebook:mainfrom
acdlite:add-error-boundary-to-flight-fixture
Apr 21, 2023
Merged

Add error boundary to Flight fixture#26695
acdlite merged 1 commit intofacebook:mainfrom
acdlite:add-error-boundary-to-flight-fixture

Conversation

@acdlite
Copy link
Copy Markdown
Collaborator

@acdlite acdlite commented Apr 21, 2023

Errors in form actions are now rethrown during render (#26689), so we can handle them using an error boundary.

@acdlite acdlite requested a review from sebmarkbage April 21, 2023 17:44
@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
Copy Markdown

react-sizebot commented Apr 21, 2023

Comparing: fd3fb8e...9cfd696

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.96 kB 163.96 kB = 51.67 kB 51.67 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 168.30 kB 168.30 kB = 52.93 kB 52.93 kB
facebook-www/ReactDOM-prod.classic.js = 567.77 kB 567.77 kB = 100.57 kB 100.57 kB
facebook-www/ReactDOM-prod.modern.js = 551.50 kB 551.50 kB = 97.90 kB 97.90 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 9cfd696

Copy link
Copy Markdown
Contributor

@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.

Also use it for src/Button.js?

Comment thread fixtures/flight/src/Form.js Outdated
<ErrorBoundary>
<form
action={async formData => {
setIsPending(true);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this even work now given that this whole thing should be a transition now?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ah yeah I guess it doesn't. I could wrap it in flushSync temporarily. Or maybe just wait to update the fixture until I add useFormPending.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

OTOH that behavior is broken anyway. So I'l do the flushSync for now, with a TODO explaining it's temporary.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The isPending state is dead code anyways. I think it's copy & paste from the button example where it controlled the disabled state.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think it's just there to illustrate what the pattern is. Everything resolves so fast anyway you don't notice in the button example either.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, when I introduced it in https://github.com/facebook/react/pull/26293/files there was a delay of 500ms in the action. 🤷

Errors in form actions are now rethrown during render (facebook#26689), so we
can handle the using an error boundary.
@acdlite acdlite force-pushed the add-error-boundary-to-flight-fixture branch from 8aa62c4 to 9cfd696 Compare April 21, 2023 18:07
@acdlite acdlite merged commit 967d46c into facebook:main Apr 21, 2023
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
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
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.

5 participants