Skip to content
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

fix(ssr): pretty print plugin error in ssrLoadModule #19290

Merged
merged 3 commits into from
Jan 27, 2025

Conversation

hi-ogawa
Copy link
Collaborator

@hi-ogawa hi-ogawa commented Jan 26, 2025

Description

Though this might look like a regression from the before/after screenshots below, this error was never pretty-printed. What's happening in sveltekit's repro is that, a plugin error of ssrLoadModule entry transform was outside this try/catch, so sveltekit were able to try/catch and pretty print on their own with "Internal server error" on Vite 5. Since Vite 6, entry level transform also goes through runner, so any plugin error will be caught through "Error when evaluating SSR module" message.

Currently this automatic logging is fairly verbose and no error pretty printing. I'm not sure what to do with an extra runnerError (maybe make it non-enumerable?), but for now, doing pretty print probably helps the situation a bit and that's what I did it in this PR.

Screenshots

before (vite 5)

https://www.sveltelab.dev/wwj9l72l4etmdhg

image

before (vite 6)

https://www.sveltelab.dev/o2e6wq4d900igtk

image

after (this pr)

https://www.sveltelab.dev/r04dodmlt1ekx5n

image

Copy link

pkg-pr-new bot commented Jan 26, 2025

Open in Stackblitz

npm i https://pkg.pr.new/vite@19290

commit: 0c819f7

@hi-ogawa hi-ogawa marked this pull request as ready for review January 26, 2025 08:28
@sapphi-red sapphi-red added p2-nice-to-have Not breaking anything but nice to have (priority) feat: ssr labels Jan 27, 2025
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

The change makes sense to me. I'm not sure what to do with the runnerError. Ideally I think we should hide them if the error is expected (e.g. the code includes incorrect syntax).

@patak-dev patak-dev merged commit 353c467 into vitejs:main Jan 27, 2025
23 checks passed
@hi-ogawa hi-ogawa deleted the fix-pretty-print-ssrLoadModule-error branch January 27, 2025 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: ssr p2-nice-to-have Not breaking anything but nice to have (priority) trigger: preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants