Plugins: Add error boundary #38397
Merged
Plugins: Add error boundary #38397
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
Mamaduka
added
[Feature] Plugins API
|
Size Change: +475 B (0%) Total Size: 1.14 MB
ℹ️ View Unchanged
|
youknowriad
reviewed
Feb 2, 2022
Sounds good to me.
I will add handlers to both Site and Widget editors. I just wanted to share the current implementation for feedback. Do you think we should add the "Copy error" action to the notice? I checked other error boundaries, and we have mixed UIs. For example, block boundary doesn't include actions, while general editor one does. |
|
@Mamaduka I think I'd prefer without it personally as in general the copied error is minified... and doesn't add much value. I didn't hear much feedback about that button in the general editor boundary so would love to be proven wrong :) |
|
I agree minified error message and trace isn't practical. At least in my experience when they are included in issues. We can always include that later if there's a need for it. Thanks again for the feedback, @youknowriad. |
gziolo
added
the
[Feature] Extensibility
Mamaduka
force-pushed
the
try/plugin-error-boundry
branch
from
bcf0ea0
to
c18facd
Compare
Mamaduka
requested review from
adamziel,
ajitbohra,
draganescu,
kevin940726,
nerrad,
noisysocks,
ntwb and
tellthemachines
as code owners
|
I think the code is ready for final review. After the checks are complete, I will push documentation updates to ensure e2e tests aren't flaky. They work locally, but you never know. |
ntsekouras
approved these changes
Feb 2, 2022
Some discussion here on the general editor boundary, btw - #34482 I'm not sure I agree that the error messages aren't useful. They can sometimes provide information on where the error is even when minified. Or at least let the developer know which kind of error is happening. |
|
Thanks for mentioning the general issue, @talldan. I just subscribed and am happy to help with development as well.
I don't have a strong opinion on this, and we can always add the "Copy" action later. |
|
Let's merge this. Happy to follow up with the "Copy" action button if/when we see a need for it. |
bph
added
the
Needs Dev Note
37 tasks
|
@bph, I'm not sure if this needs Dev Note. Yes, it improves developer experience but also requires zero actions from them :) Anyway, happy to write a few words if we want to mention the improvement. |
|
@Mamaduka I wasn't quite sure, but from what I learned listening to developer trying to work with blocks sometimes the error messages are not helpful, or don't point to the right places to start debugging. So anytime you improve things, it's a small win to be celebrated. It might not warrant an individual post but we will have a "Miscellaneous' DevNote where we gather a few smaller changes. :-) |
|
I think it's worth mentioning in Misc. P.S. Error boundary won't point folks in the right direction. It just prevents one error from crashing the whole editor. |
Mamaduka
removed
the
Needs Dev Note
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
PR adds an error boundary around the plugin renderer component.
The editors can handle displaying error messages using the new
onErrorprop ofPluginArea. It's a callback that will receive the plugin's name and an error.Resolves #9630.
Todos
onErrorprop.Testing Instructions
Screenshots
Types of changes
Enhancement
Checklist:
*.native.jsfiles for terms that need renaming or removal).