CSS Optimizer will include same file multiple times

[Karl Tiedt]

This seems a bit counter intuitive, but maybe this is necessary on some level, but opening a ticket just in case its an oversight.

If you have

a.css:
@import "common.css";

more/b.css:
@import "../common.css":

In a build where both a.css and more/b.css are included, common.css will be flattened and inlined twice....

Tested against trunk build utils from yesterday afternoon.

[bill]

Does the output of the build contain both a flattened a.css file and a flattened more/b.css file? If so, I can see why common.css is included twice, because some pages may just be importing a.css while other pages may just be importing b.css.

[Karl Tiedt]

No, I tested it by creating a new CSS file, imported it from our main CSS file (which is the only CSS file included by our apps page -- everything else is a nested import from it)

Then I added the new CSS file to an import in one of the 2nd or 3rd level files and ran a build.

In the end we have a single top level CSS file. with some duplicate content in this case.

[Rawld Gill]

Replying to ktiedt:

No, I tested it by creating a new CSS file, imported it from our main CSS file (which is the only CSS file included by our apps page -- everything else is a nested import from it)

Then I added the new CSS file to an import in one of the 2nd or 3rd level files and ran a build.

In the end we have a single top level CSS file. with some duplicate content in this case.

I would like to knock this out for 1.8, but I confess I don't follow what you're saying. Could you work up a quick, small example?

[Karl Tiedt]

I dont have the example I did here at home but it was quite simple:

app.css

.app { color: red; }
@import twice.css;
@import once.css;

once.css

.once { color: blue; }
@import twice.css;

twice.css

.twice { color: green; }

results in:

.app { color: red; }
.twice { color: green; }
.once { color: blue; }
.twice { color: green; }

[Rawld Gill]

Replying to ktiedt:

I dont have the example I did here at home but it was quite simple:

OK. But the flattening routines are constructing exactly what the input says. I don't see how this is an error in the routines. Won't the browser make the same calculations? (Not sure if this affects what you're trying to show, but, fwiw, I think @import is supposed to precede all other rules.)

afaik, this is how the css flattening has always worked.

[Karl Tiedt]

Would it be possible to get a flag added to do this? I understand your last response, but it would be extremely beneficial in build sizes to be able to dump dupe css files instead of inlining them multiple times

Essentially, resolve CSS paths, then give dupes lowest in the list priority and remove the top most dupes (that way any CSS overrides would still work as expected.

[Rawld Gill]

Replying to ktiedt:

Would it be possible to get a flag added to do this? I understand your last response, but it would be extremely beneficial in build sizes to be able to dump dupe css files instead of inlining them multiple times

Essentially, resolve CSS paths, then give dupes lowest in the list priority and remove the top most dupes (that way any CSS overrides would still work as expected.

Yes, this could be done. It will require some work because the current algorithm builds the optimized CSS file recursively on the fly, so it is not structured to easily go back and remove a possibly-deeply-nested import. Short answer: it won't make 1.8, but i'll look at after the release.

[Karl Tiedt]

Yeah obviously not 1.8 -- maybe our understanding of the build system is overly simplified... from what little we have had to dig into it our understanding of how *all* file processing was done was that each file was scanned and added to the list of "to write" files before anything else happened... then this list of files was used to generate the flattened file (since it would already be in the proper order due to the recursive scan)

(so maybe this sounds simpler in our heads than it really does in code lol)

Thanks for the quick reply.

[Rawld Gill]

Replying to ktiedt:

Yeah obviously not 1.8 -- maybe our understanding of the build system is overly simplified... from what little we have had to dig into it our understanding of how *all* file processing was done was that each file was scanned and added to the list of "to write" files before anything else happened... then this list of files was used to generate the flattened file (since it would already be in the proper order due to the recursive scan

How the CSS is optimized is really a function of util/build/transforms/optimizeCss.js--the CSS transform--more than the build program as a whole (though the new build program allowed us to output CSS to different tree structures than the input, which was impossible in v1.6-, see ​http://bugs.dojotoolkit.org/ticket/14789#comment:11).

That transform evolved from 1.6- which has this recursive algorithm that transformed the CSS on-the-fly rather than the two-pass algorithm you suggest.

As I see it, it's not possible to do this enhancement with the existing algorithm. Your sketch is exactly how I would proceed. It's not hard. But it's essentially a rewrite of that module which is why it will cost somebody the time to build it, test it, doc it, and receive all the complaining about yet another build flag :).

Don' get me wrong...I think this is a reasonable request...just explaining why it's not a trivial chore.

[dylan]

Given the rise of preprocessors and the changes to how builds will likely work for 2.x, I'm closing this out since it won't get addressed for 1.x.