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 duplicated CSS modules inlining #9531

Merged
merged 2 commits into from
Dec 27, 2023
Merged

Fix duplicated CSS modules inlining #9531

merged 2 commits into from
Dec 27, 2023

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Dec 27, 2023

Changes

fix #6885

When a .module.css file is imported by both an Astro file and framework component, it gets inlined by Astro to prevent a FOUC. However, the way we inline it before (using a link) is not how Vite handles it when it's loaded in the client (via the framework component). Vite expects the CSS module to be a style tag inlined.

This PR uses a style tag instead to fix it, which potentially causes different ordering with other style tags now (since CSS modules used to be links and they're rendered separately), but since we had changed the ordering before at #8877 (which I think worked fine?), I think we can tweak this a little bit more.

Testing

Updated tests

Docs

n/a. bug fix.

Copy link

changeset-bot bot commented Dec 27, 2023

🦋 Changeset detected

Latest commit: f157c45

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

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review labels Dec 27, 2023
Comment on lines +31 to +34
const url = new URL(importedModule.url, 'http://localhost');
// Mark url with ?inline so Vite will return the CSS as plain string, even for CSS modules
url.searchParams.set('inline', '');
const modId = `${decodeURI(url.pathname)}${url.search}`;
Copy link
Member Author

Choose a reason for hiding this comment

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

We used to load the CSS file without the ?inline query, which is fine for plain CSS files. But now we need ?inline so it also works for CSS modules. ?inline added to CSS files is also fine and should also lead to lesser processing.

}
if (
mode === 'development' && // only inline in development
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed this check (and the mode param entirely) as it seems like we only use this function in dev now.

@matthewp matthewp merged commit 662f06f into main Dec 27, 2023
13 checks passed
@matthewp matthewp deleted the fix-css-modules-inlining branch December 27, 2023 17:35
@astrobot-houston astrobot-houston mentioned this pull request Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicated css import when hydrating Solid-js component in dev mode
2 participants