fix: styleExport mode side-effects

- Disables source maps if they aren't explicitly requested - Disables file emitting - Warns if using done hook since it'll never be called
tivac · Jul 18, 2018 · 4b67b7b · 4b67b7b
1 parent 77f5e7a
commit 4b67b7b
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 6 deletions.
diff --git a/packages/rollup/rollup.js b/packages/rollup/rollup.js
@@ -12,7 +12,7 @@ const output    = require("modular-css-core/lib/output.js");
 
 // sourcemaps for css-to-js don't make much sense, so always return nothing
 // https://github.com/rollup/rollup/wiki/Plugins#conventions
-const map = {
+const emptyMappings = {
     mappings : "",
 };
 
@@ -25,7 +25,6 @@ module.exports = function(opts) {
         common : "common.css",
 
         json : false,
-        map  : true,
 
         include : "**/*.css",
 
@@ -35,14 +34,33 @@ module.exports = function(opts) {
     }, opts);
 
     const filter = utils.createFilter(options.include, options.exclude);
-
+
+    const { styleExport, done, map } = options;
+
+    if(typeof map === "undefined") {
+        // Sourcemaps don't make much sense in styleExport mode
+        // But default to true otherwise
+        options.map = !styleExport;
+    }
+
     const processor = options.processor || new Processor(options);
 
     let runs = 0;
 
     return {
         name : "modular-css-rollup",
 
+        buildStart() {
+            // done lifecycle won't ever be called on per-component styles since
+            // it only happens at bundle compilation time
+            // Need to do this on buildStart so it has access to this.warn() o_O
+            if(styleExport && done) {
+                this.warn(
+                    `Any plugins defined during the "done" lifecycle won't run when "styleExport" is set!`
+                );
+            }
+        },
+
         async transform(code, id) {
             if(!filter(id)) {
                 return null;
@@ -108,7 +126,7 @@ module.exports = function(opts) {
 
             return {
                 code : out.join("\n"),
-                map,
+                map  : emptyMappings,
                 dependencies,
             };
         },
@@ -119,6 +137,11 @@ module.exports = function(opts) {
         },
 
         async generateBundle(outputOptions, bundles) {
+            // styleExport disables all output file generation
+            if(styleExport) {
+                return;
+            }
+
             const usage = new Map();
             const common = new Map();
             const files = [];
@@ -205,7 +228,7 @@ module.exports = function(opts) {
                     });
                 }
             }
-
+            
             await Promise.all(
                 files
                 .filter(({ css }) => css.length)

diff --git a/packages/rollup/test/__snapshots__/rollup.test.js.snap b/packages/rollup/test/__snapshots__/rollup.test.js.snap
@@ -191,7 +191,7 @@ console.log(a, str, num, dim, mix);
 `;
 
 exports[`/rollup.js should provide style export 1`] = `
-"var styles = \\".ooh {\\\\n    content: \\\\\\"string\\\\\\";\\\\n}\\\\n\\\\n/*# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbInBhY2thZ2VzL3JvbGx1cC90ZXN0L3NwZWNpbWVucy9zdHlsZS1leHBvcnQuY3NzIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQUVBO0lBRkEsa0JBQXFCO0NBSXBCIiwiZmlsZSI6InBhY2thZ2VzL3JvbGx1cC90ZXN0L3NwZWNpbWVucy9zdHlsZS1leHBvcnQuY3NzIiwic291cmNlc0NvbnRlbnQiOlsiQHZhbHVlIHN0cjogXCJzdHJpbmdcIjtcblxuLm9vaCB7XG4gICAgY29udGVudDogc3RyO1xufVxuIl19 */\\";
+"var styles = \\".ooh {\\\\n    content: \\\\\\"string\\\\\\";\\\\n}\\\\n\\";
 
 console.log(styles);
 "
@@ -240,6 +240,14 @@ console.log(css, fooga);
 "
 `;
 
+exports[`/rollup.js should warn that styleExport and done aren't compatible 1`] = `
+Array [
+  Array [
+    "Any plugins defined during the \\"done\\" lifecycle won't run when \\"styleExport\\" is set!",
+  ],
+]
+`;
+
 exports[`/rollup.js shouldn't disable sourcemap generation 1`] = `
 SourceMap {
   "file": "simple.js",

diff --git a/packages/rollup/test/rollup.test.js b/packages/rollup/test/rollup.test.js
@@ -194,6 +194,28 @@ describe("/rollup.js", () => {
         expect(result.code).toMatchSnapshot();
     });
 
+    it("should warn that styleExport and done aren't compatible", async () => {
+        const spy = jest.spyOn(global.console, "warn");
+
+        spy.mockImplementation(() => { /* NO-OP */ });
+
+        await rollup({
+            input   : require.resolve("./specimens/style-export.js"),
+            plugins : [
+                plugin({
+                    namer,
+                    styleExport : true,
+                    done        : [
+                        () => { /* NO OP */ },
+                    ],
+                }),
+            ],
+        });
+
+        expect(spy).toHaveBeenCalled();
+        expect(spy.mock.calls).toMatchSnapshot();
+    });
+
     it("should generate external source maps", async () => {
         const bundle = await rollup({
             input   : require.resolve("./specimens/simple.js"),