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

feat: scope css to js module to allow treeshaking it (requires vite 6.2) #1092

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

dominikg
Copy link
Member

@dominikg dominikg commented Mar 2, 2025

see vitejs/vite#19418

This allows vite to treeshake unused svelte component css if the component was referenced in a barrel file.

@@ -133,6 +133,31 @@ export function createCompileSvelte() {
let compiled;
try {
compiled = svelte.compile(finalCode, { ...finalCompileOptions, filename });
if (compiled.css) {
if (finalCode.includes(':global') && compiled.ast.css) {
Copy link
Member

Choose a reason for hiding this comment

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

Could we have a comment here of what this is trying to do? Is it checking for query selectors that starts with :global() and not the nested kind?

Also a bit concerned of the perf implication with the additional walking but I guess it can be optimized later.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, unscoped :global must be included or it would be a breaking change and also differ in experience between dev and build because in dev vite still includes the css, its only treeshaken in build.

the include check before walking should minimize impact. If you feel this is still too much of a regression, should we make this opt-in?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to make this opt-in or is a regression, just wondering what specifically this block was doing and I'm fine with the current logic 👍 It would be nice to add a comment here only explaining what it's doing for.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@dominikg
Copy link
Member Author

the regex approach is a bit less clunky i think. least clunk inside vps would be an option that allows users to define the tests themselves on filename+content, similar to dynamicCompileOptions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants