-
-
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
fix(ssr): remove pure CSS dynamic import #17371
fix(ssr): remove pure CSS dynamic import #17371
Conversation
Run & review this pull request in StackBlitz Codeflow. |
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.
LGTM. I wished we could abstract the code so we don't have to duplicate it, but it seems a bit tricky.
/ecosystem-ci run |
📝 Ran ecosystem CI on
✅ analogjs, astro, histoire, ladle, laravel, marko, previewjs, quasar, qwik, rakkas, remix, unocss, vike, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-swc, vite-plugin-svelte, vite-plugin-vue, vite-setup-catalogue, vitepress |
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.
The vite-plugin-react-pages is a fluke. The SvelteKit one seems to be a snapshot-like issue after this change. Approving as it looks ok to merge this as part of the 5.3 beta. @sapphi-red letting you merge it though in case you want to check out SvelteKit
📝 Ran ecosystem CI on
✅ analogjs, astro, ladle, laravel, marko, previewjs, quasar, qwik, rakkas, remix, unocss, vike, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-swc, vite-plugin-svelte, vite-plugin-vue, vite-setup-catalogue, vitepress |
The SvelteKit tests has been failing on various different tests on each run, so it's hard to tell this is causing it to fail. I'll merge this for now and will backport to 5.2.13 soon. |
Description
The pure chunk was removed but the dynamic import importing that chunk was not removed.
This
if
condition istrue
for SSR code.vite/packages/vite/src/node/plugins/importAnalysisBuild.ts
Line 321 in 147e922
Dynamic import for non-SSR code is removed here.
vite/packages/vite/src/node/plugins/importAnalysisBuild.ts
Lines 410 to 422 in 147e922
fixes #17366
refs #16078