Closed romainmenke closed 11 months ago
Thank you for the review 🙇
Just to clarify, base64 encoding is only used for external URL imports, right?
Yes. Only when an import can not be inlined and conditions or cascade layers need to be applied.
Thanks a lot! Ready to publish as v16?
I think I'll wait until after the New Year to publish, just in case something goes wrong.
Awesome! thank you so much for all the careful reviews 🙇
supersedes : https://github.com/postcss/postcss-import/pull/532 fixes : https://github.com/postcss/postcss-import/issues/529
The core issue is how multiple statements are combined.
The previous implementation tried to squash media queries and layer conditions.
layer(foo) screen
layer(bar) (min-width: 1000px)
becomes :
In this example the
foo
layer is only defined when the media isscreen
and has a width of1000px
. While it should actually be defined even when the media isscreen
and smaller.screen
not print
becomes :
In this example the media query becomes invalid because
and
can not be used to combinescreen
andnot print
The implementation for this feature was also fairly complex.
Adding
supports
into this would have made the implementation very unwieldy and it would have increased the chance that users experience bugs.The correct way to combine is actually much simpler.
layer(foo) screen
layer(bar) (min-width: 1000px)
becomes :
screen
not print
becomes :
By using nested statements we ensure that the order of declaration is respected and that no invalid statements are produced.
At this point it also becomes trivial to add support for
supports
conditions.This method does break one aspect.
@import
statements can not be nested syntactically in CSS. They must appear first at the root of the AST.valid :
invalid :
To restore support for this we can use nested stylesheets.
@import
statement urls can be base64 encoded data urls. This doesn't actually have to be base64 encoded, but encoding makes it less prone to errors.Without encoding it looks like this :
@imports url('http://something-external.com') not print;
screen
media conditionThis technique was first thought of by the maintainer of esbuild while fixing issues brought to light by https://github.com/romainmenke/css-import-tests
This implementation change fixes a lot of subtle bugs and as far as I can tell the output is almost always smaller.
It also eliminates some complexity for users. They no longer need to define things like
nameLayer
for anonymous layers to work correctly. The issue for which we introduced that option no longer exists in this implementation.So I think this is a pure win :)