-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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(css): add support for Lightning CSS #12807
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! 🥳
default: | ||
throw new Error(`Unsupported dependency type: ${dep.type}`) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing that might be worth trying would be to use the visitor API to do this instead of doing string replacement afterward (using analyzeDependencies
). You should test the perf and see which is faster. Here's an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will try it out. A nice thing it that this would give correct source map no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at it but the issue is that the function is async (and it's rooted deep into shared resolve logic being async in Vite)
Looks awesome! Great work. I just have one request to keep preprocessor support. Sass/less is non negotiable for me, and many consumers of vite as well. If performance isn't that improved with preprocessor, so be it. It's the consumer's choice. But removing preprocessors entirely will break many apps and projects. Again, thanks for this amazing improvement to Vite. Can't wait to test it out |
Useless for me without preprocessor support... Also, it will be confusing, sometimes it works, sometimes it doesn't? |
@PuruVJ what's your need for using a pre-processor and Lightning CSS as a transformer ? (the option will be provided to use it a minifier only because not everyone will be able to migrate to it due the high usage of Tailwind) |
@@ -828,6 +895,10 @@ async function compileCSS( | |||
modules?: Record<string, string> | |||
deps?: Set<string> | |||
}> { | |||
if (config.css?.transformer === 'LightningCSS') { | |||
return compileLightningCSS(id, code, config, config.css, urlReplacer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be possible to use both Lightning CSS and PostCSS/SASS/Less together? In my prototype, I ran those pre-processors first (if needed), and then fed the output into Lightning CSS. That would make it possible to use things like Tailwind, but still use Lightning CSS for other transforms instead of autoprefixer for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be done in build to get better minification. I'm not sure of the benefit in dev (compare to the complexity of not messing up with sourcemaps for example)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lightning CSS accepts an input source map so you can pass the source map from postcss/sass in and it'll remap the output. Aside from that, is there any reason not to support it? Lightning CSS has different ways of compiling modern CSS features for older browsers than postcss-preset-env/autoprefixer, for example, and even includes support for some features that other compilers don't. Until recently, Lightning CSS was the only tool supporting relative color syntax, for example. Users may also want to use custom Lightning CSS transform plugins.
I think limiting it to be only Lightning CSS or PostCSS/Sass will severely limit the adoption, since so many people use Tailwind. I think it will be useful to give users the choice of what combination of tools they want to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah relative color syntax is a good example.
The main reason for not adding it for now was to reduce the API surface and so the number of bugs that then we have to consider. On top of that the API would not be as trivial (for example on how to be explicit on CSS modules being handled by one or the other). And in the future we will able to use Tailwind without PostCSS.
What's your opinion @patak-dev?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also leaning toward starting with Lightning CSS without preprocessing. But I think it is a case we should think about up front and design the config API to allow us to go there later. Using postcss as a preprocessor for lightning CSS, we could do it as:
css: {
transformer: 'lightingcss',
lightningcss: { ... },
postcss: { ... }
}
Or maybe allow postcss options as preprocessorOptions.postcss
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now we are on the side of not being able to use pre-processor & LightningCSS. The elephant in the room is Tailwind, but given the fact that Tailwind without postcss is something coming and UnoCSS can give you the same experience without the PostCSS requirement (for people like liking the config part with fixed number of possibilities, I built this)
0672e2e
to
67c821d
Compare
/ecosystem-ci run |
📝 Ran ecosystem CI: Open
|
Thanks for implementing this @ArnaudBarre! And thanks @devongovett for the initial POC and for pushing us to try the lib out. Let's merge this PR as we discussed Arnaud so we can let users test it in 4.4. Let me know if you intend to send a follow-up PR with some minimal docs, I'll do it if not. |
If you have feedback about Lightning CSS and the experimental API added in this PR, please share it at: We'll check if this feature could be moved out of experimental in Vite 5 based on it. |
Fixes #11029
Based on an initial work from @devongovett
I made the choice to make it incompatible with preprocessor. With the current state of CSS (vars, nesting) and modules supported by
Lightning CSS
, adding a preprocessor will juts hurt performances for very little (negative?) DX.