vitePreprocess runs Sass, but PostCSS too

[bfanger]

Describe the bug

When using <style lang="scss"> vitePreprocess also applies the PostCSS transformations.

Take the following example:

<button class="btn">Hello</button>

<style lang="scss">
  .btn {
    background: gray;
    border: 2px outset;

    &:focus-visible {
      outline: 2px solid orange;
    }
  }
</style>

When using postcss-preset-env this results in:
[vite-plugin-svelte] /src/App.svelte:8:0 Unused CSS selector ".btn.focus-visible"

It looks like the transformations are:

Step	Processor	Effect
1	Sass	creates .btn:focus-visible selector
2	PostCSS	adds .btn.focus-visible selector
3	Svelte Compiler	removes .btn.focus-visible selector
4	PostCSS	adds the .btn.focus-visible selector again

I think the default should be to remove the 2nd step, don't run PostCSS before passing the css to the Svelte Compiler.

Reproduction URL

https://stackblitz.com/edit/vitejs-vite-pkrz2a?file=src/App.svelte

Reproduction

The error is visible in the console when the App.svelte is compiled.

1. npm run dev

Logs

[vite-plugin-svelte] /src/App.svelte:8:0 Unused CSS selector ".btn.focus-visible"

System Info

Binaries:
    Node: 16.14.2 - /usr/local/bin/node
  npmPackages:
    @sveltejs/vite-plugin-svelte: ^2.0.2 => 2.0.2 
    svelte: ^3.55.1 => 3.55.1 
    vite: 4.0.4

[dominikg]

is the behavior different from using svelte-preprocess?

vitePreprocess applies vites css pipeline before compile, I'm not sure you can selectively disable parts of it

[bluwy]

Hmm I didn't take into account that Vite would also apply postcss for normal css files. I'd say this is expected behaviour as Vite does this too, and could be worked around by suppressing the warning with onwarn.

Maybe we should also let Vite preprocess regardless if lang exists in this case 🤔

[dominikg]

While the double processing is unfortunate and we should look into possible ways to avoid it, i wonder why it creates a class for focus-visible in the first place. It has really good browser support nowadays, so unless you target obscure or dead browsers, it should not even turn that into an extra class.

Did you just not add a browserslist config to your reproduction? Didn't see one in either .browserslistrc or package.json.

[bfanger]

I've added the a browserslist config, using the defaults config, aka "a reasonable configuration for most users" according to browserlist's readme.

But the issue is not with the focus-visible plugin, the issue that it is running PostCSS as preprocessor

[bluwy]

FWIW svelte-preprocess runs PostCSS with lang="scss" too if you configure postcss: true for it. (Stackblitz example). The behaviour you're looking for is without the postcss: true configuration, but I feel like vitePreprocess processing postcss even in scss is a feature rather than a bug, because that's how Vite processes CSS files too.

If we don't run postcss, someone mixing Tailwind's @apply and SCSS would break (for example, but there can be many other cases), so the PR would be a breaking change unless we introduce an option. But I don't think we really need to go that length as you can use onwarn to suppress it if it's incorrect.

[dominikg]

yes, postcss is mostly a post-processor, but there are postcss plugins that apply transforms that have to be executed before svelte.compile because the syntax they use as input is not accepted by the svelte compiler. Combine any of those (tailwind is just the most prominent example) with a preprocessor like scss and it would break if we skipped postcss before compile.

I also would like to encourage you again to investigate why this class is created in the first place and what rules it contains. looking at https://github.com/WICG/focus-visible it appears to require additional javascript to make it work in browsers that don't support it ootb. Something a postcss plugin cannot provide on it's own.

[bfanger]

<style lang="scss">
  .title {
    @apply text-3xl;
  }
</style>

@bluwy You'll probably get an Unknown at rule @apply warning in vscode, but the code will still output the desired css .title.s-y_bCXRrkrYfP{font-size: 1.875rem;line-height: 2.25rem} with or without the PR applied.

PostCSS is still run (again) after svelte has added its classes for css scoping (and removed unused selectors).

[bluwy]

Yes, but still the point is that running PostCSS is part of the style preprocessing pipeline even for SCSS in Vite and we shouldn't change that.

@apply was just one example (which seems to coincidentally still work). @media screen(sm) { doesn't.

[dominikg]

given that both svelte-preprocess and vite run postcss as preprocessor aswell and that there are postcss plugins that require this behavior to prevent breaking existing applications i'm inclined to say that this is not a bug in vite-plugin-svelte and works as designed.

the extra .focus-visible class is added like this:

<style lang="scss">.btn {
  background: gray;
  border: 2px outset;
}
.btn.focus-visible {
  outline: 2px solid orange;
}
.btn:focus-visible {
  outline: 2px solid orange;
}</style>

which requires additional javascript to add/remove the class from the .btn element. The svelte compiler is correct to complain/remove here as there is nothing in the component that uses this class and it compiles scoped styling.

As you already mentioned in the end it works because it is readded by the final postcss pass, so what you are dealing with here is a warning. You are able to remove this by providing a custom onwarn handler.

Regarding "defaults" browserslist , that includes browsers like uc-, qq- and opera mini, which are notoriously bad at web standards. Unless you have a site that caters to a usergroup that is known to use these browsers, i would suggest to exclude them.

[dominikg]

closing as described above. This doesn't mean we won't consider a feature request for adding different behavior in the future, but given that this is the only report so far and onwarn config provides an easy way to avoid the warning this doesn't have priority currently.

[bfanger]

For anyone who wants a more svelte-preprocess compatible processing of sass, use the following configuration:

// svelte.config.js
import { vitePreprocess } from "@sveltejs/kit/vite";

/** @type {import('@sveltejs/kit').Config} */
export default {
  preprocess: vitePreprocess({
    style: {
      css: {
        postcss: { plugins: [{ postcssPlugin: "vitePreprocess" }] },
      },
    },
  })
}

But you can't use <style lang="postcss"> anymore. If you also want svelte-preprocess's :global { } syntax, look at globalStyle for inspiration (or keep using svelte-preprocess).

Update: Weirdly this only fixes the processing/warning in VSCode, the vite dev server still reports warnings. Looks like the style config is ignored when the style.__resolvedConfig is used.

[dominikg]

please note that we advise against using :global from svelte-preprocess in vite projects, using plain (s)css files and importing them is the recommended way of working with global styles in vite.

see https://github.com/sveltejs/vite-plugin-svelte/blob/main/docs/faq.md#where-should-i-put-my-global-styles

You are of course welcome to use them anyways, but this isn't very "vitey".