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(css)!: remove default import in ssr dev #17922

Merged
merged 7 commits into from
Oct 30, 2024
Merged

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Aug 22, 2024

Description

Found from withastro/astro#11811

In SSR dev, we still incorrectly support default imports of a CSS file (without queries). This feels like a bug fix, but technically I guess it affects frameworks like vitest that executes in SSR and serve mode?

Copy link

stackblitz bot commented Aug 22, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@hi-ogawa
Copy link
Collaborator

I think Vinxi and Remix are relying on default export of ssrLoadModule without ?inline https://github.com/remix-run/remix/blob/c5292edb5997b0818105944b33da7c4cd4567511/packages/remix-dev/vite/styles.ts#L77-L79. Can you check ecosystem ci?

If this was meant to be deprecated already, then maybe it's okay to change though (also it might be just a one liner fix on their end?).

@bluwy
Copy link
Member Author

bluwy commented Aug 22, 2024

Yeah it was deprecated in the previous major, and removed in this major (https://vitejs.dev/guide/migration.html#removed-deprecated-apis). Frameworks need to use ?inline to work, which I thought most have needed to migrated, but maybe we didn't check thoroughly 🤔 I'll run ecosystem-ci to check

@bluwy
Copy link
Member Author

bluwy commented Aug 22, 2024

/ecosystem-ci run

@vite-ecosystem-ci
Copy link

📝 Ran ecosystem CI on b1a7eab: Open

suite result latest scheduled
astro failure success
nuxt success failure
qwik failure failure
remix failure success
storybook success undefined undefined
sveltekit failure success
vitest success failure

analogjs, histoire, ladle, laravel, marko, previewjs, quasar, rakkas, unocss, vike, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-swc, vite-plugin-svelte, vite-plugin-vue, vite-setup-catalogue, vitepress

@bluwy
Copy link
Member Author

bluwy commented Aug 22, 2024

Looks like astro, remix, and sveltekit all fails and likely related to the CSS handling, so this would be breaking then unfortunately. Let's move this to the next major then.

@bluwy bluwy added this to the 6.0 milestone Aug 22, 2024
@bluwy bluwy changed the title fix(css): remove default import in ssr dev fix(css)!: remove default import in ssr dev Oct 24, 2024
@sapphi-red
Copy link
Member

/ecosystem-ci run

@vite-ecosystem-ci
Copy link

📝 Ran ecosystem CI on c8a13ec: Open

suite result latest scheduled
astro failure failure
nuxt failure failure
redwoodjs failure failure
remix failure failure
sveltekit failure failure
vike failure failure
vite-plugin-svelte failure failure
vitest failure failure

analogjs, histoire, ladle, laravel, marko, previewjs, quasar, qwik, rakkas, storybook, unocss, vite-environment-examples, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-swc, vite-plugin-vue, vite-setup-catalogue, vitepress, vuepress

@bluwy
Copy link
Member Author

bluwy commented Oct 28, 2024

From the initial ecosystem-ci run, this affected astro, remix and sveltekit, so perhaps we should delay merging this until we can get them passing in main so it's easier to narrow down the required migration. But either ways if we want to merge this now, I don't mind.

@patak-dev patak-dev merged commit eccf663 into main Oct 30, 2024
14 checks passed
@patak-dev patak-dev deleted the ssr-css-no-default-import branch October 30, 2024 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants