feat: add error boundary for form input components #6869
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
|
No changes to documentation |
stipsan
force-pushed
the
add-input-error-boundary
branch
from
33e0a91
to
03b9cb0
Compare
|
Component Testing Report Updated Jun 7, 2024 12:47 AM (UTC)
|
cngonzalez
reviewed
Jun 7, 2024
| * when there are no errors. | ||
| * @internal | ||
| */ | ||
| function ErrorCard(props: {error: unknown; onRetry: () => void}) { |
There was a problem hiding this comment.
The unknown type here and the stack and message params become a little confusing for me here, but I understand if it's more difficult to more strictly type things here.
There was a problem hiding this comment.
It follows the precedence in StudioErrorBoundary, as well as the convention of the stricter TS rule that enforces:
try {
doSomething()
} catch(error: unknown) {
// `error` is never guaranteed to be a subclass of `Error` and can be anything, `unknown` forces you to deal with every eventuality
}Which is also why the isRecord asserters are used further down
There was a problem hiding this comment.
ahh, of course! apologies for not checking that
stipsan
force-pushed
the
add-input-error-boundary
branch
from
88aaa8c
to
9e81d00
Compare
stipsan
force-pushed
the
add-input-error-boundary
branch
from
9e81d00
to
1f64bfb
Compare
cngonzalez
approved these changes
Jun 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
While debugging issues in React 19, as well as React Compiler, I've frequently encountered crashes that takes down the Structure Tool completely:
It's difficult to track down exactly what's causing it, the same is true when handling user reports of crashes, in Next v14 and v15 embedded studios in particular.
This PR adds an error boundary for input components, which narrows down the crash to the input level:
In the image above only some of the Portable Text Editor inputs are crashing, significantly helping with narrowing down the cause. Fixes for the errors seen here will be done in follow up PRs.
What to review
Does the language strings and the UI make sense?
Testing
It's easiest to test on the compiled studio deployment, as it crashes early on race condition issues, instead of requiring tricky race condition reproduction steps: https://test-compiled-studio-git-add-input-error-boundary.sanity.build/test/structure/input-standard;portable-text;pt_allTheBellsAndWhistles;7455ba88-75d5-4a77-94a4-91738fa670c5
Notes for release
Form input components now have their own error boundaries, limiting crashes to a field level on a document form, instead of taking down the entire Structure Tool.