parcel-bundler / lightningcss

An extremely fast CSS parser, transformer, bundler, and minifier written in Rust.
https://lightningcss.dev
Mozilla Public License 2.0
6.56k stars 190 forks source link

`exclude` option does not work as expected #792

Closed chenjiahan closed 2 months ago

chenjiahan commented 3 months ago

I'm using the exclude option of Lightning CSS. According to the documentation:

The include and exclude options allow you to explicitly turn on or off certain features.

Adding MediaRangeSyntax to exclude turns out not to work:

image

playground

@media (min-resolution: 2dppx) {
  .a {
    color: green;
  }
}
@media (resolution>=2dppx){.a{color:green}}
@media (min-resolution: 2dppx){.a{color:green}}
chenjiahan commented 2 months ago

Another case: https://lightningcss.dev/playground/index.html#%7B%22minify%22%3Atrue%2C%22customMedia%22%3Afalse%2C%22cssModules%22%3Afalse%2C%22analyzeDependencies%22%3Afalse%2C%22targets%22%3A%7B%22chrome%22%3A2621440%7D%2C%22include%22%3A0%2C%22exclude%22%3A524288%2C%22source%22%3A%22.foo%20%7B%5Cn%20%20inset%3A%200%3B%5Cn%7D%5Cn%5Cn.bar%7Btop%3A0%3Bbottom%3A0%3Bleft%3A0%3Bright%3A0%7D%22%2C%22visitorEnabled%22%3Afalse%2C%22visitor%22%3A%22%7B%5Cn%20%20Color(color)%20%7B%5Cn%20%20%20%20if%20(color.type%20%3D%3D%3D%20'rgb')%20%7B%5Cn%20%20%20%20%20%20color.g%20%3D%200%3B%5Cn%20%20%20%20%20%20return%20color%3B%5Cn%20%20%20%20%7D%5Cn%20%20%7D%5Cn%7D%22%2C%22unusedSymbols%22%3A%5B%5D%2C%22version%22%3A%22local%22%7D

The LogicalProperties has been excluded, but Lightning CSS still transform to inset:

image

chenjiahan commented 2 months ago

I want to share the background why we use exclude:

Rspack uses lightlingcss-loader to downgrade CSS syntax, and LightningCssMinimizerPlugin to minify CSS files.

In the LightningCssMinimizerPlugin, we use the exclude option of Lightning CSS to prevent the CSS files from being transformed twice, but we found that exclude caused some CSS syntax to be transformed to a higher version, which was unexpected.

If we do not use the exclude in LightningCssMinimizerPlugin, then the role of LightningCssMinimizerRspackPlugin is not only to minimize, it also does the transformation, and users need to configure the same target for both lightningcss-loader and LightningCssMinimizerPlugin to ensure that the transformation results are consistent.

@devongovett Do you have any suggestions on this? ❤️

chenjiahan commented 2 months ago

@devongovett Sorry to bother you again, this is important for Rspack, could you take a look?

devongovett commented 2 months ago

Perhaps this is counterintuitive, but the way the exclude option works is as an override of the browser compat data. It means "don't compile this feature, assume it is supported by all target browsers". The include option does the opposite: it assumes the feature is not supported by any target.

For media range syntax, min-resolution: 2dppx actually parses as a range resolution >= 2dppx. Then, according to the targets you've set and the browser compat data, it either stays as that range or is output as min-resolution. So the exclude of the MediaRangeSyntax feature is actually preventing the resolution >= 2dppx range from being compiled to min-resolution: 2dppx resulting in the output you observe (we assume that all targets support it).

Similarly for logical properties, excluding the LogicalProperties feature means that we assume the target supports them. Therefore, inset remains and top/bottom/left/right can be converted.

In the LightningCssMinimizerPlugin, we use the exclude option of Lightning CSS to prevent the CSS files from being transformed twice

I think the right way to do this is to ensure that the targets option is specified in all places where you call Lightning CSS. This is necessary because Lightning CSS parses CSS into a normalized data structure that doesn't necessarily match the input syntax (a good example of this is that legacy media query min- and max- syntax is stored the same way as ranges). The targets tell Lightning CSS how it should output from this normalized structure to CSS code, so targets are always needed even if no visible transformation is occurring.

Running Lightning CSS twice with the same targets is also how we currently use it in Parcel (in @parcel/transformer-css and @parcel/optimizer-css). There isn't really any downside to doing this - it's not any more or less expensive performance wise to set targets in both places, and Lightning CSS should not result in different code when run more than once (if it is, that's a bug). In Parcel we read the browserslist to do this, so targets are configured in one place for both plugins. Perhaps you could do the same?

chenjiahan commented 2 months ago

Thank you for your detailed explanation! The way exclude works is indeed different from what we expected. We will refer to Parcel's approach and recommend users to configure the target to ensure that the CSS transformation results are the same as expected.

I will close this issue, and we will continue to discuss implementation methods in Rspack. Thanks again ❤️