-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Prevent throwing in react and solid component checks #11624
Conversation
🦋 Changeset detectedLatest commit: f132ffd The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
try { | ||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only refactored this part as the nested try
isn't necessary. Before it's try { try..catch } finally {}
. It can be flatten as try..catch..finally
and work the same.
Hey, thanks for the quick response and fix! When can we expect to see this released? |
How many frameworks does this affect? This seems like a pretty consequent drawback |
They affect the react and solid integrations only. Preact already handles it correctly now. However, this only affects if you have more than one UI framework integration, e.g. if you have React and Svelte 4, you get this error with this PR: If you only have the React integration, Astro will fallback to it and shows the error correctly (same as before this PR): |
Oh okay, fine with me then! |
Changes
fix #11615
Svelte 5 is a function component, so it messes some of our existing renderers like React and Solid where their
check()
functions assumes JSX and wouldn't throw, but Svelte 5 trips them up because it's not JSX functions.Anyways, I think it's more correct that these check functions do not throw since they only do checking, like how Preact works now.
Testing
No tests as we don't have them for Svelte 5, and it's a little tricky to emulate a component that would error 😬 Hopefully the current passing tests are enough though
EDIT: I have to tweak one test regarding runtime errors from Preact components, because previously it relied on the React renderer's check to throw the error if it failed to render, which I don't think is necessarily correct. For now, renderers with
check()
that simply runs the render will not be able to detect actual runtime errors to display on the error overlay.Docs
n/a. bug fix.