From 8f279bea4232c1a7f8d006cab6f6b8232d9c8ab0 Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Mon, 26 Nov 2018 13:54:19 -0800 Subject: [PATCH 01/13] WIP: sorta working Broken for dynamic imports though, they're missing info we really need atm. --- packages/browserify/browserify.js | 32 ++-- packages/processor/lib/output.js | 7 +- packages/processor/processor.js | 63 +++++--- packages/rollup/rollup.js | 150 +++++++++--------- .../test/__snapshots__/splitting.test.js.snap | 20 +-- packages/rollup/test/splitting.test.js | 38 ++--- 6 files changed, 157 insertions(+), 153 deletions(-) diff --git a/packages/browserify/browserify.js b/packages/browserify/browserify.js index c708f8f3a..65b1ed876 100644 --- a/packages/browserify/browserify.js +++ b/packages/browserify/browserify.js @@ -44,7 +44,7 @@ module.exports = (browserify, opts) => { function depReducer(curr, next) { curr[prefixed(options.cwd, next)] = next; - + return curr; } @@ -84,9 +84,9 @@ module.exports = (browserify, opts) => { processor.dependencies(result.id).forEach((dep) => browserify.emit("file", dep, dep) ); - + push(outputs(result)); - + done(); }, @@ -94,9 +94,9 @@ module.exports = (browserify, opts) => { // Thrown from the current bundler instance, NOT the main browserify // instance. This is so that watchify won't explode. bundler.emit("error", error); - + push(buffer); - + done(); } ); @@ -108,24 +108,24 @@ module.exports = (browserify, opts) => { if(path.extname(row.file) !== options.ext) { return done(null, row); } - + handled[row.id] = true; - + // Ensure that browserify knows about the CSS dependency tree by updating // any referenced entries w/ their dependencies row.deps = processor.dependencies(row.file).reduce(depReducer, {}); - + return done(null, row); }, function(done) { // Ensure that any CSS dependencies not directly referenced are // injected into the stream of files being managed const push = this.push.bind(this); - + processor.dependencies().forEach((dep) => { if(dep in handled) { return; } - + push({ id : path.resolve(options.cwd, dep), file : path.resolve(options.cwd, dep), @@ -133,7 +133,7 @@ module.exports = (browserify, opts) => { deps : processor.dependencies(dep).reduce(depReducer, {}), }); }); - + done(); })); @@ -169,24 +169,24 @@ module.exports = (browserify, opts) => { // in case things have changed out from under us, like when using watchify bundles = {}; handled = {}; - + // cache set to false means we need to create a new Processor each run-through if(!options.cache) { processor = new Processor(options); } bundler = current; - + // Listen for bundling to finish bundler.on("end", () => { const bundling = Object.keys(bundles).length > 0; if(options.json) { mkdirp.sync(path.dirname(options.json)); - + fs.writeFileSync( options.json, - JSON.stringify(output.compositions(options.cwd, processor), null, 4) + JSON.stringify(output.compositions(processor), null, 4) ); } @@ -231,7 +231,7 @@ module.exports = (browserify, opts) => { if(!common.length && !options.empty) { return Promise.resolve(); } - + return write(bundling && common, options.css); }); }); diff --git a/packages/processor/lib/output.js b/packages/processor/lib/output.js index 54bdfda08..6f598e6b6 100644 --- a/packages/processor/lib/output.js +++ b/packages/processor/lib/output.js @@ -11,14 +11,15 @@ exports.join = (output) => classes.toString() )); -exports.compositions = (cwd, { files }) => { +exports.compositions = ({ options, files }) => { + const { cwd } = options; const json = {}; - + Object.keys(files) .sort() .forEach((file) => (json[relative(cwd, file)] = exports.join(files[file].exports)) ); - + return json; }; diff --git a/packages/processor/processor.js b/packages/processor/processor.js index 40c9078b6..c461a3d13 100644 --- a/packages/processor/processor.js +++ b/packages/processor/processor.js @@ -2,7 +2,7 @@ const fs = require("fs"); const path = require("path"); - + const Graph = require("dependency-graph").DepGraph; const postcss = require("postcss"); const slug = require("unique-slug"); @@ -106,28 +106,28 @@ class Processor { // Add a file on disk to the dependency graph file(file) { const id = this._normalize(file); - + this._log("file()", id); - + return this._add(id, fs.readFileSync(id, "utf8")); } - + // Add a file by name + contents to the dependency graph async string(file, text) { const id = this._normalize(file); - + this._log("string()", id); return this._add(id, text); } - + // Remove a file from the dependency graph remove(input) { // Only want files actually in the array const files = (Array.isArray(input) ? input : [ input ]) .map(this._normalize) .filter((file) => this._graph.hasNode(file)); - + if(!files.length) { return files; } @@ -142,7 +142,7 @@ class Processor { return files; } - + // Get the dependency order for a file or the entire tree dependencies(file) { if(file) { @@ -153,7 +153,7 @@ class Processor { return this._graph.overallOrder(); } - + // Get the dependant files for a file dependents(file) { if(!file) { @@ -161,21 +161,21 @@ class Processor { } const id = this._normalize(file); - + return this._graph.dependantsOf(id); } - + // Get the ultimate output for specific files or the entire tree async output(args = false) { let { files } = args; - + if(!Array.isArray(files)) { files = tiered(this._graph); } // Throw normalize values into a Set to remove dupes files = new Set(files.map(this._normalize)); - + // Then turn it back into array because the iteration story is better files = [ ...files.values() ]; @@ -195,7 +195,7 @@ class Processor { // Have to do this every time because target file might be different! // const results = []; - + for(const dep of files) { // eslint-disable-next-line no-await-in-loop const result = await this._after.process( @@ -203,7 +203,7 @@ class Processor { // modifies the .result root itself and you process URLs multiple times // See https://github.com/tivac/modular-css/issues/35 this._files[dep].result.root.clone(), - + params(this, { from : dep, to : args.to, @@ -239,9 +239,9 @@ class Processor { }); root.append([ comment ].concat(result.root.nodes)); - + const idx = root.index(comment); - + // Need to manually insert a newline after the comment, but can only // do that via whatever comes after it for some reason? // I'm not clear why comment nodes lack a `.raws.after` property @@ -251,17 +251,21 @@ class Processor { root.nodes[idx + 1].raws.before = "\n"; } }); - + const result = await this._done.process( root, params(this, args) ); - result.compositions = output.compositions(this._options.cwd, this); - + Object.defineProperty(result, "compositions", { + get() { + return output.compositions(this); + } + }); + return result; } - + // Expose files get files() { return this._files; @@ -272,6 +276,15 @@ class Processor { return this._options; } + // Return all the compositions for the files loaded into the processor instance + get compositions() { + // Ensure all files are fully-processed first + return Promise.all( + Object.values(this._files).map(({ result }) => result) + ) + .then(() => output.compositions(this)); + } + // Take a file id and some text, walk it for dependencies, then // process and return details async _add(id, text) { @@ -298,9 +311,9 @@ class Processor { // eslint-disable-next-line no-await-in-loop file.result = await file.processed; - + const { result } = file; - + file.exports = Object.assign( Object.create(null), // export @value entries @@ -350,7 +363,7 @@ class Processor { }) ), }; - + await file.result; // Add all the found dependencies to the graph @@ -371,7 +384,7 @@ class Processor { if(!this._files[dependency]) { promises.push(this.file(dependency)); } - + return promises; }, []) ); diff --git a/packages/rollup/rollup.js b/packages/rollup/rollup.js index 5d5d8137c..4324d467c 100644 --- a/packages/rollup/rollup.js +++ b/packages/rollup/rollup.js @@ -1,4 +1,4 @@ -/* eslint max-statements: [ 1, 20 ] */ +/* eslint max-statements: [ 1, 30 ] */ "use strict"; const path = require("path"); @@ -9,7 +9,7 @@ const dedent = require("dedent"); const slash = require("slash"); const Processor = require("@modular-css/processor"); -const output = require("@modular-css/processor/lib/output.js"); +const output = require("@modular-css/processor/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 @@ -37,20 +37,20 @@ module.exports = (opts) => { dev : false, verbose : false, }, opts); - + const filter = utils.createFilter(options.include, options.exclude); const { styleExport, done, map, dev, verbose } = options; // eslint-disable-next-line no-console, no-empty-function - const log = verbose ? console.log.bind(console, "[rollup]") : () => {}; + const log = verbose ? console.log.bind(console, "[rollup]") : () => { }; 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); return { @@ -70,7 +70,7 @@ module.exports = (opts) => { }, watchChange(file) { - if(!processor.files[file]) { + if(!processor.files[ file ]) { return; } @@ -95,6 +95,7 @@ module.exports = (opts) => { const exported = output.join(exports); const out = [ + ...processor.dependencies(id).map((dep) => `import ${JSON.stringify(dep)};`), dev ? dedent(` const data = ${JSON.stringify(exported)}; @@ -106,13 +107,13 @@ module.exports = (opts) => { throw new ReferenceError( key + " is not exported by " + ${JSON.stringify( - slash(path.relative(process.cwd(), id)) - )} + slash(path.relative(process.cwd(), id)) + )} ); } }) `) : - `export default ${JSON.stringify(exported, null, 4)};`, + `export default ${JSON.stringify(exported, null, 4)};`, ]; if(options.namedExports) { @@ -145,11 +146,10 @@ module.exports = (opts) => { } const usage = new Map(); - const common = new Map(); - const files = []; + const files = new Map(); const { assetFileNames = "" } = outputOptions; - + let to; if(!outputOptions.file && !outputOptions.dir) { @@ -163,12 +163,16 @@ module.exports = (opts) => { // First pass is used to calculate JS usage of CSS dependencies Object.entries(bundles).forEach(([ entry, bundle ]) => { - const file = makeFile({ + const name = path.basename(entry, path.extname(entry)); + + const file = { entry, - css : new Set(), - }); + name, + base : path.join(path.dirname(entry), name), + css : new Set(), + }; - const { modules } = bundle; + const { imports, modules } = bundle; // Get CSS files being used by each entry point const css = Object.keys(modules).filter(filter); @@ -181,91 +185,81 @@ module.exports = (opts) => { ]; used.forEach((dep) => { - file.css.add(dep); - if(!usage.has(dep)) { usage.set(dep, new Set()); } - usage.get(dep).add(entry); + const users = usage.get(dep).add(entry); + + // CSS included in this chunk only if not already included + // by one of its imports + if(imports.some((f) => users.has(f))) { + return; + } + + file.css.add(dep); }); }); - files.push(file); + files.set(entry, file); }); - if(files.length === 1) { - // Only one entry file means we only need one bundle. - files[0].css = new Set(processor.dependencies()); - } else { - // Multiple bundles means ref-counting to find the shared deps - // Second pass removes any dependencies appearing in multiple bundles - files.forEach((file) => { - const { css } = file; + // TODO: Ensure that all CSS files only appear in a single bundle - file.css = new Set([ ...css ].filter((dep) => { - if(usage.get(dep).size > 1) { - common.set(dep, true); + console.log("BUNDLES"); + Object.entries(bundles).forEach(([ key, bundle ]) => { + console.log(key); + // console.log(Object.keys(modules)); + console.log(bundle); + }); + console.log("\n\n\n"); - return false; - } + console.log("CSS"); + console.log(files); + console.log("\n\n\n"); - return true; - })); - }); + console.log("USAGE"); + console.log(usage); - // Add any other files that weren't part of a bundle to the common chunk - Object.keys(processor.files).forEach((file) => { - if(!usage.has(file)) { - common.set(file, true); - } - }); - - // Common chunk only emitted if necessary - if(common.size) { - files.push(makeFile({ - entry : options.common, - css : new Set([ ...common.keys() ]), - })); + for(const [ , { base, name, css }] of files) { + if(!css.size) { + continue; } - } - await Promise.all( - files - .filter(({ css }) => css.size) - .map(async ({ base, name, css }, idx) => { - const id = this.emitAsset(`${base}.css`); + const id = this.emitAsset(`${base}.css`); - log("css output", id); + log("css output", id); - const result = await processor.output({ - to : to.replace(/\[(name|extname)\]/g, (match, field) => - (field === "name" ? name : ".css") - ), - files : [ ...css ], - }); + /* eslint-disable-next-line no-await-in-loop */ + const result = await processor.output({ + to : to.replace(/\[(name|extname)\]/g, (match, field) => + (field === "name" ? name : ".css") + ), + files : [ ...css ], + }); - this.setAssetSource(id, result.css); + this.setAssetSource(id, result.css); - // result.compositions always includes all the info, so it - // doesn't actually matter which result we use. First one seems reasonable! - if(options.json && idx === 0) { - const file = typeof options.json === "string" ? options.json : "exports.json"; + if(result.map) { + const dest = `${base}.css.map`; - log("json output", file); - - this.emitAsset(file, JSON.stringify(result.compositions, null, 4)); - } + log("map output", dest); - if(result.map) { - const file = `${base}.css.map`; + this.emitAsset(dest, result.map.toString()); + } + } - log("map output", file); + // result.compositions always includes all the info, so it + // doesn't actually matter which result we use. First one seems reasonable! + if(options.json) { + const dest = typeof options.json === "string" ? options.json : "exports.json"; - this.emitAsset(file, result.map.toString()); - } - }) - ); + log("json output", dest); + + const compositions = await processor.compositions; + + this.emitAsset(dest, JSON.stringify(compositions, null, 4)); + } }, }; }; diff --git a/packages/rollup/test/__snapshots__/splitting.test.js.snap b/packages/rollup/test/__snapshots__/splitting.test.js.snap index 820b11722..770c43145 100644 --- a/packages/rollup/test/__snapshots__/splitting.test.js.snap +++ b/packages/rollup/test/__snapshots__/splitting.test.js.snap @@ -3,7 +3,7 @@ exports[`/rollup.js code splitting should ouput only 1 JSON file 1`] = ` Array [ Object { - "file": "common.css", + "file": "chunk.css", "text": "/* packages/rollup/test/specimens/simple.css */ .fooga { color: red; @@ -94,7 +94,7 @@ Array [ ", }, Object { - "file": "common.css", + "file": "shared.css", "text": "/* packages/rollup/test/specimens/manual-chunks/d.css */ /* packages/rollup/test/specimens/manual-chunks/c.css */ .c { @@ -109,7 +109,7 @@ Array [ exports[`/rollup.js code splitting should support splitting up CSS files 1`] = ` Array [ Object { - "file": "common.css", + "file": "chunk.css", "text": "/* packages/rollup/test/specimens/simple.css */ .fooga { color: red; @@ -150,19 +150,15 @@ Array [ }, Object { "file": "chunk.css", - "text": "/* packages/rollup/test/specimens/css-chunks/c.css */ -.c { - - color: cyan; -} -", - }, - Object { - "file": "common.css", "text": "/* packages/rollup/test/specimens/css-chunks/shared.css */ .shared { color: snow; } +/* packages/rollup/test/specimens/css-chunks/c.css */ +.c { + + color: cyan; +} ", }, ] diff --git a/packages/rollup/test/splitting.test.js b/packages/rollup/test/splitting.test.js index 8d257d97d..33fd9b1c3 100644 --- a/packages/rollup/test/splitting.test.js +++ b/packages/rollup/test/splitting.test.js @@ -24,8 +24,8 @@ const sourcemap = false; const json = true; describe("/rollup.js", () => { - beforeAll(() => shell.rm("-rf", prefix("./output/rollup/*"))); - + beforeAll(() => shell.rm("-rf", prefix("./output/*"))); + describe("code splitting", () => { const experimentalCodeSplitting = true; const chunkFileNames = "[name].js"; @@ -33,7 +33,7 @@ describe("/rollup.js", () => { it("should support splitting up CSS files", async () => { const bundle = await rollup({ experimentalCodeSplitting, - + input : [ require.resolve("./specimens/simple.js"), require.resolve("./specimens/dependencies.js"), @@ -46,18 +46,18 @@ describe("/rollup.js", () => { }), ], }); - + await bundle.write({ format, sourcemap, assetFileNames, chunkFileNames, - - dir : prefix(`./output/rollup/splitting`), + + dir : prefix(`./output/splitting`), }); - expect(dir("./rollup/splitting/assets")).toMatchSnapshot(); + expect(dir("./splitting/assets")).toMatchSnapshot(); }); it("should support splitting up CSS files w/ shared assets", async () => { @@ -84,12 +84,12 @@ describe("/rollup.js", () => { assetFileNames, chunkFileNames, - dir : prefix(`./output/rollup/css-chunking`), + dir : prefix(`./output/css-chunks`), }); - expect(dir("./rollup/css-chunking/assets")).toMatchSnapshot(); + expect(dir("./css-chunks/assets")).toMatchSnapshot(); }); - + it("shouldn't put bundle-specific CSS in common.css", async () => { const bundle = await rollup({ experimentalCodeSplitting, @@ -114,10 +114,10 @@ describe("/rollup.js", () => { assetFileNames, chunkFileNames, - dir : prefix(`./output/rollup/common-splitting`), + dir : prefix(`./output/common-splitting`), }); - expect(dir("./rollup/common-splitting/assets")).toMatchSnapshot(); + expect(dir("./common-splitting/assets")).toMatchSnapshot(); }); it("should support manual chunks", async () => { @@ -150,13 +150,13 @@ describe("/rollup.js", () => { assetFileNames, chunkFileNames, - dir : prefix(`./output/rollup/manual-chunks`), + dir : prefix(`./output/manual-chunks`), }); - expect(dir("./rollup/manual-chunks/assets")).toMatchSnapshot(); + expect(dir("./manual-chunks/assets")).toMatchSnapshot(); }); - it("should support dynamic imports", async () => { + it.only("should support dynamic imports", async () => { const bundle = await rollup({ experimentalCodeSplitting, @@ -182,10 +182,10 @@ describe("/rollup.js", () => { assetFileNames, chunkFileNames, - dir : prefix(`./output/rollup/dynamic-imports`), + dir : prefix(`./output/dynamic-imports`), }); - expect(dir("./rollup/dynamic-imports/assets/")).toMatchSnapshot(); + expect(dir("./dynamic-imports/assets/")).toMatchSnapshot(); }); it("should ouput only 1 JSON file", async () => { @@ -213,10 +213,10 @@ describe("/rollup.js", () => { assetFileNames, chunkFileNames, - dir : prefix(`./output/rollup/json-splitting`), + dir : prefix(`./output/json-splitting`), }); - expect(dir("./rollup/json-splitting/assets")).toMatchSnapshot(); + expect(dir("./json-splitting/assets")).toMatchSnapshot(); }); }); }); From ed10067d684831fda167b15d410ee69e1c5e95b8 Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Mon, 26 Nov 2018 21:58:07 -0800 Subject: [PATCH 02/13] WIP: clean up a touch Dynamic imports still failing: https://github.com/rollup/rollup/pull/2553#issuecomment-441837237 --- packages/rollup/rollup.js | 48 ++++++++++---------------- packages/rollup/test/splitting.test.js | 2 +- 2 files changed, 19 insertions(+), 31 deletions(-) diff --git a/packages/rollup/rollup.js b/packages/rollup/rollup.js index 4324d467c..796c3cead 100644 --- a/packages/rollup/rollup.js +++ b/packages/rollup/rollup.js @@ -1,4 +1,4 @@ -/* eslint max-statements: [ 1, 30 ] */ +/* eslint-disable max-statements */ "use strict"; const path = require("path"); @@ -17,16 +17,6 @@ const emptyMappings = { mappings : "", }; -const makeFile = (details) => { - const { entry } = details; - const name = path.basename(entry, path.extname(entry)); - - return Object.assign(details, { - base : path.join(path.dirname(entry), name), - name, - }); -}; - module.exports = (opts) => { const options = Object.assign(Object.create(null), { common : "common.css", @@ -43,7 +33,7 @@ module.exports = (opts) => { const { styleExport, done, map, dev, verbose } = options; // eslint-disable-next-line no-console, no-empty-function - const log = verbose ? console.log.bind(console, "[rollup]") : () => { }; + const log = verbose ? console.log.bind(console, "[rollup]") : () => {}; if(typeof map === "undefined") { // Sourcemaps don't make much sense in styleExport mode @@ -70,7 +60,7 @@ module.exports = (opts) => { }, watchChange(file) { - if(!processor.files[ file ]) { + if(!processor.files[file]) { return; } @@ -128,7 +118,7 @@ module.exports = (opts) => { }); } - if(options.styleExport) { + if(styleExport) { out.push(`export var styles = ${JSON.stringify(details.result.css)};`); } @@ -161,12 +151,11 @@ module.exports = (opts) => { ); } - // First pass is used to calculate JS usage of CSS dependencies + // calculate JS usage of CSS modules since rollup throws away most of that info Object.entries(bundles).forEach(([ entry, bundle ]) => { const name = path.basename(entry, path.extname(entry)); const file = { - entry, name, base : path.join(path.dirname(entry), name), css : new Set(), @@ -205,21 +194,22 @@ module.exports = (opts) => { }); // TODO: Ensure that all CSS files only appear in a single bundle + // Right now dynamic imports are breaking things - console.log("BUNDLES"); - Object.entries(bundles).forEach(([ key, bundle ]) => { - console.log(key); - // console.log(Object.keys(modules)); - console.log(bundle); - }); - console.log("\n\n\n"); + // console.log("BUNDLES"); + // Object.entries(bundles).forEach(([ key, bundle ]) => { + // console.log(key); + // // console.log(Object.keys(modules)); + // console.log(bundle); + // }); + // console.log("\n\n\n"); - console.log("CSS"); - console.log(files); - console.log("\n\n\n"); + // console.log("CSS"); + // console.log(files); + // console.log("\n\n\n"); - console.log("USAGE"); - console.log(usage); + // console.log("USAGE"); + // console.log(usage); for(const [ , { base, name, css }] of files) { if(!css.size) { @@ -249,8 +239,6 @@ module.exports = (opts) => { } } - // result.compositions always includes all the info, so it - // doesn't actually matter which result we use. First one seems reasonable! if(options.json) { const dest = typeof options.json === "string" ? options.json : "exports.json"; diff --git a/packages/rollup/test/splitting.test.js b/packages/rollup/test/splitting.test.js index 33fd9b1c3..94bb202b3 100644 --- a/packages/rollup/test/splitting.test.js +++ b/packages/rollup/test/splitting.test.js @@ -156,7 +156,7 @@ describe("/rollup.js", () => { expect(dir("./manual-chunks/assets")).toMatchSnapshot(); }); - it.only("should support dynamic imports", async () => { + it("should support dynamic imports", async () => { const bundle = await rollup({ experimentalCodeSplitting, From c1ec58b960262362d81365e6f70ed1befdd77e44 Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Mon, 26 Nov 2018 22:04:24 -0800 Subject: [PATCH 03/13] WIP: fix getter context using arrow fn --- packages/processor/processor.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/processor/processor.js b/packages/processor/processor.js index c461a3d13..b58c08fb2 100644 --- a/packages/processor/processor.js +++ b/packages/processor/processor.js @@ -258,9 +258,7 @@ class Processor { ); Object.defineProperty(result, "compositions", { - get() { - return output.compositions(this); - } + get : () => output.compositions(this) }); return result; From fc97c8671b021f19516bf0fdc43daf8fe95b0765 Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Mon, 3 Dec 2018 00:24:33 -0800 Subject: [PATCH 04/13] feat: Handle dynamic imports BREAKING CHANGE: reworked CSS so it better matches rollup output chunk format Also removed extraneous "rollup" directory from test output for easier visibility. --- package-lock.json | 11 +- package.json | 2 +- packages/rollup/rollup.js | 145 +++++++++--------- .../test/__snapshots__/rollup.test.js.snap | 51 ++---- .../test/__snapshots__/splitting.test.js.snap | 26 ++-- packages/rollup/test/rollup.test.js | 83 ++++------ packages/rollup/test/specimens/casing/foo.css | 2 + .../test/specimens/dynamic-imports/b.css | 2 + .../test/specimens/dynamic-imports/d.css | 2 + .../test/specimens/dynamic-imports/e.css | 3 + .../test/specimens/dynamic-imports/f.css | 3 + packages/rollup/test/splitting.test.js | 2 - 12 files changed, 141 insertions(+), 191 deletions(-) create mode 100644 packages/rollup/test/specimens/dynamic-imports/e.css create mode 100644 packages/rollup/test/specimens/dynamic-imports/f.css diff --git a/package-lock.json b/package-lock.json index e197b5ae1..6d14547ca 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1130,9 +1130,9 @@ "dev": true }, "@types/node": { - "version": "10.12.9", - "resolved": "https://registry.npmjs.org/@types/node/-/node-10.12.9.tgz", - "integrity": "sha512-eajkMXG812/w3w4a1OcBlaTwsFPO5F7fJ/amy+tieQxEMWBlbV1JGSjkFM+zkHNf81Cad+dfIRA+IBkvmvdAeA==", + "version": "10.12.11", + "resolved": "https://registry.npmjs.org/@types/node/-/node-10.12.11.tgz", + "integrity": "sha512-3iIOhNiPGTdcUNVCv9e5G7GotfvJJe2pc9w2UgDXlUwnxSZ3RgcUocIU+xYm+rTU54jIKih998QE4dMOyMN1NQ==", "dev": true }, "@webassemblyjs/ast": { @@ -10085,9 +10085,8 @@ } }, "rollup": { - "version": "0.67.3", - "resolved": "https://registry.npmjs.org/rollup/-/rollup-0.67.3.tgz", - "integrity": "sha512-TyNQCz97rKuVVbsKUTXfwIjV7UljWyTVd7cTMuE+aqlQ7WJslkYF5QaYGjMLR2BlQtUOO5CAxSVnpQ55iYp5jg==", + "version": "github:rollup/rollup#eabffd73730c6dd2bfbeb5310b6abc08c0c5bb50", + "from": "github:rollup/rollup#refactor-chunking", "dev": true, "requires": { "@types/estree": "0.0.39", diff --git a/package.json b/package.json index 8db0ca0df..3761bc92a 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,7 @@ "lerna": "^3.4.3", "pegjs": "0.10.0", "read-dir-deep": "1.0.4", - "rollup": "^0.67.3", + "rollup": "github:rollup/rollup#refactor-chunking", "rollup-plugin-svelte": "^4.3.2", "shelljs": "^0.8.3", "svelte": "^2.15.3", diff --git a/packages/rollup/rollup.js b/packages/rollup/rollup.js index 796c3cead..b3fcc1d9f 100644 --- a/packages/rollup/rollup.js +++ b/packages/rollup/rollup.js @@ -7,6 +7,7 @@ const { keyword } = require("esutils"); const utils = require("rollup-pluginutils"); const dedent = require("dedent"); const slash = require("slash"); +const Graph = require("dependency-graph").DepGraph; const Processor = require("@modular-css/processor"); const output = require("@modular-css/processor/lib/output.js"); @@ -83,26 +84,26 @@ module.exports = (opts) => { const { details, exports } = await processor.string(id, code); const exported = output.join(exports); + const relative = path.relative(processor.options.cwd, id); const out = [ ...processor.dependencies(id).map((dep) => `import ${JSON.stringify(dep)};`), - dev ? dedent(` - const data = ${JSON.stringify(exported)}; - - export default new Proxy(data, { - get(tgt, key) { - if(key in tgt) { - return tgt[key]; + dev ? + dedent(` + const data = ${JSON.stringify(exported)}; + + export default new Proxy(data, { + get(tgt, key) { + if(key in tgt) { + return tgt[key]; + } + + throw new ReferenceError( + key + " is not exported by " + ${JSON.stringify(slash(relative))} + ); } - - throw new ReferenceError( - key + " is not exported by " + ${JSON.stringify( - slash(path.relative(process.cwd(), id)) - )} - ); - } - }) - `) : + }) + `) : `export default ${JSON.stringify(exported, null, 4)};`, ]; @@ -129,21 +130,19 @@ module.exports = (opts) => { }; }, - async generateBundle(outputOptions, bundles) { + async generateBundle(outputOptions, chunks) { // styleExport disables all output file generation if(styleExport) { return; } - const usage = new Map(); - const files = new Map(); - const { assetFileNames = "" } = outputOptions; - + + // Determine the correct to option for PostCSS by doing a bit of a dance let to; if(!outputOptions.file && !outputOptions.dir) { - to = path.join(process.cwd(), assetFileNames); + to = path.join(processor.options.cwd, assetFileNames); } else { to = path.join( outputOptions.dir ? outputOptions.dir : path.dirname(outputOptions.file), @@ -151,87 +150,83 @@ module.exports = (opts) => { ); } - // calculate JS usage of CSS modules since rollup throws away most of that info - Object.entries(bundles).forEach(([ entry, bundle ]) => { - const name = path.basename(entry, path.extname(entry)); + // Build chunk dependency graph so it can be walked in order later to + // Allow for outputting CSS alongside chunks as optimally as possible + const usage = new Graph(); - const file = { - name, - base : path.join(path.dirname(entry), name), - css : new Set(), - }; + Object.entries(chunks).forEach(([ entry, chunk ]) => { + const { imports, dynamicImports } = chunk; - const { imports, modules } = bundle; + usage.addNode(entry, true); - // Get CSS files being used by each entry point - const css = Object.keys(modules).filter(filter); + [ ...imports, ...dynamicImports ].forEach((dep) => { + usage.addNode(dep, true); + usage.addDependency(entry, dep); + }); + }); - // Get dependency chains for each file - css.forEach((start) => { - const used = [ - ...processor.dependencies(start), - start, - ]; + // Output CSS chunks + const out = new Map(); - used.forEach((dep) => { - if(!usage.has(dep)) { - usage.set(dep, new Set()); - } + // Keep track of files that are queued to be written + const queued = new Set(); - const users = usage.get(dep).add(entry); + usage.overallOrder().forEach((entry) => { + const { modules } = chunks[entry]; + const css = new Set(); - // CSS included in this chunk only if not already included - // by one of its imports - if(imports.some((f) => users.has(f))) { - return; - } + // Get CSS files being used by this chunk + const styles = Object.keys(modules).filter(filter); + + // Get dependency chains for each css file & record them into the usage graph + styles.forEach((style) => { + css.add(style); - file.css.add(dep); - }); + processor.dependencies(style).forEach((file) => css.add(file)); }); + + // Set up the CSS chunk to be written + out.set( + `${path.basename(entry, path.extname(entry))}.css`, + [ ...css ].filter((file) => !queued.has(file)) + ); - files.set(entry, file); + // Flag all the files that are queued for writing so they don't get double-output + css.forEach((file) => queued.add(file)); }); - // TODO: Ensure that all CSS files only appear in a single bundle - // Right now dynamic imports are breaking things - - // console.log("BUNDLES"); - // Object.entries(bundles).forEach(([ key, bundle ]) => { - // console.log(key); - // // console.log(Object.keys(modules)); - // console.log(bundle); - // }); - // console.log("\n\n\n"); - - // console.log("CSS"); - // console.log(files); - // console.log("\n\n\n"); + // Shove any unreferenced CSS files onto the beginning of the first chunk + // TODO: this is inelegant, but seems reasonable-ish + processor.dependencies().forEach((css) => { + if(queued.has(css)) { + return; + } - // console.log("USAGE"); - // console.log(usage); + out.values().next().value.unshift(css); + queued.add(css); + }); - for(const [ , { base, name, css }] of files) { - if(!css.size) { + for(const [ css, files ] of out.entries()) { + if(!files.length) { continue; } - - const id = this.emitAsset(`${base}.css`); + + const id = this.emitAsset(css); log("css output", id); /* eslint-disable-next-line no-await-in-loop */ const result = await processor.output({ to : to.replace(/\[(name|extname)\]/g, (match, field) => - (field === "name" ? name : ".css") + (field === "name" ? path.basename(css, path.extname(css)) : ".css") ), - files : [ ...css ], + files, }); this.setAssetSource(id, result.css); if(result.map) { - const dest = `${base}.css.map`; + const dest = `${css}.map`; log("map output", dest); diff --git a/packages/rollup/test/__snapshots__/rollup.test.js.snap b/packages/rollup/test/__snapshots__/rollup.test.js.snap index 654ee4347..3ea797359 100644 --- a/packages/rollup/test/__snapshots__/rollup.test.js.snap +++ b/packages/rollup/test/__snapshots__/rollup.test.js.snap @@ -8,17 +8,21 @@ Array [ .mc79ab9c62_bar { display: none; } -/* packages/rollup/test/specimens/casing/foo.css */", +/* packages/rollup/test/specimens/casing/foo.css */ +.mc3dd08d27_foo { + + color: #F00; +}", }, Object { "file": "main.js", - "text": "var foo$1 = \\"mc79ab9c62_bar mc3dd08d27_foo\\"; + "text": "var bar$1 = \\"mc79ab9c62_bar\\"; -var bar$1 = \\"mc79ab9c62_bar\\"; +var foo$1 = \\"mc79ab9c62_bar mc3dd08d27_foo\\"; var bar$3 = \\"mc79ab9c62_bar\\"; -console.log({ foo: foo$1, bar: bar$1, bar2: bar$3 }); +console.log({ foo: foo$1, bar: bar$3, bar2: bar$1 }); ", }, ] @@ -56,7 +60,7 @@ exports[`/rollup.js should correctly pass to/from params for relative paths 1`] "/* packages/rollup/test/specimens/relative-paths.css */ .wooga { color: red; - background: url(\\"../../../../specimens/folder/to.png\\"); + background: url(\\"../../../specimens/folder/to.png\\"); } " `; @@ -103,7 +107,7 @@ Object { "mappings": "AAAA,+CAAC;AAED;IACI,WAAW;CACd", "names": Array [], "sources": Array [ - "../../../../specimens/simple.css", + "../../../specimens/simple.css", ], "sourcesContent": Array [ "@value str: \\"string\\"; @@ -342,38 +346,3 @@ console.log(css); "version": 3, } `; - -exports[`/rollup.js shouldn't over-remove files from an existing processor instance 1`] = ` -"var css = { - \\"a\\": \\"c a\\" -}; - -var css$1 = { - \\"b\\": \\"c b\\" -}; - -console.log(css, css$1); -" -`; - -exports[`/rollup.js shouldn't over-remove files from an existing processor instance 2`] = ` -"/* packages/rollup/test/specimens/repeated-references/d.css */ -.d { - color: darkblue; -} -/* packages/rollup/test/specimens/repeated-references/c.css */ -.c { - color: cadetblue; -} -/* packages/rollup/test/specimens/repeated-references/b.css */ -.b { - - color: blue; -} -/* packages/rollup/test/specimens/repeated-references/a.css */ -.a { - - color: red; -} -" -`; diff --git a/packages/rollup/test/__snapshots__/splitting.test.js.snap b/packages/rollup/test/__snapshots__/splitting.test.js.snap index 770c43145..23deb8296 100644 --- a/packages/rollup/test/__snapshots__/splitting.test.js.snap +++ b/packages/rollup/test/__snapshots__/splitting.test.js.snap @@ -43,31 +43,37 @@ Array [ color: aqua; } +/* packages/rollup/test/specimens/dynamic-imports/f.css */ +.f { + color: floralwhite; +} +/* packages/rollup/test/specimens/dynamic-imports/d.css */ +.d { + + color: darkred; +} ", }, Object { "file": "b.css", - "text": "/* packages/rollup/test/specimens/dynamic-imports/b.css */ + "text": "/* packages/rollup/test/specimens/dynamic-imports/e.css */ +.e { + color: #EEE; +} +/* packages/rollup/test/specimens/dynamic-imports/b.css */ .b { + color: blue; } ", }, Object { - "file": "c.css", + "file": "chunk.css", "text": "/* packages/rollup/test/specimens/dynamic-imports/c.css */ .c { color: cyan; } -", - }, - Object { - "file": "common.css", - "text": "/* packages/rollup/test/specimens/dynamic-imports/d.css */ -.d { - color: darkred; -} ", }, ] diff --git a/packages/rollup/test/rollup.test.js b/packages/rollup/test/rollup.test.js index 68725d433..0a0db38c8 100644 --- a/packages/rollup/test/rollup.test.js +++ b/packages/rollup/test/rollup.test.js @@ -29,7 +29,7 @@ const map = false; const sourcemap = false; describe("/rollup.js", () => { - beforeAll(() => shell.rm("-rf", prefix("./output/rollup/*"))); + beforeAll(() => shell.rm("-rf", prefix("./output/*"))); it("should be a function", () => expect(typeof plugin).toBe("function") @@ -79,10 +79,10 @@ describe("/rollup.js", () => { await bundle.write({ format, assetFileNames, - file : prefix(`./output/rollup/css/simple.js`), + file : prefix(`./output/css/simple.js`), }); - expect(read("./rollup/css/assets/simple.css")).toMatchSnapshot(); + expect(read("./css/assets/simple.css")).toMatchSnapshot(); }); it("should handle assetFileNames being undefined", async () => { @@ -120,10 +120,10 @@ describe("/rollup.js", () => { await bundle.write({ format, assetFileNames, - file : prefix(`./output/rollup/relative-paths/relative-paths.js`), + file : prefix(`./output/relative-paths/relative-paths.js`), }); - expect(read("./rollup/relative-paths/assets/relative-paths.css")).toMatchSnapshot(); + expect(read("./relative-paths/assets/relative-paths.css")).toMatchSnapshot(); }); it("should avoid generating empty CSS", async () => { @@ -139,10 +139,10 @@ describe("/rollup.js", () => { await bundle.write({ format, assetFileNames, - file : prefix(`./output/rollup/no-css/no-css.js`), + file : prefix(`./output/no-css/no-css.js`), }); - expect(exists("./output/rollup/no-css/assets/no-css.css")).toBe(false); + expect(exists("./output/no-css/assets/no-css.css")).toBe(false); }); it("should generate JSON", async () => { @@ -159,10 +159,10 @@ describe("/rollup.js", () => { await bundle.write({ format, assetFileNames, - file : prefix(`./output/rollup/json/simple.js`), + file : prefix(`./output/json/simple.js`), }); - expect(read("./rollup/json/assets/exports.json")).toMatchSnapshot(); + expect(read("./json/assets/exports.json")).toMatchSnapshot(); }); it("should generate JSON with a custom name", async () => { @@ -179,10 +179,10 @@ describe("/rollup.js", () => { await bundle.write({ format, assetFileNames, - file : prefix(`./output/rollup/json-named/simple.js`), + file : prefix(`./output/json-named/simple.js`), }); - expect(read("./rollup/json-named/assets/custom.json")).toMatchSnapshot(); + expect(read("./json-named/assets/custom.json")).toMatchSnapshot(); }); it("should provide named exports", async () => { @@ -251,16 +251,16 @@ describe("/rollup.js", () => { await bundle.write({ format, assetFileNames, - file : prefix(`./output/rollup/external-source-maps/simple.js`), + file : prefix(`./output/external-source-maps/simple.js`), }); // Have to parse it into JSON so the propertyMatcher can exclude the file property // since it is a hash value and changes constantly - expect(JSON.parse(read("./rollup/external-source-maps/assets/simple.css.map"))).toMatchSnapshot({ + expect(JSON.parse(read("./external-source-maps/assets/simple.css.map"))).toMatchSnapshot({ file : expect.any(String), }); - expect(read("./rollup/external-source-maps/assets/simple.css")).toMatchSnapshot(); + expect(read("./external-source-maps/assets/simple.css")).toMatchSnapshot(); }); it("should warn & not export individual keys when they are not valid identifiers", async () => { @@ -346,10 +346,10 @@ describe("/rollup.js", () => { format, sourcemap, - file : prefix(`./output/rollup/no-maps/no-maps.js`), + file : prefix(`./output/no-maps/no-maps.js`), }); - expect(read("./rollup/no-maps/assets/no-maps.css")).toMatchSnapshot(); + expect(read("./no-maps/assets/no-maps.css")).toMatchSnapshot(); }); it("should respect the CSS dependency tree", async () => { @@ -368,11 +368,11 @@ describe("/rollup.js", () => { assetFileNames, sourcemap, - file : prefix(`./output/rollup/dependencies/dependencies.js`), + file : prefix(`./output/dependencies/dependencies.js`), }); - expect(read("./rollup/dependencies/dependencies.js")).toMatchSnapshot(); - expect(read("./rollup/dependencies/assets/dependencies.css")).toMatchSnapshot(); + expect(read("./dependencies/dependencies.js")).toMatchSnapshot(); + expect(read("./dependencies/assets/dependencies.css")).toMatchSnapshot(); }); it("should accept an existing processor instance", async () => { @@ -401,39 +401,10 @@ describe("/rollup.js", () => { sourcemap, assetFileNames, - file : prefix(`./output/rollup/existing-processor/existing-processor.js`), + file : prefix(`./output/existing-processor/existing-processor.js`), }); - expect(read("./rollup/existing-processor/assets/existing-processor.css")).toMatchSnapshot(); - }); - - it("shouldn't over-remove files from an existing processor instance", async () => { - const processor = new Processor({ - namer, - map, - }); - - await processor.file(require.resolve("./specimens/repeated-references/b.css")); - - const bundle = await rollup({ - input : require.resolve("./specimens/repeated-references/a.js"), - plugins : [ - plugin({ - processor, - }), - ], - }); - - await bundle.write({ - format, - sourcemap, - assetFileNames, - - file : prefix(`./output/rollup/repeated-references/repeated-references.js`), - }); - - expect(read("./rollup/repeated-references/repeated-references.js")).toMatchSnapshot(); - expect(read("./rollup/repeated-references/assets/repeated-references.css")).toMatchSnapshot(); + expect(read("./existing-processor/assets/existing-processor.css")).toMatchSnapshot(); }); it("should output a proxy in dev mode", async () => { @@ -513,10 +484,10 @@ describe("/rollup.js", () => { format, assetFileNames, sourcemap, - file : prefix(`./output/rollup/casing/main.js`), + file : prefix(`./output/casing/main.js`), }); - expect(readdir("./rollup/casing")).toMatchSnapshot(); + expect(readdir("./casing")).toMatchSnapshot(); }); }); @@ -531,7 +502,7 @@ describe("/rollup.js", () => { plugins : [ plugin({ namer, - css : prefix(`./output/rollup/errors.css`), + css : prefix(`./output/errors.css`), before : [ error ], }), ], @@ -545,7 +516,7 @@ describe("/rollup.js", () => { plugins : [ plugin({ namer, - css : prefix(`./output/rollup/errors.css`), + css : prefix(`./output/errors.css`), after : [ error ], }), ], @@ -560,14 +531,14 @@ describe("/rollup.js", () => { plugins : [ plugin({ namer, - css : prefix(`./output/rollup/errors.css`), + css : prefix(`./output/errors.css`), done : [ error ], }), ], }) .then((bundle) => bundle.write({ format, - file : prefix(`./output/rollup/done-error.js`), + file : prefix(`./output/done-error.js`), })) ); }); diff --git a/packages/rollup/test/specimens/casing/foo.css b/packages/rollup/test/specimens/casing/foo.css index bae18d6c3..a5d7d9ab4 100644 --- a/packages/rollup/test/specimens/casing/foo.css +++ b/packages/rollup/test/specimens/casing/foo.css @@ -1,4 +1,6 @@ .foo { composes: bar from "./bar.css"; composes: bar from "../Casing/bar.css"; + + color: #F00; } diff --git a/packages/rollup/test/specimens/dynamic-imports/b.css b/packages/rollup/test/specimens/dynamic-imports/b.css index 908ccb0c9..a96500907 100644 --- a/packages/rollup/test/specimens/dynamic-imports/b.css +++ b/packages/rollup/test/specimens/dynamic-imports/b.css @@ -1,3 +1,5 @@ .b { + composes: e from "./e.css"; + color: blue; } diff --git a/packages/rollup/test/specimens/dynamic-imports/d.css b/packages/rollup/test/specimens/dynamic-imports/d.css index bbb030a2e..b5b84c77c 100644 --- a/packages/rollup/test/specimens/dynamic-imports/d.css +++ b/packages/rollup/test/specimens/dynamic-imports/d.css @@ -1,3 +1,5 @@ .d { + composes: f from "./f.css"; + color: darkred; } diff --git a/packages/rollup/test/specimens/dynamic-imports/e.css b/packages/rollup/test/specimens/dynamic-imports/e.css new file mode 100644 index 000000000..a25afea91 --- /dev/null +++ b/packages/rollup/test/specimens/dynamic-imports/e.css @@ -0,0 +1,3 @@ +.e { + color: #EEE; +} diff --git a/packages/rollup/test/specimens/dynamic-imports/f.css b/packages/rollup/test/specimens/dynamic-imports/f.css new file mode 100644 index 000000000..f816dded1 --- /dev/null +++ b/packages/rollup/test/specimens/dynamic-imports/f.css @@ -0,0 +1,3 @@ +.f { + color: floralwhite; +} diff --git a/packages/rollup/test/splitting.test.js b/packages/rollup/test/splitting.test.js index 94bb202b3..4875fdd9d 100644 --- a/packages/rollup/test/splitting.test.js +++ b/packages/rollup/test/splitting.test.js @@ -160,8 +160,6 @@ describe("/rollup.js", () => { const bundle = await rollup({ experimentalCodeSplitting, - // treeshake : false, - input : [ require.resolve("./specimens/dynamic-imports/a.js"), require.resolve("./specimens/dynamic-imports/b.js"), From 5b68e5608d25c36bb0759028eb76950c000a31c7 Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Sun, 16 Dec 2018 23:12:44 -0800 Subject: [PATCH 05/13] chore: rollup@0.68 :package: --- package-lock.json | 11 ++++++----- package.json | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index 6d14547ca..652264118 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1130,9 +1130,9 @@ "dev": true }, "@types/node": { - "version": "10.12.11", - "resolved": "https://registry.npmjs.org/@types/node/-/node-10.12.11.tgz", - "integrity": "sha512-3iIOhNiPGTdcUNVCv9e5G7GotfvJJe2pc9w2UgDXlUwnxSZ3RgcUocIU+xYm+rTU54jIKih998QE4dMOyMN1NQ==", + "version": "10.12.15", + "resolved": "https://registry.npmjs.org/@types/node/-/node-10.12.15.tgz", + "integrity": "sha512-9kROxduaN98QghwwHmxXO2Xz3MaWf+I1sLVAA6KJDF5xix+IyXVhds0MAfdNwtcpSrzhaTsNB0/jnL86fgUhqA==", "dev": true }, "@webassemblyjs/ast": { @@ -10085,8 +10085,9 @@ } }, "rollup": { - "version": "github:rollup/rollup#eabffd73730c6dd2bfbeb5310b6abc08c0c5bb50", - "from": "github:rollup/rollup#refactor-chunking", + "version": "0.68.0", + "resolved": "https://registry.npmjs.org/rollup/-/rollup-0.68.0.tgz", + "integrity": "sha512-UbmntCf8QBlOqJnwsNWQCI0oonHOgs9y1OLoO8BHf2r8gCyRLp3JzLHXARJpsNDAS08Qm3LDjzyWma5sqnCxDQ==", "dev": true, "requires": { "@types/estree": "0.0.39", diff --git a/package.json b/package.json index 3761bc92a..a97e3d52c 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,7 @@ "lerna": "^3.4.3", "pegjs": "0.10.0", "read-dir-deep": "1.0.4", - "rollup": "github:rollup/rollup#refactor-chunking", + "rollup": "^0.68.0", "rollup-plugin-svelte": "^4.3.2", "shelljs": "^0.8.3", "svelte": "^2.15.3", From ca28b3909223ac5cb32586e91f704d5be62d77b1 Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Sun, 16 Dec 2018 23:16:58 -0800 Subject: [PATCH 06/13] fix: Use entry name if code-splitting To avoid any ugly double-hashed filenames. Fixes #525 --- packages/rollup/rollup.js | 8 +++-- .../test/__snapshots__/rollup.test.js.snap | 23 +++++++++++++++ .../test/__snapshots__/splitting.test.js.snap | 23 +++++++++++++++ packages/rollup/test/rollup.test.js | 19 ++++++++++++ packages/rollup/test/splitting.test.js | 29 +++++++++++++++++++ 5 files changed, 100 insertions(+), 2 deletions(-) diff --git a/packages/rollup/rollup.js b/packages/rollup/rollup.js index b3fcc1d9f..e732710ef 100644 --- a/packages/rollup/rollup.js +++ b/packages/rollup/rollup.js @@ -172,7 +172,7 @@ module.exports = (opts) => { const queued = new Set(); usage.overallOrder().forEach((entry) => { - const { modules } = chunks[entry]; + const { modules, name } = chunks[entry]; const css = new Set(); // Get CSS files being used by this chunk @@ -184,10 +184,14 @@ module.exports = (opts) => { processor.dependencies(style).forEach((file) => css.add(file)); }); + + // Want to use source chunk name when code-splitting, otherwise match + // bundle name + const identifier = outputOptions.dir ? name : entry; // Set up the CSS chunk to be written out.set( - `${path.basename(entry, path.extname(entry))}.css`, + `${path.basename(identifier, path.extname(identifier))}.css`, [ ...css ].filter((file) => !queued.has(file)) ); diff --git a/packages/rollup/test/__snapshots__/rollup.test.js.snap b/packages/rollup/test/__snapshots__/rollup.test.js.snap index 3ea797359..fdc5a133f 100644 --- a/packages/rollup/test/__snapshots__/rollup.test.js.snap +++ b/packages/rollup/test/__snapshots__/rollup.test.js.snap @@ -56,6 +56,29 @@ console.log(fooga); " `; +exports[`/rollup.js should correctly handle hashed output 1`] = ` +Array [ + Object { + "file": "assets\\\\hashes-8d1bcf7b.css", + "text": "/* packages/rollup/test/specimens/simple.css */ +.fooga { + color: red; +} +", + }, + Object { + "file": "hashes.js", + "text": "var css = { + \\"str\\": \\"\\\\\\"string\\\\\\"\\", + \\"fooga\\": \\"fooga\\" +}; + +console.log(css); +", + }, +] +`; + exports[`/rollup.js should correctly pass to/from params for relative paths 1`] = ` "/* packages/rollup/test/specimens/relative-paths.css */ .wooga { diff --git a/packages/rollup/test/__snapshots__/splitting.test.js.snap b/packages/rollup/test/__snapshots__/splitting.test.js.snap index 23deb8296..0c08a7e09 100644 --- a/packages/rollup/test/__snapshots__/splitting.test.js.snap +++ b/packages/rollup/test/__snapshots__/splitting.test.js.snap @@ -200,3 +200,26 @@ Array [ }, ] `; + +exports[`/rollup.js code splitting shouldn't use entry hashes as part of the CSS file names 1`] = ` +Array [ + Object { + "file": "assets\\\\simple-8d1bcf7b.css", + "text": "/* packages/rollup/test/specimens/simple.css */ +.fooga { + color: red; +} +", + }, + Object { + "file": "simple.15bac60f.js", + "text": "var css = { + \\"str\\": \\"\\\\\\"string\\\\\\"\\", + \\"fooga\\": \\"fooga\\" +}; + +console.log(css); +", + }, +] +`; diff --git a/packages/rollup/test/rollup.test.js b/packages/rollup/test/rollup.test.js index 0a0db38c8..648c0303d 100644 --- a/packages/rollup/test/rollup.test.js +++ b/packages/rollup/test/rollup.test.js @@ -125,6 +125,25 @@ describe("/rollup.js", () => { expect(read("./relative-paths/assets/relative-paths.css")).toMatchSnapshot(); }); + + it("should correctly handle hashed output", async () => { + const bundle = await rollup({ + input : require.resolve("./specimens/simple.js"), + plugins : [ + plugin({ + namer, + map, + }), + ], + }); + + await bundle.write({ + format, + file : prefix(`./output/hashes/hashes.js`), + }); + + expect(readdir("./hashes")).toMatchSnapshot(); + }); it("should avoid generating empty CSS", async () => { const bundle = await rollup({ diff --git a/packages/rollup/test/splitting.test.js b/packages/rollup/test/splitting.test.js index 4875fdd9d..64a2566ca 100644 --- a/packages/rollup/test/splitting.test.js +++ b/packages/rollup/test/splitting.test.js @@ -216,5 +216,34 @@ describe("/rollup.js", () => { expect(dir("./json-splitting/assets")).toMatchSnapshot(); }); + + it("shouldn't use entry hashes as part of the CSS file names", async () => { + const bundle = await rollup({ + experimentalCodeSplitting, + + input : [ + require.resolve("./specimens/simple.js") + ], + + plugins : [ + plugin({ + namer, + map, + }), + ], + + }); + + await bundle.write({ + format, + sourcemap, + + entryFileNames : "[name].[hash].js", + + dir : prefix(`./output/no-hash-names/`), + }); + + expect(dir("./no-hash-names")).toMatchSnapshot(); + }); }); }); From c0099ddd9f3439168238fa1cddf86b1b2cf506d0 Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Sun, 16 Dec 2018 23:29:10 -0800 Subject: [PATCH 07/13] test: fix read-dir helper to output / --- packages/rollup/test/__snapshots__/rollup.test.js.snap | 4 ++-- packages/rollup/test/__snapshots__/splitting.test.js.snap | 2 +- packages/test-utils/read-dir.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/rollup/test/__snapshots__/rollup.test.js.snap b/packages/rollup/test/__snapshots__/rollup.test.js.snap index fdc5a133f..9e4e9f1ff 100644 --- a/packages/rollup/test/__snapshots__/rollup.test.js.snap +++ b/packages/rollup/test/__snapshots__/rollup.test.js.snap @@ -3,7 +3,7 @@ exports[`/rollup.js case sensitivity tests should remove repeated references that point at the same files 1`] = ` Array [ Object { - "file": "assets\\\\main.css", + "file": "assets/main.css", "text": "/* packages/rollup/test/specimens/casing/bar.css */ .mc79ab9c62_bar { display: none; @@ -59,7 +59,7 @@ console.log(fooga); exports[`/rollup.js should correctly handle hashed output 1`] = ` Array [ Object { - "file": "assets\\\\hashes-8d1bcf7b.css", + "file": "assets/hashes-8d1bcf7b.css", "text": "/* packages/rollup/test/specimens/simple.css */ .fooga { color: red; diff --git a/packages/rollup/test/__snapshots__/splitting.test.js.snap b/packages/rollup/test/__snapshots__/splitting.test.js.snap index 0c08a7e09..b5dea5ff9 100644 --- a/packages/rollup/test/__snapshots__/splitting.test.js.snap +++ b/packages/rollup/test/__snapshots__/splitting.test.js.snap @@ -204,7 +204,7 @@ Array [ exports[`/rollup.js code splitting shouldn't use entry hashes as part of the CSS file names 1`] = ` Array [ Object { - "file": "assets\\\\simple-8d1bcf7b.css", + "file": "assets/simple-8d1bcf7b.css", "text": "/* packages/rollup/test/specimens/simple.css */ .fooga { color: red; diff --git a/packages/test-utils/read-dir.js b/packages/test-utils/read-dir.js index 127965181..534d49559 100644 --- a/packages/test-utils/read-dir.js +++ b/packages/test-utils/read-dir.js @@ -11,7 +11,7 @@ module.exports = (cwd) => const files = read.sync(dir); return files.sort().map((file) => ({ - file, + file : file.replace(/\\/g, "/"), text : fs.readFileSync(path.join(dir, file), "utf8"), })); }; From 83ed96627cfa7a6eda85d5bf51169369141f54aa Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Tue, 18 Dec 2018 10:04:24 -0800 Subject: [PATCH 08/13] WIP: handle multiple chunks better --- package-lock.json | 18 ++-- packages/rollup/rollup.js | 32 ++++--- .../test/__snapshots__/splitting.test.js.snap | 84 +++++++++++++++++++ .../test/specimens/multiple-chunks/a.css | 5 ++ .../test/specimens/multiple-chunks/a.js | 8 ++ .../test/specimens/multiple-chunks/b.css | 5 ++ .../test/specimens/multiple-chunks/b.js | 4 + .../test/specimens/multiple-chunks/c.css | 5 ++ .../test/specimens/multiple-chunks/c.js | 4 + .../test/specimens/multiple-chunks/common.js | 1 + .../specimens/multiple-chunks/constants.css | 1 + .../test/specimens/multiple-chunks/shared.css | 5 ++ packages/rollup/test/splitting.test.js | 32 ++++++- 13 files changed, 180 insertions(+), 24 deletions(-) create mode 100644 packages/rollup/test/specimens/multiple-chunks/a.css create mode 100644 packages/rollup/test/specimens/multiple-chunks/a.js create mode 100644 packages/rollup/test/specimens/multiple-chunks/b.css create mode 100644 packages/rollup/test/specimens/multiple-chunks/b.js create mode 100644 packages/rollup/test/specimens/multiple-chunks/c.css create mode 100644 packages/rollup/test/specimens/multiple-chunks/c.js create mode 100644 packages/rollup/test/specimens/multiple-chunks/common.js create mode 100644 packages/rollup/test/specimens/multiple-chunks/constants.css create mode 100644 packages/rollup/test/specimens/multiple-chunks/shared.css diff --git a/package-lock.json b/package-lock.json index 652264118..a47248285 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1442,7 +1442,7 @@ }, "ansi-escapes": { "version": "3.1.0", - "resolved": "http://registry.npmjs.org/ansi-escapes/-/ansi-escapes-3.1.0.tgz", + "resolved": "https://registry.npmjs.org/ansi-escapes/-/ansi-escapes-3.1.0.tgz", "integrity": "sha512-UgAb8H9D41AQnu/PbWlCofQVcnV4Gs2bBJi9eZPxfU/hgglFh3SMDMENRIqdr7H6XFnXdoknctFByVsCOotTVw==", "dev": true }, @@ -1923,7 +1923,7 @@ }, "util": { "version": "0.10.3", - "resolved": "http://registry.npmjs.org/util/-/util-0.10.3.tgz", + "resolved": "https://registry.npmjs.org/util/-/util-0.10.3.tgz", "integrity": "sha1-evsa/lCAUkZInj23/g7TeTNqwPk=", "dev": true, "requires": { @@ -4394,7 +4394,7 @@ }, "through2": { "version": "1.1.1", - "resolved": "http://registry.npmjs.org/through2/-/through2-1.1.1.tgz", + "resolved": "https://registry.npmjs.org/through2/-/through2-1.1.1.tgz", "integrity": "sha1-CEfLxESfNAVXTb3M2buEG4OsNUU=", "dev": true, "requires": { @@ -4491,7 +4491,7 @@ "dependencies": { "through2": { "version": "1.1.1", - "resolved": "http://registry.npmjs.org/through2/-/through2-1.1.1.tgz", + "resolved": "https://registry.npmjs.org/through2/-/through2-1.1.1.tgz", "integrity": "sha1-CEfLxESfNAVXTb3M2buEG4OsNUU=", "dev": true, "requires": { @@ -4509,7 +4509,7 @@ }, "through2": { "version": "0.5.1", - "resolved": "http://registry.npmjs.org/through2/-/through2-0.5.1.tgz", + "resolved": "https://registry.npmjs.org/through2/-/through2-0.5.1.tgz", "integrity": "sha1-390BLrnHAOIyP9M084rGIqs3Lac=", "dev": true, "requires": { @@ -6349,7 +6349,7 @@ }, "htmlescape": { "version": "1.1.1", - "resolved": "http://registry.npmjs.org/htmlescape/-/htmlescape-1.1.1.tgz", + "resolved": "https://registry.npmjs.org/htmlescape/-/htmlescape-1.1.1.tgz", "integrity": "sha1-OgPtwiFLyjtmQko+eVk0lQnLA1E=", "dev": true }, @@ -10154,7 +10154,7 @@ }, "safe-regex": { "version": "1.1.0", - "resolved": "http://registry.npmjs.org/safe-regex/-/safe-regex-1.1.0.tgz", + "resolved": "https://registry.npmjs.org/safe-regex/-/safe-regex-1.1.0.tgz", "integrity": "sha1-QKNmnzsHfR6UPURinhV91IAjvy4=", "dev": true, "requires": { @@ -11108,7 +11108,7 @@ }, "strip-eof": { "version": "1.0.0", - "resolved": "http://registry.npmjs.org/strip-eof/-/strip-eof-1.0.0.tgz", + "resolved": "https://registry.npmjs.org/strip-eof/-/strip-eof-1.0.0.tgz", "integrity": "sha1-u0P/VZim6wXYm1n80SnJgzE2Br8=", "dev": true }, @@ -11209,7 +11209,7 @@ }, "tar": { "version": "2.2.1", - "resolved": "http://registry.npmjs.org/tar/-/tar-2.2.1.tgz", + "resolved": "https://registry.npmjs.org/tar/-/tar-2.2.1.tgz", "integrity": "sha1-jk0qJWwOIYXGsYrWlK7JaLg8sdE=", "dev": true, "requires": { diff --git a/packages/rollup/rollup.js b/packages/rollup/rollup.js index e732710ef..a2a9c6855 100644 --- a/packages/rollup/rollup.js +++ b/packages/rollup/rollup.js @@ -137,7 +137,7 @@ module.exports = (opts) => { } const { assetFileNames = "" } = outputOptions; - + // Determine the correct to option for PostCSS by doing a bit of a dance let to; @@ -171,13 +171,15 @@ module.exports = (opts) => { // Keep track of files that are queued to be written const queued = new Set(); + usage.overallOrder().forEach((entry) => { - const { modules, name } = chunks[entry]; + const { modules, name, fileName } = chunks[entry]; const css = new Set(); + let counter = 1; // Get CSS files being used by this chunk const styles = Object.keys(modules).filter(filter); - + // Get dependency chains for each css file & record them into the usage graph styles.forEach((style) => { css.add(style); @@ -185,13 +187,17 @@ module.exports = (opts) => { processor.dependencies(style).forEach((file) => css.add(file)); }); - // Want to use source chunk name when code-splitting, otherwise match - // bundle name - const identifier = outputOptions.dir ? name : entry; - + // Want to use source chunk name when code-splitting, otherwise match bundle name + let identifier = outputOptions.dir ? name : path.basename(entry, path.extname(entry)); + + // Replicate rollup chunk name de-duping logic here since that isn't exposed for us + while(out.has(identifier)) { + identifier = `${identifier}${++counter}`; + } + // Set up the CSS chunk to be written out.set( - `${path.basename(identifier, path.extname(identifier))}.css`, + identifier, [ ...css ].filter((file) => !queued.has(file)) ); @@ -210,19 +216,19 @@ module.exports = (opts) => { queued.add(css); }); - for(const [ css, files ] of out.entries()) { + for(const [ name, files ] of out.entries()) { if(!files.length) { continue; } - - const id = this.emitAsset(css); + + const id = this.emitAsset(`${name}.css`); log("css output", id); /* eslint-disable-next-line no-await-in-loop */ const result = await processor.output({ to : to.replace(/\[(name|extname)\]/g, (match, field) => - (field === "name" ? path.basename(css, path.extname(css)) : ".css") + (field === "name" ? name : ".css") ), files, }); @@ -230,7 +236,7 @@ module.exports = (opts) => { this.setAssetSource(id, result.css); if(result.map) { - const dest = `${css}.map`; + const dest = `${name}.css.map`; log("map output", dest); diff --git a/packages/rollup/test/__snapshots__/splitting.test.js.snap b/packages/rollup/test/__snapshots__/splitting.test.js.snap index b5dea5ff9..7f003ddfd 100644 --- a/packages/rollup/test/__snapshots__/splitting.test.js.snap +++ b/packages/rollup/test/__snapshots__/splitting.test.js.snap @@ -1,5 +1,89 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`/rollup.js code splitting should dedupe chunk names using rollup's incrementing counter logic 1`] = ` +Array [ + Object { + "file": "a.js", + "text": "import { a as common } from './chunk-c1a8c5ee.js'; + +var css = { + \\"a\\": \\"shared a\\" +}; + +console.log(css, common); + +const c = import('./chunk-928a7e8e.js'); + +c.then(console.log); +", + }, + Object { + "file": "assets/a-92d8d302.css", + "text": "/* packages/rollup/test/specimens/multiple-chunks/a.css */ +.a { + + color: aqua; +} +", + }, + Object { + "file": "assets/b-912c273a.css", + "text": "/* packages/rollup/test/specimens/multiple-chunks/b.css */ +.b { + + color: blue; +} +", + }, + Object { + "file": "assets/chunk-6aa4dec1.css", + "text": "/* packages/rollup/test/specimens/multiple-chunks/constants.css */ /* packages/rollup/test/specimens/multiple-chunks/shared.css */ +.shared { color: salmon; } + +.other { color: blue; } +", + }, + Object { + "file": "assets/chunk2-5c2ce32d.css", + "text": "/* packages/rollup/test/specimens/multiple-chunks/c.css */ +.c { + + color: coral; +} +", + }, + Object { + "file": "b.js", + "text": "import { a as common } from './chunk-c1a8c5ee.js'; + +var css = { + \\"b\\": \\"shared b\\" +}; + +console.log(css, common); +", + }, + Object { + "file": "chunk-928a7e8e.js", + "text": "import { a as common } from './chunk-c1a8c5ee.js'; + +var css = { + \\"c\\": \\"shared c\\" +}; + +console.log(css, common); +", + }, + Object { + "file": "chunk-c1a8c5ee.js", + "text": "var common = \\"common\\"; + +export { common as a }; +", + }, +] +`; + exports[`/rollup.js code splitting should ouput only 1 JSON file 1`] = ` Array [ Object { diff --git a/packages/rollup/test/specimens/multiple-chunks/a.css b/packages/rollup/test/specimens/multiple-chunks/a.css new file mode 100644 index 000000000..b2a281c34 --- /dev/null +++ b/packages/rollup/test/specimens/multiple-chunks/a.css @@ -0,0 +1,5 @@ +.a { + composes: shared from "./shared.css"; + + color: aqua; +} diff --git a/packages/rollup/test/specimens/multiple-chunks/a.js b/packages/rollup/test/specimens/multiple-chunks/a.js new file mode 100644 index 000000000..48e8b3aa7 --- /dev/null +++ b/packages/rollup/test/specimens/multiple-chunks/a.js @@ -0,0 +1,8 @@ +import css from "./a.css"; +import common from "./common.js"; + +console.log(css, common); + +const c = import("./c.js"); + +c.then(console.log); diff --git a/packages/rollup/test/specimens/multiple-chunks/b.css b/packages/rollup/test/specimens/multiple-chunks/b.css new file mode 100644 index 000000000..16e87cdb5 --- /dev/null +++ b/packages/rollup/test/specimens/multiple-chunks/b.css @@ -0,0 +1,5 @@ +.b { + composes: shared from "./shared.css"; + + color: blue; +} diff --git a/packages/rollup/test/specimens/multiple-chunks/b.js b/packages/rollup/test/specimens/multiple-chunks/b.js new file mode 100644 index 000000000..18e9ce0ee --- /dev/null +++ b/packages/rollup/test/specimens/multiple-chunks/b.js @@ -0,0 +1,4 @@ +import css from "./b.css"; +import common from "./common.js"; + +console.log(css, common); diff --git a/packages/rollup/test/specimens/multiple-chunks/c.css b/packages/rollup/test/specimens/multiple-chunks/c.css new file mode 100644 index 000000000..eec134a9f --- /dev/null +++ b/packages/rollup/test/specimens/multiple-chunks/c.css @@ -0,0 +1,5 @@ +.c { + composes: shared from "./shared.css"; + + color: coral; +} diff --git a/packages/rollup/test/specimens/multiple-chunks/c.js b/packages/rollup/test/specimens/multiple-chunks/c.js new file mode 100644 index 000000000..bd48d857b --- /dev/null +++ b/packages/rollup/test/specimens/multiple-chunks/c.js @@ -0,0 +1,4 @@ +import css from "./c.css"; +import common from "./common.js"; + +console.log(css, common); diff --git a/packages/rollup/test/specimens/multiple-chunks/common.js b/packages/rollup/test/specimens/multiple-chunks/common.js new file mode 100644 index 000000000..c735fc8c8 --- /dev/null +++ b/packages/rollup/test/specimens/multiple-chunks/common.js @@ -0,0 +1 @@ +export default "common"; diff --git a/packages/rollup/test/specimens/multiple-chunks/constants.css b/packages/rollup/test/specimens/multiple-chunks/constants.css new file mode 100644 index 000000000..96ba5d092 --- /dev/null +++ b/packages/rollup/test/specimens/multiple-chunks/constants.css @@ -0,0 +1 @@ +@value main: blue; diff --git a/packages/rollup/test/specimens/multiple-chunks/shared.css b/packages/rollup/test/specimens/multiple-chunks/shared.css new file mode 100644 index 000000000..296f11c8f --- /dev/null +++ b/packages/rollup/test/specimens/multiple-chunks/shared.css @@ -0,0 +1,5 @@ +@value main from "./constants.css"; + +.shared { color: salmon; } + +.other { color: main; } diff --git a/packages/rollup/test/splitting.test.js b/packages/rollup/test/splitting.test.js index 64a2566ca..eb4080c10 100644 --- a/packages/rollup/test/splitting.test.js +++ b/packages/rollup/test/splitting.test.js @@ -224,7 +224,7 @@ describe("/rollup.js", () => { input : [ require.resolve("./specimens/simple.js") ], - + plugins : [ plugin({ namer, @@ -239,11 +239,39 @@ describe("/rollup.js", () => { sourcemap, entryFileNames : "[name].[hash].js", - + dir : prefix(`./output/no-hash-names/`), }); expect(dir("./no-hash-names")).toMatchSnapshot(); }); + + it("should dedupe chunk names using rollup's incrementing counter logic", async () => { + const bundle = await rollup({ + experimentalCodeSplitting, + + input : [ + require.resolve("./specimens/multiple-chunks/a.js"), + require.resolve("./specimens/multiple-chunks/b.js"), + ], + + plugins : [ + plugin({ + namer, + map, + }), + ], + + }); + + await bundle.write({ + format, + sourcemap, + + dir : prefix(`./output/multiple-chunks/`), + }); + + expect(dir("./multiple-chunks")).toMatchSnapshot(); + }); }); }); From 8817c6bbb202addd02ac483f7ce15aaf83713c5b Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Wed, 19 Dec 2018 08:09:45 -0800 Subject: [PATCH 09/13] test: remove hash from results It's not stable on other machines, which makes snapshots fail all over the place. --- .../test/__snapshots__/splitting.test.js.snap | 52 ++----------------- packages/rollup/test/splitting.test.js | 10 ++-- 2 files changed, 10 insertions(+), 52 deletions(-) diff --git a/packages/rollup/test/__snapshots__/splitting.test.js.snap b/packages/rollup/test/__snapshots__/splitting.test.js.snap index 7f003ddfd..462c27dca 100644 --- a/packages/rollup/test/__snapshots__/splitting.test.js.snap +++ b/packages/rollup/test/__snapshots__/splitting.test.js.snap @@ -3,22 +3,7 @@ exports[`/rollup.js code splitting should dedupe chunk names using rollup's incrementing counter logic 1`] = ` Array [ Object { - "file": "a.js", - "text": "import { a as common } from './chunk-c1a8c5ee.js'; - -var css = { - \\"a\\": \\"shared a\\" -}; - -console.log(css, common); - -const c = import('./chunk-928a7e8e.js'); - -c.then(console.log); -", - }, - Object { - "file": "assets/a-92d8d302.css", + "file": "a.css", "text": "/* packages/rollup/test/specimens/multiple-chunks/a.css */ .a { @@ -27,7 +12,7 @@ c.then(console.log); ", }, Object { - "file": "assets/b-912c273a.css", + "file": "b.css", "text": "/* packages/rollup/test/specimens/multiple-chunks/b.css */ .b { @@ -36,7 +21,7 @@ c.then(console.log); ", }, Object { - "file": "assets/chunk-6aa4dec1.css", + "file": "chunk.css", "text": "/* packages/rollup/test/specimens/multiple-chunks/constants.css */ /* packages/rollup/test/specimens/multiple-chunks/shared.css */ .shared { color: salmon; } @@ -44,41 +29,12 @@ c.then(console.log); ", }, Object { - "file": "assets/chunk2-5c2ce32d.css", + "file": "chunk2.css", "text": "/* packages/rollup/test/specimens/multiple-chunks/c.css */ .c { color: coral; } -", - }, - Object { - "file": "b.js", - "text": "import { a as common } from './chunk-c1a8c5ee.js'; - -var css = { - \\"b\\": \\"shared b\\" -}; - -console.log(css, common); -", - }, - Object { - "file": "chunk-928a7e8e.js", - "text": "import { a as common } from './chunk-c1a8c5ee.js'; - -var css = { - \\"c\\": \\"shared c\\" -}; - -console.log(css, common); -", - }, - Object { - "file": "chunk-c1a8c5ee.js", - "text": "var common = \\"common\\"; - -export { common as a }; ", }, ] diff --git a/packages/rollup/test/splitting.test.js b/packages/rollup/test/splitting.test.js index eb4080c10..000e06a2a 100644 --- a/packages/rollup/test/splitting.test.js +++ b/packages/rollup/test/splitting.test.js @@ -18,18 +18,17 @@ function error(root) { error.postcssPlugin = "error-plugin"; const assetFileNames = "assets/[name][extname]"; +const chunkFileNames = "[name].js"; const format = "es"; const map = false; const sourcemap = false; const json = true; +const experimentalCodeSplitting = true; describe("/rollup.js", () => { beforeAll(() => shell.rm("-rf", prefix("./output/*"))); describe("code splitting", () => { - const experimentalCodeSplitting = true; - const chunkFileNames = "[name].js"; - it("should support splitting up CSS files", async () => { const bundle = await rollup({ experimentalCodeSplitting, @@ -268,10 +267,13 @@ describe("/rollup.js", () => { format, sourcemap, + assetFileNames, + chunkFileNames, + dir : prefix(`./output/multiple-chunks/`), }); - expect(dir("./multiple-chunks")).toMatchSnapshot(); + expect(dir("./multiple-chunks/assets")).toMatchSnapshot(); }); }); }); From ee513be57ed0cd370a7a1c89673ed03b6a6b03b1 Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Wed, 19 Dec 2018 08:12:05 -0800 Subject: [PATCH 10/13] docs: bump minimum rollup version --- packages/rollup/README.md | 3 ++- packages/rollup/package.json | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/rollup/README.md b/packages/rollup/README.md index f711c2665..af2d7e1b2 100644 --- a/packages/rollup/README.md +++ b/packages/rollup/README.md @@ -21,8 +21,9 @@ Rollup support for [`modular-css`](https://github.com/tivac/modular-css). Due to API changes, certain major versions of this plugin will require a specific minimum rollup version. -- `@modular-css/rollup@11` requires `rollup@0.60.0` +- `@modular-css/rollup@18` requires `rollup@0.68.0` - `@modular-css/rollup@15` requires `rollup@0.65.0` +- `@modular-css/rollup@11` requires `rollup@0.60.0` ## Usage diff --git a/packages/rollup/package.json b/packages/rollup/package.json index 9791f4f5d..ca1a15bc1 100644 --- a/packages/rollup/package.json +++ b/packages/rollup/package.json @@ -29,6 +29,6 @@ "slash": "^2.0.0" }, "peerDependencies": { - "rollup": ">0.65.0" + "rollup": ">0.68.0" } } From f49e35522549f21f361bacbdeb901db5f56742ed Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Wed, 19 Dec 2018 15:27:28 -0800 Subject: [PATCH 11/13] fix: properly wait for prior resolutions There was a dependency ordering bug hiding in the old code, yikes! --- packages/processor/processor.js | 18 ++--- .../test/__snapshots__/options.test.js.snap | 48 +++++++++++-- .../test/__snapshots__/rollup.test.js.snap | 60 ++++++++++++++-- .../test/__snapshots__/svelte.test.js.snap | 72 +++++++++++++++++-- 4 files changed, 175 insertions(+), 23 deletions(-) diff --git a/packages/processor/processor.js b/packages/processor/processor.js index b58c08fb2..9c5fa675a 100644 --- a/packages/processor/processor.js +++ b/packages/processor/processor.js @@ -197,6 +197,8 @@ class Processor { const results = []; for(const dep of files) { + this._log("_after()", dep); + // eslint-disable-next-line no-await-in-loop const result = await this._after.process( // NOTE: the call to .clone() is really important here, otherwise this call @@ -296,7 +298,7 @@ class Processor { const file = this._files[dep]; if(!file.processed) { - this._log("_add() processing", dep); + this._log("_process()", dep); file.processed = this._process.process( file.result, @@ -350,6 +352,8 @@ class Processor { this._graph.addNode(name); + this._log("_before()", name); + const file = this._files[name] = { text, exports : false, @@ -378,13 +382,11 @@ class Processor { // Walk this node's dependencies, reading new files from disk as necessary await Promise.all( - this._graph.dependenciesOf(name).reduce((promises, dependency) => { - if(!this._files[dependency]) { - promises.push(this.file(dependency)); - } - - return promises; - }, []) + this._graph.dependenciesOf(name).map((dependency) => ( + this._files[dependency] ? + this._files[dependency].result : + this.file(dependency) + )) ); } } diff --git a/packages/processor/test/__snapshots__/options.test.js.snap b/packages/processor/test/__snapshots__/options.test.js.snap index 67f725f37..6daab3bd3 100644 --- a/packages/processor/test/__snapshots__/options.test.js.snap +++ b/packages/processor/test/__snapshots__/options.test.js.snap @@ -86,6 +86,11 @@ Array [ "_add()", "packages/processor/test/specimens/start.css", ], + Array [ + "[processor]", + "_before()", + "packages/processor/test/specimens/start.css", + ], Array [ "[processor]", "file()", @@ -96,6 +101,11 @@ Array [ "_add()", "packages/processor/test/specimens/local.css", ], + Array [ + "[processor]", + "_before()", + "packages/processor/test/specimens/local.css", + ], Array [ "[processor]", "file()", @@ -108,17 +118,22 @@ Array [ ], Array [ "[processor]", - "_add() processing", + "_before()", "packages/processor/test/specimens/folder/folder.css", ], Array [ "[processor]", - "_add() processing", + "_process()", + "packages/processor/test/specimens/folder/folder.css", + ], + Array [ + "[processor]", + "_process()", "packages/processor/test/specimens/local.css", ], Array [ "[processor]", - "_add() processing", + "_process()", "packages/processor/test/specimens/start.css", ], Array [ @@ -133,9 +148,34 @@ Array [ ], Array [ "[processor]", - "_add() processing", + "_before()", + "packages/processor/test/specimens/string.css", + ], + Array [ + "[processor]", + "_process()", "packages/processor/test/specimens/string.css", ], + Array [ + "[processor]", + "_after()", + "packages/processor/test/specimens/folder/folder.css", + ], + Array [ + "[processor]", + "_after()", + "packages/processor/test/specimens/string.css", + ], + Array [ + "[processor]", + "_after()", + "packages/processor/test/specimens/local.css", + ], + Array [ + "[processor]", + "_after()", + "packages/processor/test/specimens/start.css", + ], ] `; diff --git a/packages/rollup/test/__snapshots__/rollup.test.js.snap b/packages/rollup/test/__snapshots__/rollup.test.js.snap index 9e4e9f1ff..47ac244a7 100644 --- a/packages/rollup/test/__snapshots__/rollup.test.js.snap +++ b/packages/rollup/test/__snapshots__/rollup.test.js.snap @@ -184,7 +184,12 @@ Array [ ], Array [ "[processor]", - "_add() processing", + "_before()", + "packages/rollup/test/specimens/simple.css", + ], + Array [ + "[processor]", + "_process()", "packages/rollup/test/specimens/simple.css", ], Array [ @@ -192,6 +197,11 @@ Array [ "css output", "40a365e7", ], + Array [ + "[processor]", + "_after()", + "packages/rollup/test/specimens/simple.css", + ], Array [ "[processor]", "file()", @@ -202,6 +212,11 @@ Array [ "_add()", "packages/processor/test/specimens/start.css", ], + Array [ + "[processor]", + "_before()", + "packages/processor/test/specimens/start.css", + ], Array [ "[processor]", "file()", @@ -212,6 +227,11 @@ Array [ "_add()", "packages/processor/test/specimens/local.css", ], + Array [ + "[processor]", + "_before()", + "packages/processor/test/specimens/local.css", + ], Array [ "[processor]", "file()", @@ -224,17 +244,22 @@ Array [ ], Array [ "[processor]", - "_add() processing", + "_before()", + "packages/processor/test/specimens/folder/folder.css", + ], + Array [ + "[processor]", + "_process()", "packages/processor/test/specimens/folder/folder.css", ], Array [ "[processor]", - "_add() processing", + "_process()", "packages/processor/test/specimens/local.css", ], Array [ "[processor]", - "_add() processing", + "_process()", "packages/processor/test/specimens/start.css", ], Array [ @@ -249,9 +274,34 @@ Array [ ], Array [ "[processor]", - "_add() processing", + "_before()", "packages/processor/test/specimens/string.css", ], + Array [ + "[processor]", + "_process()", + "packages/processor/test/specimens/string.css", + ], + Array [ + "[processor]", + "_after()", + "packages/processor/test/specimens/folder/folder.css", + ], + Array [ + "[processor]", + "_after()", + "packages/processor/test/specimens/string.css", + ], + Array [ + "[processor]", + "_after()", + "packages/processor/test/specimens/local.css", + ], + Array [ + "[processor]", + "_after()", + "packages/processor/test/specimens/start.css", + ], ] `; diff --git a/packages/svelte/test/__snapshots__/svelte.test.js.snap b/packages/svelte/test/__snapshots__/svelte.test.js.snap index f0674d86e..bbf7e9b3f 100644 --- a/packages/svelte/test/__snapshots__/svelte.test.js.snap +++ b/packages/svelte/test/__snapshots__/svelte.test.js.snap @@ -439,6 +439,11 @@ Array [ "_add()", "packages/svelte/test/specimens/external.css", ], + Array [ + "[processor]", + "_before()", + "packages/svelte/test/specimens/external.css", + ], Array [ "[processor]", "file()", @@ -449,6 +454,11 @@ Array [ "_add()", "packages/svelte/test/specimens/simple.css", ], + Array [ + "[processor]", + "_before()", + "packages/svelte/test/specimens/simple.css", + ], Array [ "[processor]", "file()", @@ -461,17 +471,22 @@ Array [ ], Array [ "[processor]", - "_add() processing", + "_before()", + "packages/svelte/test/specimens/dependencies.css", + ], + Array [ + "[processor]", + "_process()", "packages/svelte/test/specimens/simple.css", ], Array [ "[processor]", - "_add() processing", + "_process()", "packages/svelte/test/specimens/dependencies.css", ], Array [ "[processor]", - "_add() processing", + "_process()", "packages/svelte/test/specimens/external.css", ], Array [ @@ -483,6 +498,21 @@ Array [ "[svelte]", "[\\"color\\",\\"flex\\",\\"wrapper\\",\\"hd\\",\\"bd\\",\\"text\\",\\"active\\"]", ], + Array [ + "[processor]", + "_after()", + "packages/svelte/test/specimens/simple.css", + ], + Array [ + "[processor]", + "_after()", + "packages/svelte/test/specimens/dependencies.css", + ], + Array [ + "[processor]", + "_after()", + "packages/svelte/test/specimens/external.css", + ], ] `; @@ -507,6 +537,11 @@ Array [ "_add()", "packages/svelte/test/specimens/style.html", ], + Array [ + "[processor]", + "_before()", + "packages/svelte/test/specimens/style.html", + ], Array [ "[processor]", "file()", @@ -517,6 +552,11 @@ Array [ "_add()", "packages/svelte/test/specimens/simple.css", ], + Array [ + "[processor]", + "_before()", + "packages/svelte/test/specimens/simple.css", + ], Array [ "[processor]", "file()", @@ -529,17 +569,22 @@ Array [ ], Array [ "[processor]", - "_add() processing", + "_before()", + "packages/svelte/test/specimens/dependencies.css", + ], + Array [ + "[processor]", + "_process()", "packages/svelte/test/specimens/simple.css", ], Array [ "[processor]", - "_add() processing", + "_process()", "packages/svelte/test/specimens/dependencies.css", ], Array [ "[processor]", - "_add() processing", + "_process()", "packages/svelte/test/specimens/style.html", ], Array [ @@ -551,6 +596,21 @@ Array [ "[svelte]", "[\\"flex\\",\\"wrapper\\",\\"hd\\",\\"bd\\",\\"text\\",\\"active\\"]", ], + Array [ + "[processor]", + "_after()", + "packages/svelte/test/specimens/simple.css", + ], + Array [ + "[processor]", + "_after()", + "packages/svelte/test/specimens/dependencies.css", + ], + Array [ + "[processor]", + "_after()", + "packages/svelte/test/specimens/style.html", + ], ] `; From 00172c9a7b381599dd00398432231deba10c10bf Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Wed, 19 Dec 2018 15:59:32 -0800 Subject: [PATCH 12/13] refactor: remove lodash.foreach Object.entries does what we need w/o any flopping around in a 3rd party lib. --- packages/processor/plugins/values-replace.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/processor/plugins/values-replace.js b/packages/processor/plugins/values-replace.js index 510f0e292..f8e8789b7 100644 --- a/packages/processor/plugins/values-replace.js +++ b/packages/processor/plugins/values-replace.js @@ -3,10 +3,9 @@ const selector = require("postcss-selector-parser"); const value = require("postcss-value-parser"); const escape = require("escape-string-regexp"); -const each = require("lodash/forEach"); const get = require("lodash/get"); const Graph = require("dependency-graph").DepGraph; - + const namespaced = require("./values-namespaced.js"); module.exports = (css, { opts, messages }) => { @@ -22,8 +21,10 @@ module.exports = (css, { opts, messages }) => { messages .filter(({ plugin }) => plugin === namespaced.postcssPlugin) .forEach((msg) => - each(msg.values, (children, ns) => - each(children, (details, child) => (values[`${ns}.${child}`] = details)) + Object.entries(msg.values).forEach(([ ns, children ]) => + Object.entries(children).forEach(([ child, details ]) => + (values[`${ns}.${child}`] = details) + ) ) ); @@ -54,17 +55,17 @@ module.exports = (css, { opts, messages }) => { const replacer = (prop) => (thing) => { const parsed = value(thing[prop]); - + parsed.walk((node) => { if(node.type !== "word") { return; } - + // Replace any value instances node.value = node.value.replace(matchRegex, (match) => { // Source map support thing.source = values[match].source; - + return values[match].value; }); }); @@ -73,7 +74,7 @@ module.exports = (css, { opts, messages }) => { }; // Walk through all values & build dependency graph - each(values, (details, name) => { + Object.entries(values).forEach(([ name, details ]) => { graph.addNode(name); value(details.value).walk((node) => { @@ -89,7 +90,7 @@ module.exports = (css, { opts, messages }) => { // Walk through values in dependency order & update any inter-dependent values graph.overallOrder().forEach((name) => { const parsed = value(values[name].value); - + parsed.walk((node) => { if(node.type !== "word" || !values[node.value]) { return; From aa11e236899ef09ee4bde231df1137734298d49e Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Fri, 21 Dec 2018 23:46:31 -0800 Subject: [PATCH 13/13] chore: fix rollup peer dep semver --- packages/rollup/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rollup/package.json b/packages/rollup/package.json index ca1a15bc1..2056f5f8e 100644 --- a/packages/rollup/package.json +++ b/packages/rollup/package.json @@ -29,6 +29,6 @@ "slash": "^2.0.0" }, "peerDependencies": { - "rollup": ">0.68.0" + "rollup": ">=0.68" } }