systemjs / plugin-css

CSS loader plugin
MIT License
92 stars 60 forks source link

Check for DOM in CSSPluginBase.prototype.instantiate() #113

Closed mikol-styra closed 7 years ago

mikol-styra commented 7 years ago

Fixes #111

guybedford commented 7 years ago

Let me know if you have any ideas moving forward here. Otherwise I'll get to it when I get to it.

mikol-styra commented 7 years ago

Wait. One thing that might be happening is that the test is generating a bundle.css file… Not sure why that isn’t checked in, etc. But the CSS is being built.

mikol-styra commented 7 years ago

So I think that this change is not breaking. The following test overwrites test/bundle.js, which is what I included in this PR.

it('Should support separateCSS: true and sourceMaps: false', function() {

If I skip this test (with xit), then I think bundle.js comes out as expected. Namely:

System.registerDynamic("test/data/test.css!css.js", [], false, function ($__require, $__exports, $__module) {
  var _retrieveGlobal = System.get("@@global-helpers").prepareGlobal($__module.id, null, null);

  (function ($__global) {})(this);

  return _retrieveGlobal();
});
(function(c){if (typeof document == 'undefined') return; var d=document,a='appendChild',i='styleSheet',s=d.createElement('style');s.type='text/css';d.getElementsByTagName('head')[0][a](s);s[a](d.createTextNode(c));})
("@import \"./dep.css\";body{background-color:red;background-image:url(test/data/x.png)}\n/*# sourceMappingURL=__.css.map */");

I’m not sure why bundle.js is checked in to begin with. But perhaps the tests should be updated to generate a separate bundle rather than overwriting the one file.

Let me know how you want to proceed, and I will update my PR accordingly.

Thanks!

guybedford commented 7 years ago

Ahh thanks, sorry yes I hadn't updated those files here.

Ok, looks great to me! Thanks again.