Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should clean-css read CSS from the file system when passing the input as hash? #791

Closed
stanislawosinski opened this issue Jun 28, 2016 · 6 comments
Milestone

Comments

@stanislawosinski
Copy link

stanislawosinski commented Jun 28, 2016

I've been trying to get inlining of imports to work with metalsmith-clean-css, which passes the contents of the CSS files to clean as a hash:

new CleanCSS().minify({
  'path/to/file/one': {
    styles: 'contents of file one'
  },
  'path/to/file/two': {
    styles: 'contents of file two'
  }
});

I've skimmed clean-css code very quickly and it appears that clean-css still tries to read the CSS text from the file system in this case. Shouldn't it instead use the contents provided in the data hash? In the specific case of metalsmith-clean-css, reading from the file system won't work because the files are not there -- all content is processed in-memory and flushed to disk once all processing is complete.

@jakubpawlowicz
Copy link
Collaborator

Here are some tests when input is passed as a hash: https:/jakubpawlowicz/clean-css/blob/master/test/module-test.js#L480 and code processing input hashes: https:/jakubpawlowicz/clean-css/blob/master/lib/utils/source-reader.js#L60.

I am pretty sure clean-css does not try to load those files from disk.
Could you please double check?

@stanislawosinski
Copy link
Author

This is probably my misunderstanding of the hash data feature. I assumed that when given multiple CSS files on input:

new CleanCSS().minify({
  'one.css: {
    styles: '@import url("extra/two.css");'
  },
  'extra/two.css': {
    styles: 'h1 { color: red }'
  }
});

clean-css would resolve the import without reading two.css from the file system. A test case would look similar to:

    'with more imports': {
      'topic': function () {
        var prefix = "test/fixtures/partials/";
        var hash = sourcesAsHash([prefix + 'two.css', prefix + "one.css", prefix + 'extra/three.css', prefix + 'extra/four.css']);
        var data = {};
        for (var src in hash) {
          data[src.substring(prefix.length)] = hash[src];
        }
        return new CleanCSS().minify(data);
      },
      'should give right output': function (minified) {
        assert.equal(minified.errors.length, 0);
        assert.equal(minified.styles, '.one{color:red}.three{color:#0f0}.four{color:#00f}.two{color:#fff}');
      }
    }

I'm assuming that's not the way clean-css was designed to work?

@jakubpawlowicz
Copy link
Collaborator

Interesting use case. Why using it that way? Is that a metalsmith-specific flow?

@stanislawosinski
Copy link
Author

stanislawosinski commented Jun 30, 2016

Metalsmith runs each file it processes through a pipeline of transformers. In case of my setup and CSS processing, the pipeline consists of the Less processor and clean-css. Metalsmith does all processing in-memory, it does not materialize the intermediate results (*.less transformed to *.css) to the file system. For this reason, the current approach to import inlining won't work -- the files being imported are not there physically on the disk. Since the content of Less transformed to CSS is available in memory, to make import inlining work, the imports would have to be resolved based on the paths and CSS contents provided in the hash input data (obviously, the caller would have to pass entries containing path and content of each imported CSS).

@jakubpawlowicz jakubpawlowicz added this to the 4.0 milestone Dec 12, 2016
jakubpawlowicz added a commit that referenced this issue Dec 26, 2016
Why:

* When a hash is given as a source, some imports can be inlined
  in-memory taking styles from the hash.
jakubpawlowicz added a commit that referenced this issue Dec 26, 2016
Why:

* when using a hash as an input, the sources it references
  should be processed lazily instead of a whole hash being
  serialized & tokenized eagerly.

This change addresses the need by:

* See #791 - in-memory imports require such lazy processing.
  The other way, we end up with duplicated rules, and have to rely
  on advanced processing to remove them.
jakubpawlowicz added a commit that referenced this issue Dec 26, 2016
Why:

* when using a hash as an input, the sources it references
  should be processed lazily instead of a whole hash being
  serialized & tokenized eagerly.

This change addresses the need by:

* See #791 - in-memory imports require such lazy processing.
  The other way, we end up with duplicated rules, and have to rely
  on advanced processing to remove them.
@jakubpawlowicz
Copy link
Collaborator

It's ready on master now and due in 4.0 release. Thanks @stanislawosinski for the idea!

@stanislawosinski
Copy link
Author

Thanks @jakubpawlowicz! I'll give clean-css a try once version 4.0 is out.

jakubpawlowicz added a commit that referenced this issue Jan 9, 2017
Stores all paths internally as normalized to *nix format but
still outputs Windows friendly paths if needed.

`source-map` does it internally so there's no need to transform
paths in source maps.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants