Use this.emitFile to generate css files

[shirotech]

Fixes #71

[elgertam]

This PR would be quite useful for a project I'm working on. I'm using Gulp with Rollup, and using fs.writeFileSync() in CssWriter.write() is a bit unexpected. Would strongly prefer to follow the Rollup conventions than to modify the filesystem in place.

[shirotech]

@elgertam I believe rollup can fully replace gulp, is there a specific reason you're using them together?

[elgertam]

@shirotech TL;DR: Gulp is very simple yet powerful, and is also debuggable. Rollup has a bit more complexity, but does code bundling very well.

I find that Gulp makes the build process very simple and understandable both to my team and me. I can commit a gulpfile.js, and my team of engineers and DevOps can understand it. If any of us doesn't understand something gulp-related, a 5-minute review of the documentation can typically resolve the issue. If that doesn't help, we can add a breakpoint in our gulpfile.js and begin debugging at almost any arbitrary point without much effort. Further, if there is some capability that doesn't exist as a Gulp plugin on NPM, I can usually roll something relatively quickly, given that Gulp is friendly with streams & Promises/async/await and has a simple and easy-to-understand interface. (Indeed, this is Rollup's recommended way to integrate with Gulp, and is how I accomplished it - no plugin required.)

Rollup is superb at orchestrating the build pipeline. But since it favors configuration slightly more than code (though not to the degree of, say, Webpack, which invokes magic to the point of sorcery), I find Rollup with a bunch of plugins a bit harder to navigate. While I am capable of digging through documentation and code for arbitrary plugins in order to figure out why my build isn't exactly what I expect, this process has a bit more friction in Rollup as the number of plugins increase. Also, if I have some task or process I wish to use that has not been implemented as a Rollup plugin, it's a bit harder for me to roll something myself, since the interface Rollup provides is quite a bit more complex than that for Gulp. (The complexity of Rollup's interface seems to be this PR's raison d'être.)

I strongly prefer my build code (and my production code) to be as simple as possible, but not simpler. I find Gulp + Rollup are a good mix.

[elgertam]

@shirotech I should add that regardless of my preferences, this PR fixes a fairly significant defect in the plugin. The status quo behavior would be a bit unexpected to me even if I was using Rollup without Gulp. I hope it's merged soon.

[shirotech]

@elgertam sorry I was just curious, I see that makes sense now, I do admit the setup for rollup can be complex, I've spent countless hours just to make it simple to the point where I gave up relying on 3rd party plugins and rolling our own versions.

Back on topic, yes hope this can be merged soon.

[benmccann]

@shirotech I'd love to get this merged. Can you take a look at why the tests are failing?

[benmccann]

@shirotech the fix to the test is relatively easy. You need to change the three calls to css.write in test/test.js to remove the path and just use the filename as you did in the README. E.g. change css.write('test/filename-test/dist/bundle.css'); to css.write('bundle.css');

[pngwn]

I fixed those tests, this looks good now.

[Conduitry]

Is there a way to do this that's also compatible with older versions of Rollup?

If not, since this is a breaking change anyway, is this now using the latest version of Rollup's asset handling APIs, and not one of the interim ones?

[pngwn]

Apparently this repo is using a version below 1? I must be missing something, this doesn't seem possible.

[pngwn]

Or did it get updated in this PR?

[pngwn]

It did. Ignore me.

[pngwn]

I believe emitFile with type set to 'asset' is the latest incarnation. emitAsset Was the previous one.

[benmccann]

I just tested. This PR only works with Rollup >= 1.19.2, which was released on Aug 5, 2019

[benmccann]

Merging this per discussion with @pngwn and @lukeed

[mustafa0x]

The change in this PR is breaking to 100% of users. Something needs to be done to notify them. For example, if there is a directory separator or if the directory to write to doesn't exist, warn that paths are now relative.