Add failing test for same file contents

[hgl]

I can confirm that the file is not imported.

[hgl]

PTAL

I manage to come up with a fix. The crux here is files containing any import rules should not be skipped.

[MoOx]

From what I understand you are getting some files that looks the same with import(s) + rules but imports refer to different files ?

[hgl]

Yep, exactly.

[hgl]

Also I now think skipping files silently might not be a good idea. It can cause some rule failed to be overridden.

For example

@import "foo/red.css";
@import "reset.css";
@import "bar/red.css";

and both foo/red.css and bar/red.css have this content:

body {
    background: url(red.png);
}

reset.css

body {
    background: none;
}

If bar/red.css is skipped, there will be no background on body. And even if the two red.css file have the same file, they refer to different images, so really shouldn't be skipped.

I think skipping files based on absolute url is good enough.

[MoOx]

Good point. Any objection @magsout ?

[hgl]

Well, just realize skipping files based on absolutely url is wrong too:

@import "foo/red.css";
@import "reset.css";
@import "foo/red.css";

I think for css, no files should be skipped.

[magsout]

Also I now think skipping files silently might not be a good idea. It can cause some rule failed to be overridden.

I understand that the test on the hash can introduce a bug on your example. Even though I do not consider it as a real bug. But I try to understand the relevance of your example. It's not really logical to do that, if I may? Or at compile we can specifie that the file is skipped because the same content as such a file.

[hgl]

@magsout, you can not expect users to be always logical and not make mistakes like this.

I think we should leave the job of removing unnecessary rules to a css compressor.

[MoOx]

I agree that @hgl example doesn't really make sense in practise.
Maybe we should add an option for this, so people can choose to enable or disable skipping.

@hgl the idea was (for example) to avoid having normalize.css several times if some modules use it.

And importing normalize (still for the example) can override previously defined values... So in both cases, so "bugs" are possible...

[MoOx]

@hgl you are the first to complain about this "feature", so I guess for now, we should just add an option to allow changing this behavior. What bout skipDuplicates (default to true, in order to add this as a minor change). PR welcome :)
Seems good to you @magsout ?

[magsout]

What bout skipDuplicates (default to true, in order to add this as a minor change). PR welcome :)

LGTM 👍

[hgl]

@MoOx what we are currently discussing is if files with the same contents (but contain no imports) should be skipped. But this PR is to not skipping files containing imports (the previous implementation seems to also do that, but with a plain /@import (.*);/ replace, it can fail in many cases).

I hope this PR is still good for merging.

[MoOx]

Yeap.

[hgl]

I still think skipping files with same content is a bad idea. I will open a new issue with a more practical example.

[MoOx]

#67 ?

[hgl]

Yep, I will add to that issue. writing it.