-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
feat: read error from custom element attributes in vite-error-overlay #5686
Conversation
@patak-dev did you get the chance to ask the team about this? |
#6184 is doing something similar to tackle this issue too. It's exporting the |
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.
Just my approval for the code, but please read @bluwy's comment
@bluwy Yeah, that's not very different from what we're currently doing. |
Looks like #6184 got merged. I think we should follow that to have one way of doing things, hopefully it'll make the code less hacky 😬 |
@patak-dev Looks like we are moving towards using ErrorOverlay in an imperative way instead of declarative so perhaps we can just close this PR? |
Could you explain a bit more how #6184 affected this PR (that had approval from the team)? I thought that #6184 was only correcting the case when there is malformed HTML. Are your use cases covered without this PR? Or do you intend to add support for them with another approach? As a note, there is now work to use the error overlay for runtime errors here #6274 |
@patak-dev In #6184, it made a change to export the |
We could mark |
I currently leaning towards the current (imperative) API as we don't have to maintain a separate source of errors retrieved via attributes. But SvelteKit hasn't used the error overlay element before, so I can't speak of how ergonomic it will be in practice. I guess it's @frandiox's call here since they have more experience with this. |
Closing this for now as it has been stale. We should stick with the imperative way at the meantime. |
Description
Currently, SSR frameworks based on Vite that want to keep the same error overlay experience on SSR errors need to either:
vite.ws.send({ type : 'error', err })
(ornext(err)
) wrapped in an arbitrary timeout to wait until the browser has downloaded the main Vite code that registersvite-error-overlay
custom element and handles the WebSocket.vite-error-overlay
in the CE registry and calls it.This PR enables
vite-error-overlay
custom element to read the error information from attributes if nothing is passed to the constructor. This allows creating error overlays declaratively as HTML:<vite-error-overlay message="..." stack="..."></vite-error-overlay>
, which can be sent as part of the SSR response.Additional context
@patak-js Is this something the Vite team is comfortable with?
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).