Allow plugins to hook into `preprocessCSS()`

philipp-spiess commented 2 months ago

Description

Vite has an API that can be used to process CSS files outside of the regular module graph: https://github.com/vitejs/vite/pull/10429 Integrations like the vite-plugin-svelte use these to process styles inside <style> blocks.

However, when this API is used, no hook is called inside Vite extensions before the CSS preprocessing is done and the processed styles is used by the respective users of this API.

This means that eventual CSS pre processor extensions (like Tailwind CSS) can not access the styles before the CSS pipeline (with either postcss or lightningcss) or before the styles are passed through the Svelte extension.

Alternative

Alternatives right now require plugin-specific workarounds. In the case of vite-plugin-svelte, our Vite extension can register a hook into the Svelte CSS pre-processor which will run before the svelte Vite plugin handles things. This has to be done for all possible users of the preprocessCSS() API.

Additional context

No response

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

bluwy commented 2 months ago

What about writing a postcss plugin? preprocessCSS is currently only about Vite's internal CSS handling and isn't meant to be a general "run plugins to process CSS" utility.

philipp-spiess commented 2 months ago

@bluwy

What about writing a postcss plugin?

We thought about this, but this won't work if css.transformer is set to lightningcss.

preprocessCSS is currently only about Vite's internal CSS handling and isn't meant to be a general "run plugins to process CSS" utility.

Yeah, this makes sense. In the case of Svelte, defining the custom Svelte preprocessor is definitly the better approach conceptually. It just means that we will have to hope that other users of preprocessCSS() provide an API for us to hook into their preprocessor list too to make it "just work" when someone adds the Tailwind CSS Vite plugin.

bluwy commented 2 months ago

Lightningcss also supports the visitor API which you could, though it looks like Vite's types may not be updated for it yet, and you'd have to maintain two implementations.

Is this only for vite-plugin-svelte? A Svelte preprocessor is definitely the better choice then, and if you already have a Vite plugin, you can add the preproceesor automatically like this: https://github.com/sveltejs/vite-plugin-svelte/blob/f06beb3836774b4a033a780712c7e84011698668/docs/faq.md#how-do-i-add-a-svelte-preprocessor-from-a-vite-plugin

Lalem001 commented 2 months ago

If I may tag along, I have a plugin I am trying to write to minify css template literals. Being able to hook into the same minification as standalone files would be a boon. Would this be a case you would consider @bluwy ?

bluwy commented 2 months ago

Do you mean const foo = css`.foo { color: red }`? Wouldn't that be more of transforming JS code which would fit as a Vite plugin?

philipp-spiess commented 2 months ago

@bluwy

The same issue exits with other users of the preprocessCSS() API, like Astro. The issue there is less urgent, since Astro passes through the CSS until it eventually hits the Vite transform hook (where as Svelte would be more strict about the input CSS) but it still means for the case of Tailwind CSS that some features don't work when the pre-processor is set to lightningcss—e.g. take a look at this issue).

I think what is confusing here most is that we add a transform() hook in the enforce: 'pre' phas, yet styles passed through the preprocessCSS() pipeline will already be processed beforehand—even though they will still eventually end up inside the transform() hook (potentially even double-processed?).

Lalem001 commented 2 months ago

Do you mean const foo = css`.foo { color: red }`? Wouldn't that be more of transforming JS code which would fit as a Vite plugin?

@bluwy Yes that is my intention. I have code that can parse out the template literal content and put in placeholders for the dynamic bits. I would like to use preprocessCSS, or similar, to minify that string.

Willing to move to a new issue so I dont derail this issue further.

bluwy commented 2 months ago

@bluwy Yes that is my intention. I have code that can parse out the template literal content and put in placeholders for the dynamic bits. I would like to use preprocessCSS, or similar, to minify that string.

@Lalem001 I'd suggest opening a GitHub discussion instead, I don't think preprocessCSS is the right fit here again. Its usecase is to only preprocess CSS, not completely transforming and minifying it.

The issue there is less urgent, since Astro passes through the CSS until it eventually hits the Vite transform hook (where as Svelte would be more strict about the input CSS)

@philipp-spiess I think Astro and Svelte should handle CSS the same, and both should go through Vite's transform hook at the end. Unless you mean Svelte is stricter than it'll strip out unused CSS?

I think what is confusing here most is that we add a transform() hook in the enforce: 'pre' phas, yet styles passed through the preprocessCSS() pipeline will already be processed beforehand—even though they will still eventually end up inside the transform() hook (potentially even double-processed?).

Yeah the intention here is to process them twice. preprocessCSS handles CSS preprocessors (e.g. sass, stylus, @import etc), and the rest of the Vite pipeline do the final URL replace, minifying, etc. This may cause some postcss plugins to be executed twice, but it isn't possible to know which phase they're only required in.

On the original topic, I think I understand the usecase, but it would be hard to implement it. We need the pluginContainer to run the plugins transform hook, but that's not possible inside preprocessCSS. It requires the dev server or the rollup build instance. And even if we can get that to work, performance would also be a concern. Manually calling it will also cause issues with missing this APIs or incorrect sourcemap mappings.

Given this, I still think the most viable choice is with a postcss plugin or the lightningcss visitor API. Or if Svelte's custom preprocess hook works for you, then I think Astro could also consider adopting something similar.

philipp-spiess commented 2 months ago

@bluwy Yeah, that makes sense. I'll give the postcss plugin/lightningcss visitor approach a try or if there are similar features to the svelte preprocess hook on Astro etc.

Update: It looks like trying to use the visitor API will crash lightningcss for our setup. I have to try a few things to rule out it's because of our setup:

thread '<unnamed>' panicked at napi/src/threadsafe_function.rs:235:57:
called `Result::unwrap()` on an `Err` value: Error { status: InvalidArg, reason: "expect Function, got: Undefined", maybe_raw: 0x0 }
stack backtrace:
   0:        0x1406af184 - _napi_register_module_v1
   1:        0x140007e68 - <unknown>
   2:        0x14068f7cc - _napi_register_module_v1
   3:        0x1406b0488 - _napi_register_module_v1
   4:        0x1406afd70 - _napi_register_module_v1
   5:        0x1406b0c6c - _napi_register_module_v1
   6:        0x1406b07d0 - _napi_register_module_v1
   7:        0x1406b0734 - _napi_register_module_v1
   8:        0x1406b0728 - _napi_register_module_v1
   9:        0x1406dfc10 - _napi_register_module_v1
  10:        0x1406dff48 - _napi_register_module_v1
  11:        0x1406660e0 - <unknown>
  12:        0x1012dd268 - __mh_execute_header
  13:        0x1012db61c - __mh_execute_header
  14:        0x100f17d40 - __mh_execute_header
  15:        0x100e6c41c - __mh_execute_header
error: Failed to run "vite" due to signal SIGABRT

bluwy commented 6 days ago

Hey! I believe the conclusion here is that this will be quite hard to support as the two concepts are kinda separate and merging them would cause preprocessCSS to expand in scope and potentially slow down when running too. I'll close this issue for now but happy to discuss further if needed.

vitejs / vite