postcss / postcss-import

PostCSS plugin to inline at-import rules content
MIT License
1.38k stars 115 forks source link

Error on embedded imports in data urls #543

Closed romainmenke closed 1 year ago

romainmenke commented 1 year ago

The current code prevents further inlining of embedded imports but it doesn't stop a browser from possible loading and applying it.

Adding not all has the intended effect.

Setting stmt.children = [] prevents inlining and removes the @import statement.

RyanZim commented 1 year ago

If such imports are completely useless, why do we output useless imports vs. erroring?

romainmenke commented 1 year ago

A warning would help users to write more effective CSS.

But a hard error might be too much as this is perfectly valid CSS, it's just CSS that isn't applied.

This is similar to the warning for empty files. Empty files are valid, just not very useful.

romainmenke commented 1 year ago

I've added a suggestion for a warning, please let me know if this is clear.


An alternative would be to warn and completely remove the @import node.

But that would require a larger refactor, adding not all is a non-intrusive and technically correct way to ignore these @import statements.


Update :

I found that setting stmt.children to an empty array is an effective way to prevent inlining.

RyanZim commented 1 year ago

Needs rebase.

But a hard error might be too much as this is perfectly valid CSS, it's just CSS that isn't applied.

In other words, you're saying browsers ignore imports like these? I'm not sure if that's a valid premise to build on, because browsers will continue to parse the CSS they can, even if an import results in a 404; however, as a build tool, we'd be remiss not to error out if we can't resolve an import.

romainmenke commented 1 year ago

we'd be remiss not to error out if we can't resolve an import.

Makes sense! I've changed it to an error.

Thank you for reviewing these 🙇