Only inline @import files if none will be skipped

[ryanfitzer]

The flattenCss function in optimize.js will skip imports that contain media queries with expressions:

@import url('somefile.css') screen and (min-width:768px)

This makes sense. The problem is that the function will still inline other imports that occur before ones that are skipped. These skipped @import rules become invalid and are ignored by browsers.

The W3C spec for CSS 2.1 states in section 4.1.5 At-rules:

CSS 2.1 user agents must ignore any @import rule that occurs inside a block or after any non-ignored statement other than an @charset or an @import rule.

flattenCss should first check if the file contains any imports that would be skipped before inlining imports.

[jrburke]

Makes sense, putting this in the 2.0 bucket as that is the current dev branch and likely next release.

[ryanfitzer]

Cool. I worked on a solution where all @import rules are first checked for expressions. It's about 10 extra lines in the function I can submit a pull request if you'd like.

Ryan

On May 12, 2012, at 10:50 PM, James Burkereply@reply.github.com wrote:

Makes sense, putting this in the 2.0 bucket as that is the current dev branch and likely next release.

---

Reply to this email directly or view it on GitHub:
#167 (comment)

[jrburke]

I'm thinking of changing the flattenCss so that it returns an object structure, that has two properties, whether an import was skipped, and any text that should be used for the inlining. Or something like that -- but basically, just do the file scanning/traversal once, and have a way to signal if the traversal and flattening should be discarded at the top level.

If you patch does something like that, yes please submit it.

[ryanfitzer]

Agreed, the file object makes sense.

For my current project, I've written a method to do targeted builds. It's a responsive design site with a lot of css files and a lot of @import rules with media queries. The method creates a files object and each file in the object looks like the following:

'/absolute/path/to/file.css': {
    hasMediaExprs: true,
    imports: {
        '/absolute/path/to/import-file.css': {
            rule: '@import url( \'404/600.css\' ) screen and ( min-width:600px );',
            mediaTypes: 'screen',
            mediaExprs: '( min-width:600px )'
        }
    }
}

The files object is created by looping through each file and getting its contents and then looping through each @import match in the contents to create the imports member.

Once the files object is built, I then loop through each file object and check the hasMediaExprs (for the current file and its imports). If everything is clear down the chain, I run requirejs.optimize() on the file. Any file that has media query expressions down the chain is ignored.

Each imports member has its rule broken out into mediaTypes and mediaExprs because I can see that a user may want the option to inline the import into an @media {} block. Having these properties will allow the block to be easily built.

If this object structure makes sense to you, I can integrate this into flattenCss so you could give it a spin. It's much more code than my original solution (which couldn't check a file imports recursively).

Thoughts?

[jrburke]

@ryanfitzer: I'm still new to the ins and outs of media queries, so your comment is helpful. I was thinking of just not doing any inlining if there was an @import with a media query. But I suppose it is possible to inline them if they are wrapped in an @media, is that right?

Maybe the inlining code could just always @media wrap the imported code? Do you think that would be too much to inline?

I'm trying to find the right balance of configurability vs. just doing the most likely best optimization. Also if there is just one @import that cannot be inlined, it seems a waste to not be able to inline anything.

Maybe I just need to "hoist" the @imports that cannot be inlined to the top of the file? I feel like that would break the CSS cascade, or rather change it vs. what the developer saw during development.

[ryanfitzer]

But I suppose it is possible to inline them if they are wrapped in an @media, is that right?

Correct.

Maybe the inlining code could just always @media wrap the imported code? Do you think that would be too much to inline?

The problem there is that you'd lose the ability to structure your files and their dependencies for best caching. Caching @import files has good support these days. We were taking the "inline everything" route originally (via LESS), but quickly realized that there was a lot of global css that could take advantage of browser caching.

With that said, inlining any @import would necessitate inlining all imports that come after it the order. Right now my solution simply ignores the whole file if media queries are found.

Maybe I just need to "hoist" the @imports that cannot be inlined to the top of the file? I feel like that would break the CSS cascade, or rather change it vs. what the developer saw during development.

Yeah, the hoisting would greatly conflict with any css that was media query inlined. The cascade become doubly important in this case.

I'm trying to find the right balance of configurability vs. just doing the most likely best optimization.

I agree. I'm all for minimal configuration. I previously read that you were thinking about breaking the css optimizer out into it's own tool. Having it be extendable to provide more options makes sense to me if that's still the case.

[jrburke]

I committed a fix for this in the near term, just not inline any @imports if they are found, see the commit above. I would like to leave this ticket open for a bit though to see how we could allow for some kind of config that would allow streamlined builds of targeted media queries -- in that case it would just remove the media queries that were not part of the target. Or maybe that does not make sense, and is trying to get too optimized. If you think so, feel free to close the ticket for now, but otherwise, let's keep talking it through.

I have decided for now to not break out the optimizer, see #102 for info, but that was more out of a "not sure how to do it vs. time needed/payoff to do so" decision, and I want to wrap up the code for a 2.0 release candidate.

Taking this out of the 2.0 milestone bucket for now, but not closing it yet.

[ryanfitzer]

Makes sense. I agree that more thought needs to be put into a config strategy. Here's where I'm at so far. I've written a targeted optimization routine that takes a first pass through each css file and turns:

@import url('some/file.css') screen and (min-width:600px);

into:

@media screen and (min-width:600px) {
@import 'some/file.css';
}

This file is then handed off to requirejs.optimize for inlining (via 1.0.8) as normal. I end up with a properly minified and concatenated file.

The sticky points reside in being able to have a strategy that allows one to configure which files should be inlined and which to preserve as an @import. This would be a non-issue if @import rules were allowed to fully participate in the cascade part of CSS instead of only being allowed to exist before all other styles (as previously discussed).

I'm mitigating this issue by how I organize my @import rule dependency chain. Only the top-level css file can declare a dependency that can be ignored. All of it's dependancies can import anything they want, as long as those imports are always inlined. While I have yet to write any error handling for this strategy, I intend to as it's easy to do via the files object I wrote about earlier.

Supporting a convention like this would allow for a simple config object. One that mimics the modules config would make for consistency:

cssModules: [
  {
      name: 'some/file.css',
      ignore: [
          'some/other/file.css',
          'and/another/file.css'
      ]
  }
]

The name property would act as the id that mirrors its index in the files object. The files object uses absoute paths, so name would have to be resolved in a reliable manner to create an absolute path. Adding a cssBaseUrl perhaps?

The ignore prop would default to a boolean with false as its value and inherit whatever routine is determined by value of optimizeCss. If ignore is true, the file is totally skipped. It might also make sense to allow any value that the cssImportIgnore property allows so to enable overwriting the global default on an individual basis.

This conventions feels somewhat analogous to how circular js dependencies are handled. Except with css, all a files dependancies would simply be ignored.

My build file is a little too tightly coupled to my project at the moment. I'm working to change that and will post an example to play around with soon.

Thoughts?

[jrburke]

I have been meaning to comment on this more, but I'm weak on media query rules.

I like the idea of doing an initial transform that converts the @import to a braced media thing with the @import inside. The cssModules approach that uses full paths seems to make sense too. Maybe cssLayers to indicate these are not module IDs but paths. A cssBaseUrl seems to make sense in that case.

I'm also wondering if there is way to use something like matchMedia to indicate what media queries should be kept in the build. But I'm just throwing out buzzwords, not sure how it all wires together.

[ryanfitzer]

I'll get a demo project together soon so you can give some feedback on what I've implemented.

Regarding using matchMedia, that is a browser-only API, as far as I know. It needs the window object to test queries against. It's useful for branching js behavior. Not sure it would help for building CSS.

On Jul 7, 2012, at 9:50 PM, James Burkereply@reply.github.com wrote:

I have been meaning to comment on this more, but I'm weak on media query rules.

I like the idea of doing an initial transform that converts the @import to a braced media thing with the @import inside. The cssModules approach that uses full paths seems to make sense too. Maybe cssLayers to indicate these are not module IDs but paths. A cssBaseUrl seems to make sense in that case.

I'm also wondering if there is way to use something like matchMedia to indicate what media queries should be kept in the build. But I'm just throwing out buzzwords, not sure how it all wires together.

---

Reply to this email directly or view it on GitHub:
#167 (comment)

[jrburke]

On the matchMedia, I was thinking more about using the same kind of way to specify what media queries to keep in a build, just as a way to reuse common apis, even though the implementation may be different. But it still might not make sense.

[ryanfitzer]

For css optimization config options, I propose storing them in a css object. When the css option is present, it supersede's optimizeCss and cssImportIgnore. This way both can be supported during transition (if they get deprecated).

Here are the basic options I feel are needed:

    css: {

        /*
            All css files are located relative to this path.
            If not explicitly set, then all files are loaded 
            relative to the directory that holds the build file.
            If `appDir` is set, then `css.baseUrl` should be
            specified as relative to the `appDir`.
        */
        baseUrl: 'css/',

        /*
            By default, each file is optimized with the following options:
            - @import inlining
            - line return removal
            - comment removal
        */
        optimize: {
            imports: false // Preserves @import rules
            lines: false, // Preserves line returns
            comments: false // Preserves comments
        },

        /*
            Array of of files to not optimize.
            The file paths are resolved relative to `css.baseUrl`.
        */
        ignore: [
            'path/to/file.css'
        ]
    }

I've left out equivalent support for cssImportIgnore because of the following caveats:

1. the @import rule to ignore must be positioned before all other @import rules:

   CSS 2.1 user agents must ignore any '@import' rule that occurs inside a block or after any non-ignored statement other than an @charset or an @import rule.

2. any file that directly or indirectly imports the ignored file will be ignored to prevent the first caveat

Another reason is due to browser performance. Files loaded via @import rules are blocked from loading until their parent file is fully loaded. I was originally for it, but now realize load blocking definitely outweighs the caching benefits.

_On Tue, Sep 4, 2012 at 9:02 PM, James Burke wrote_:
Do you know of any good CSS parsers, something that creates an AST or
token stream? I wonder if it is worth using something like that, if it
helps make for instance the config options easier to apply since we
can parse the CSS better.

Maybe it is not needed, CSS is more uniform in shape than say JS
though. Just thinking out loud.

Since we only need to find the @import strings, I think a CSS parser would be overkill. My regex still needs a little more work to properly tokenize @import rules that contain multiple media queries, but it's very solid otherwise.

[jrburke]

I wanted to get this in for 2.1, but my time window to work on 2.1 is closing, and there is enough in 2.1 as far as fixes that I want to get it out in this time window. I also wonder if it is better to prototype this as a separate tool. Sort of related to #102, in that it would be good to just use as a standalone tool. In any case, it unfortunately will not make the 2.1 window, and given my expectations of the day job over the next month, it is unlikely I'll have bandwidth to contribute much to this.