CaffeineOnIce / AutoMinify

A VSCode extension to minify HTML, CSS and JS files.
https://marketplace.visualstudio.com/items?itemName=CaffeineOnIce.autominify
MIT License
2 stars 1 forks source link

Removing "@import url();" from css file #14

Open moje-s opened 7 months ago

moje-s commented 7 months ago

After css file minification there is missing the "@import url();".

CaffeineOnIce commented 7 months ago

can you explain a bit more? Is it for every minification? is the import not working?

moje-s commented 7 months ago

Whole part of code "@import url('dir/file.css');" is missing, like it was removed while minification.

Yes, it happens every time.

CaffeineOnIce commented 7 months ago

@owhs can you check if it occurs with you? I can't seem to replicate in mine

owhs commented 7 months ago

Yes, I could replicate.

By logging the output, it says:

"errors":["Ignoring local @import of \"app.css\" as resource is missing."]

Fixed by adding in the CSS settings.json:

"inline": ["none"]

My css settings now read:

"autominify.cssMinifierOptions": {
    "inline": ["none"],
    "level": {
        "1": {
            "all": true
        }
    }
}

And works as expected 😊 Let me know if this resolves it for you @moje-s

owhs commented 7 months ago

Whole part of code "@import url('dir/file.css');" is missing, like it was removed while minification.

Yes, it happens every time.

By doing the above steps, it will just include the @import lines "as is", there's a method to "inline" the imports, but personally I wouldn't like that behaviour, hopefully you agree :)

CaffeineOnIce commented 7 months ago

@moje-s check if owhs' method works for you. if it does, do close the issue.

owhs commented 7 months ago

@moje-s check if owhs' methods works for you. if it does, do close the issue.

SSSfine for now, but for the record, the options break when you chose when you chose level 0 or 2, At a guess, you're doing an object.assign from the default optionObj with the user options So it still keeps the default values for level 1

As I said, it's not really an "issue" - more nuanced if people wanted to use the other levels, as 1 is fine for 99% use cases :) No point spending time looking into fixing an issue that nobody experiences, but if you ever have someone mention it, that's the behaviour behind it.

moje-s commented 7 months ago
"autominify.cssMinifierOptions": {
    "inline": ["none"],
    "level": {
        "1": {
            "all": true
        }
    }
}

Almost good, result is without ' or " in path - @import url(dir/file.css); I tested it for both ' and ".

CaffeineOnIce commented 7 months ago

wait, level 1 doesn't get removed? It should work. Did you remove the entire level 1 code and replaced it with level 2?

moje-s commented 7 months ago

Still the same.

CaffeineOnIce commented 7 months ago

@owhs mmm. Weird. Replacing the level 1 code for level 2 works for me. I'll check the code tomorrow. If you find something, you can pull it. I'll merge the update.

owhs commented 7 months ago

Still the same.

 @import url('app.css');
 body{
     background: #fff;}

works fine, but then:

body{
    background: #fff;}
@import url('app.css');

the import disappears, are all your imports at the very top?

if not, probs will have to push an update that logs debug info to the output panel to be able to diagnose further

CaffeineOnIce commented 7 months ago

yeah, I'll look into that by today. if not, by tomorrow.

moje-s commented 7 months ago
@charset "UTF-8";

@import url("file.css");

1st and 3rd line.

owhs commented 7 months ago
@charset "UTF-8";

@import url("file.css");

1st and 3rd line.

Ah it's likely the @ charset line, there'll prbs be another arg for other @ queries, I'll look tmr or over weekend some point

2 points;

  1. Does it still fail if you remove the utf charset line?
  2. Why do you even need to specify UTF-8? Does it break your site if you don't have that line? Do you have special characters in your CSS- if so, you should probs escape those values :)
moje-s commented 7 months ago
  1. Yes, it still generate file without ' or ".
  2. There aren't any special characters but the UTF-8 encoding is for older browsers.

By the way.

@charset "UTF-8";

@viewport {
    width: device-width;
}

@import url("fonts.css");

Gives:

@charset "UTF-8";@viewport{width:device-width;}

This:

@viewport {
    width: device-width;
}

@import url("fonts.css");

Gives this:

@viewport{width:device-width;}

And this:

@charset "UTF-8";

@import url("fonts.css");

@viewport {
    width: device-width;
}

Gives this:

@charset "UTF-8";@import url(fonts.css);@viewport{width:device-width;}
CaffeineOnIce commented 7 months ago

@owhs you think it's a clean-css problem?

owhs commented 7 months ago

@owhs you think it's a clean-css problem?

I came to this conclusion, after lots of messing about with arguments. not a "problem" per say, as other up to date minifiers seem to have very similar logic I think most are aimed at compressing entire projects rather than files at a time, so they aggressively try to resolve and sort things

The most modern lib is cssnano build on postcss, but i couldn't get it to load ;/ I used the-minifier and it works as intended, but i've not done extensive testing on more modern media, nested etc

https://www.npmjs.com/package/@minify-html/wasm Also looks good as a universal tool to use, looks easy enough to to implement - https://github.com/wilsonzlin/minify-html#use-3 Speed hasn't been an issue for me, but this could offer a nice boost to the speed :)

CaffeineOnIce commented 7 months ago

the-minifier hasn't been updated over 2 years. Isn't feasible to rework the code for a package that hasn't been updated only for it to have bugs and stuff to be found not working. I looked at lightning-css but I didn't understand jackshit of its api and docs.

CaffeineOnIce commented 7 months ago

minify-html is a good one, but it's a 2 in 1 stuff. Both CSS and HTML apparently. Should I change it? @owhs

owhs commented 7 months ago

Owner

postcss comments about charset

Loosely related -- https://github.com/CaffeineOnIce/AutoMinify/issues/8, In my tests, lightningcss was fast but had similar output to cleancss -- you can see why in above link :)

So my latest PR keeps cleancss, and adds the minifier as a legacy option

ran test, passed 👍