From 1fab7774474e2163f972f24afb8ad3636811d03a Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Tue, 1 Jan 2019 14:32:28 -0800 Subject: [PATCH] feat: Rollup@1.0 support (#532) BREAKING CHANGE: requires rollup@1.0 --- package-lock.json | 23 ++++---- package.json | 4 +- packages/processor/processor.js | 26 +++++++- .../test/__snapshots__/api.test.js.snap | 56 ++++++++++++++++++ packages/processor/test/api.test.js | 59 +++++++++++++++++++ packages/rollup/package.json | 2 +- packages/rollup/rollup.js | 21 +++---- .../test/__snapshots__/rollup.test.js.snap | 52 ++++++++++------ packages/rollup/test/rollup.test.js | 31 ++++------ packages/rollup/test/splitting.test.js | 17 ------ packages/test-utils/rollup-code-snapshot.js | 22 +++++++ 11 files changed, 231 insertions(+), 82 deletions(-) create mode 100644 packages/test-utils/rollup-code-snapshot.js diff --git a/package-lock.json b/package-lock.json index d42d9505b..d405a5bea 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1164,9 +1164,9 @@ "dev": true }, "@tivac/eslint-config": { - "version": "2.2.2", - "resolved": "https://registry.npmjs.org/@tivac/eslint-config/-/eslint-config-2.2.2.tgz", - "integrity": "sha512-0nXhLCnGEWVulujQojYP9Ss8ME+p6pp0nr2IB8Ek0rDTDpp1XY+Ovy3hZ1lgz3n99UmPu7SQK/adSSckIbGqKQ==", + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/@tivac/eslint-config/-/eslint-config-2.3.0.tgz", + "integrity": "sha512-vsGB0bAxVJf6R5bvRx5PJh2ugdmC1GUXCLzGk2YaSoUFeE+zfM+JiGLHOsLoXVAEQSvQuTYbrf+ojSe6+aTwSg==", "dev": true }, "@types/estree": { @@ -4702,7 +4702,7 @@ }, "is-accessor-descriptor": { "version": "0.1.6", - "resolved": "http://registry.npmjs.org/is-accessor-descriptor/-/is-accessor-descriptor-0.1.6.tgz", + "resolved": "https://registry.npmjs.org/is-accessor-descriptor/-/is-accessor-descriptor-0.1.6.tgz", "integrity": "sha1-qeEss66Nh2cn7u84Q/igiXtcmNY=", "dev": true, "requires": { @@ -4722,7 +4722,7 @@ }, "is-data-descriptor": { "version": "0.1.4", - "resolved": "http://registry.npmjs.org/is-data-descriptor/-/is-data-descriptor-0.1.4.tgz", + "resolved": "https://registry.npmjs.org/is-data-descriptor/-/is-data-descriptor-0.1.4.tgz", "integrity": "sha1-C17mSDiOLIYCgueT8YVv7D8wG1Y=", "dev": true, "requires": { @@ -5810,7 +5810,7 @@ }, "string-width": { "version": "1.0.2", - "resolved": "http://registry.npmjs.org/string-width/-/string-width-1.0.2.tgz", + "resolved": "https://registry.npmjs.org/string-width/-/string-width-1.0.2.tgz", "integrity": "sha1-EYvfW4zcUaKn5w0hHgfisLmxB9M=", "dev": true, "requires": { @@ -8576,7 +8576,7 @@ }, "multimatch": { "version": "2.1.0", - "resolved": "http://registry.npmjs.org/multimatch/-/multimatch-2.1.0.tgz", + "resolved": "https://registry.npmjs.org/multimatch/-/multimatch-2.1.0.tgz", "integrity": "sha1-nHkGoi+0wCkZ4vX3UWG0zb1LKis=", "dev": true, "requires": { @@ -10496,13 +10496,14 @@ } }, "rollup": { - "version": "0.68.1", - "resolved": "https://registry.npmjs.org/rollup/-/rollup-0.68.1.tgz", - "integrity": "sha512-8DNKos2p/B7gDoxI42kyIHHX8d+Zt+bwhjUgXnTqnSP+CSPkRNNIQyHIcTqeGdYWR70qG6c1DaRcrjWAiG6Akg==", + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/rollup/-/rollup-1.0.0.tgz", + "integrity": "sha512-LV6Qz+RkuDAfxr9YopU4k5o5P/QA7YNq9xi2Ug2IqOmhPt9sAm89vh3SkNtFok3bqZHX54eMJZ8F68HPejgqtw==", "dev": true, "requires": { "@types/estree": "0.0.39", - "@types/node": "*" + "@types/node": "*", + "acorn": "^6.0.4" } }, "rollup-plugin-svelte": { diff --git a/package.json b/package.json index b3c092f52..52e26009a 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "devDependencies": { "@modular-css/test-utils": "file:packages/test-utils", "@modular-css/website": "file:packages/www", - "@tivac/eslint-config": "^2.2.2", + "@tivac/eslint-config": "^2.3.0", "browserify": "^16.2.3", "cli-tester": "2.0.0", "dedent": "0.7.0", @@ -36,7 +36,7 @@ "lerna": "^3.8.0", "pegjs": "0.10.0", "read-dir-deep": "1.0.4", - "rollup": "^0.68.1", + "rollup": "^1.0.0", "rollup-plugin-svelte": "^5.0.0", "shelljs": "^0.8.3", "svelte": "^2.16.0", diff --git a/packages/processor/processor.js b/packages/processor/processor.js index 9c5fa675a..3c6f5ca69 100644 --- a/packages/processor/processor.js +++ b/packages/processor/processor.js @@ -143,6 +143,27 @@ class Processor { return files; } + // Mark a file and everything that depends on it as invalid so + // it can be overwritten + invalidate(input) { + if(!input) { + throw new Error("invalidate() requires a file argument"); + } + + // Only want files actually in the array + const source = this._normalize(input); + + if(!this._graph.hasNode(source)) { + throw new Error(`Unknown file: ${input}`); + } + + const deps = this.dependents(source); + + deps.concat(source).forEach((file) => { + this._files[file].valid = false; + }); + } + // Get the dependency order for a file or the entire tree dependencies(file) { if(file) { @@ -345,8 +366,8 @@ class Processor { // Process files and walk their composition/value dependency tree to find // new files we need to process async _walk(name, text) { - // No need to re-process files - if(this._files[name]) { + // No need to re-process files unless they've been marked invalid + if(this._files[name] && this._files[name].valid) { return; } @@ -358,6 +379,7 @@ class Processor { text, exports : false, values : false, + valid : true, result : this._before.process( text, params(this, { diff --git a/packages/processor/test/__snapshots__/api.test.js.snap b/packages/processor/test/__snapshots__/api.test.js.snap index d5058a64b..7bc862b74 100644 --- a/packages/processor/test/__snapshots__/api.test.js.snap +++ b/packages/processor/test/__snapshots__/api.test.js.snap @@ -87,6 +87,62 @@ exports[`/processor.js API .file() should process an absolute file 4`] = ` " `; +exports[`/processor.js API .invalidate() should invalidate a relative file 1`] = ` +Array [ + Array [ + "simple.css", + false, + ], +] +`; + +exports[`/processor.js API .invalidate() should invalidate all dependents as well 1`] = ` +Array [ + Array [ + "packages/processor/test/specimens/start.css", + false, + ], + Array [ + "packages/processor/test/specimens/local.css", + false, + ], + Array [ + "packages/processor/test/specimens/folder/folder.css", + false, + ], +] +`; + +exports[`/processor.js API .invalidate() should invalidate an absolute file 1`] = ` +Array [ + Array [ + "packages/processor/test/specimens/simple.css", + false, + ], +] +`; + +exports[`/processor.js API .invalidate() should reprocess invalidated files 1`] = ` +Array [ + Array [ + "packages/processor/test/specimens/start.css", + true, + ], + Array [ + "packages/processor/test/specimens/local.css", + true, + ], + Array [ + "packages/processor/test/specimens/folder/folder.css", + true, + ], +] +`; + +exports[`/processor.js API .invalidate() should throw if an invalid file is passed 1`] = `"Unknown file: nope.css"`; + +exports[`/processor.js API .invalidate() should throw if no file is passed 1`] = `"invalidate() requires a file argument"`; + exports[`/processor.js API .output() should allow for seperate source map output 1`] = ` Object { "file": "to.css", diff --git a/packages/processor/test/api.test.js b/packages/processor/test/api.test.js index 03b27d6c5..647d24453 100644 --- a/packages/processor/test/api.test.js +++ b/packages/processor/test/api.test.js @@ -106,6 +106,65 @@ describe("/processor.js", () => { ).toMatchSnapshot(); }); }); + + describe(".invalidate()", () => { + const status = (source) => + Object.entries(source).map(([ key, value ]) => + ([ relative([ key ])[0], value.valid ]) + ); + + it("should invalidate a relative file", async () => { + await processor.string( + "./simple.css", + ".wooga { }" + ); + + processor.invalidate("./simple.css"); + + expect(status(processor.files)).toMatchSnapshot(); + }); + + it("should invalidate an absolute file", async () => { + await processor.string( + "./packages/processor/test/specimens/simple.css", + ".wooga { }" + ); + + processor.invalidate(require.resolve("./specimens/simple.css")); + + expect(status(processor.files)).toMatchSnapshot(); + }); + + it("should throw if no file is passed", async () => { + await processor.file("./packages/processor/test/specimens/start.css"); + + expect(() => processor.invalidate()).toThrowErrorMatchingSnapshot(); + }); + + it("should throw if an invalid file is passed", async () => { + await processor.file("./packages/processor/test/specimens/start.css"); + + expect(() => processor.invalidate("nope.css")).toThrowErrorMatchingSnapshot(); + }); + + it("should invalidate all dependents as well", async () => { + await processor.file("./packages/processor/test/specimens/start.css"); + + processor.invalidate("./packages/processor/test/specimens/folder/folder.css"); + + expect(status(processor.files)).toMatchSnapshot(); + }); + + it("should reprocess invalidated files", async () => { + await processor.file("./packages/processor/test/specimens/start.css"); + + processor.invalidate("./packages/processor/test/specimens/start.css"); + + await processor.file("./packages/processor/test/specimens/start.css"); + + expect(status(processor.files)).toMatchSnapshot(); + }); + }); describe(".dependencies()", () => { it("should return the dependencies of the specified file", async () => { diff --git a/packages/rollup/package.json b/packages/rollup/package.json index ee6292c36..aba0ee0c1 100644 --- a/packages/rollup/package.json +++ b/packages/rollup/package.json @@ -29,6 +29,6 @@ "slash": "^2.0.0" }, "peerDependencies": { - "rollup": ">=0.68" + "rollup": "^1" } } diff --git a/packages/rollup/rollup.js b/packages/rollup/rollup.js index a2a9c6855..1adba4e8d 100644 --- a/packages/rollup/rollup.js +++ b/packages/rollup/rollup.js @@ -67,11 +67,8 @@ module.exports = (opts) => { log("file changed", file); - processor.dependents(file).forEach((dep) => - processor.remove(dep) - ); - - processor.remove(file); + // TODO: should the file be removed if it's gone? + processor.invalidate(file); }, async transform(code, id) { @@ -123,10 +120,11 @@ module.exports = (opts) => { out.push(`export var styles = ${JSON.stringify(details.result.css)};`); } + processor.dependencies(id).forEach((dependency) => this.addWatchFile(dependency)); + return { - code : out.join("\n"), - map : emptyMappings, - dependencies : processor.dependencies(id), + code : out.join("\n"), + map : emptyMappings, }; }, @@ -171,9 +169,8 @@ module.exports = (opts) => { // Keep track of files that are queued to be written const queued = new Set(); - usage.overallOrder().forEach((entry) => { - const { modules, name, fileName } = chunks[entry]; + const { modules, name } = chunks[entry]; const css = new Set(); let counter = 1; @@ -223,8 +220,6 @@ module.exports = (opts) => { 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) => @@ -233,6 +228,8 @@ module.exports = (opts) => { files, }); + log("css output", `${name}.css`); + this.setAssetSource(id, result.css); if(result.map) { diff --git a/packages/rollup/test/__snapshots__/rollup.test.js.snap b/packages/rollup/test/__snapshots__/rollup.test.js.snap index 47ac244a7..add442bed 100644 --- a/packages/rollup/test/__snapshots__/rollup.test.js.snap +++ b/packages/rollup/test/__snapshots__/rollup.test.js.snap @@ -40,20 +40,24 @@ exports[`/rollup.js should accept an existing processor instance 1`] = ` `; exports[`/rollup.js should allow disabling of named exports 1`] = ` -"var css = { +Object { + "simple": "var css = { \\"str\\": \\"\\\\\\"string\\\\\\"\\", \\"fooga\\": \\"fooga\\" }; console.log(css); -" +", +} `; exports[`/rollup.js should be able to tree-shake results 1`] = ` -"var fooga = \\"fooga\\"; +Object { + "tree-shaking": "var fooga = \\"fooga\\"; console.log(fooga); -" +", +} `; exports[`/rollup.js should correctly handle hashed output 1`] = ` @@ -115,13 +119,15 @@ exports[`/rollup.js should generate JSON with a custom name 1`] = ` `; exports[`/rollup.js should generate exports 1`] = ` -"var css = { +Object { + "simple": "var css = { \\"str\\": \\"\\\\\\"string\\\\\\"\\", \\"fooga\\": \\"fooga\\" }; console.log(css); -" +", +} `; exports[`/rollup.js should generate external source maps 1`] = ` @@ -192,16 +198,16 @@ Array [ "_process()", "packages/rollup/test/specimens/simple.css", ], - Array [ - "[rollup]", - "css output", - "40a365e7", - ], Array [ "[processor]", "_after()", "packages/rollup/test/specimens/simple.css", ], + Array [ + "[rollup]", + "css output", + "simple.css", + ], Array [ "[processor]", "file()", @@ -314,7 +320,8 @@ exports[`/rollup.js should not output sourcemaps when they are disabled 1`] = ` `; exports[`/rollup.js should output a proxy in dev mode 1`] = ` -"const data = {\\"str\\":\\"\\\\\\"string\\\\\\"\\",\\"fooga\\":\\"fooga\\"}; +Object { + "simple": "const data = {\\"str\\":\\"\\\\\\"string\\\\\\"\\",\\"fooga\\":\\"fooga\\"}; var css = new Proxy(data, { get(tgt, key) { @@ -329,25 +336,30 @@ var css = new Proxy(data, { }); console.log(css); -" +", +} `; exports[`/rollup.js should provide named exports 1`] = ` -"var str = \\"\\\\\\"string\\\\\\"\\"; +Object { + "named": "var str = \\"\\\\\\"string\\\\\\"\\"; var num = \\"10\\"; var dim = \\"10px\\"; var mix = \\"1px solid red\\"; var a = \\"a\\"; console.log(a, str, num, dim, mix); -" +", +} `; exports[`/rollup.js should provide style export 1`] = ` -"var styles = \\".ooh {\\\\n content: \\\\\\"string\\\\\\";\\\\n}\\\\n\\"; +Object { + "style-export": "var styles = \\".ooh {\\\\n content: \\\\\\"string\\\\\\";\\\\n}\\\\n\\"; console.log(styles); -" +", +} `; exports[`/rollup.js should respect the CSS dependency tree 1`] = ` @@ -384,14 +396,16 @@ Object { `; exports[`/rollup.js should warn & not export individual keys when they are not valid identifiers 2`] = ` -"var css = { +Object { + "invalid-name": "var css = { \\"fooga\\": \\"fooga\\", \\"fooga-wooga\\": \\"fooga-wooga\\" }; var fooga = \\"fooga\\"; console.log(css, fooga); -" +", +} `; exports[`/rollup.js should warn that styleExport and done aren't compatible 1`] = ` diff --git a/packages/rollup/test/rollup.test.js b/packages/rollup/test/rollup.test.js index 648c0303d..c1448d4f8 100644 --- a/packages/rollup/test/rollup.test.js +++ b/packages/rollup/test/rollup.test.js @@ -13,6 +13,8 @@ const prefix = require("@modular-css/test-utils/prefix.js")(__dirname); const namer = require("@modular-css/test-utils/namer.js"); const logs = require("@modular-css/test-utils/logs.js"); +require("@modular-css/test-utils/rollup-code-snapshot.js"); + const Processor = require("@modular-css/processor"); const plugin = require("../rollup.js"); @@ -46,8 +48,8 @@ describe("/rollup.js", () => { }); const result = await bundle.generate({ format }); - - expect(result.code).toMatchSnapshot(); + + expect(result).toMatchRollupCodeSnapshot(); }); it("should be able to tree-shake results", async () => { @@ -62,7 +64,7 @@ describe("/rollup.js", () => { const result = await bundle.generate({ format }); - expect(result.code).toMatchSnapshot(); + expect(result).toMatchRollupCodeSnapshot(); }); it("should generate CSS", async () => { @@ -216,7 +218,7 @@ describe("/rollup.js", () => { const result = await bundle.generate({ format }); - expect(result.code).toMatchSnapshot(); + expect(result).toMatchRollupCodeSnapshot(); }); it("should provide style export", async () => { @@ -232,7 +234,7 @@ describe("/rollup.js", () => { const result = await bundle.generate({ format }); - expect(result.code).toMatchSnapshot(); + expect(result).toMatchRollupCodeSnapshot(); }); it("should warn that styleExport and done aren't compatible", async () => { @@ -298,7 +300,7 @@ describe("/rollup.js", () => { assetFileNames, }); - expect(result.code).toMatchSnapshot(); + expect(result).toMatchRollupCodeSnapshot(); }); it("should allow disabling of named exports", async () => { @@ -317,7 +319,7 @@ describe("/rollup.js", () => { assetFileNames, }); - expect(result.code).toMatchSnapshot(); + expect(result).toMatchRollupCodeSnapshot(); }); it("shouldn't disable sourcemap generation", async () => { @@ -331,14 +333,15 @@ describe("/rollup.js", () => { ], }); - const result = await bundle.generate({ + const { output } = await bundle.generate({ format, assetFileNames, sourcemap : true, }); - expect(result.map).toMatchSnapshot(); + // Find first chunk w/ a .map property, then compare it to snapshot + expect(output.find((chunk) => chunk.map).map).toMatchSnapshot(); }); it("should not output sourcemaps when they are disabled", async () => { @@ -352,14 +355,6 @@ describe("/rollup.js", () => { ], }); - const source = await bundle.generate({ - format, - assetFileNames, - sourcemap, - }); - - expect(source.map).toBe(null); - await bundle.write({ assetFileNames, format, @@ -439,7 +434,7 @@ describe("/rollup.js", () => { const result = await bundle.generate({ format }); - expect(result.code).toMatchSnapshot(); + expect(result).toMatchRollupCodeSnapshot(); }); it("should log in verbose mode", async () => { diff --git a/packages/rollup/test/splitting.test.js b/packages/rollup/test/splitting.test.js index 000e06a2a..73d3634ca 100644 --- a/packages/rollup/test/splitting.test.js +++ b/packages/rollup/test/splitting.test.js @@ -23,7 +23,6 @@ const format = "es"; const map = false; const sourcemap = false; const json = true; -const experimentalCodeSplitting = true; describe("/rollup.js", () => { beforeAll(() => shell.rm("-rf", prefix("./output/*"))); @@ -31,8 +30,6 @@ describe("/rollup.js", () => { describe("code splitting", () => { it("should support splitting up CSS files", async () => { const bundle = await rollup({ - experimentalCodeSplitting, - input : [ require.resolve("./specimens/simple.js"), require.resolve("./specimens/dependencies.js"), @@ -61,8 +58,6 @@ describe("/rollup.js", () => { it("should support splitting up CSS files w/ shared assets", async () => { const bundle = await rollup({ - experimentalCodeSplitting, - input : [ require.resolve("./specimens/css-chunks/a.js"), require.resolve("./specimens/css-chunks/b.js"), @@ -91,8 +86,6 @@ describe("/rollup.js", () => { it("shouldn't put bundle-specific CSS in common.css", async () => { const bundle = await rollup({ - experimentalCodeSplitting, - input : [ require.resolve("./specimens/common-splitting/a.js"), require.resolve("./specimens/common-splitting/c.js"), @@ -121,8 +114,6 @@ describe("/rollup.js", () => { it("should support manual chunks", async () => { const bundle = await rollup({ - experimentalCodeSplitting, - input : [ require.resolve("./specimens/manual-chunks/a.js"), require.resolve("./specimens/manual-chunks/b.js"), @@ -157,8 +148,6 @@ describe("/rollup.js", () => { it("should support dynamic imports", async () => { const bundle = await rollup({ - experimentalCodeSplitting, - input : [ require.resolve("./specimens/dynamic-imports/a.js"), require.resolve("./specimens/dynamic-imports/b.js"), @@ -187,8 +176,6 @@ describe("/rollup.js", () => { it("should ouput only 1 JSON file", async () => { const bundle = await rollup({ - experimentalCodeSplitting, - input : [ require.resolve("./specimens/simple.js"), require.resolve("./specimens/dependencies.js"), @@ -218,8 +205,6 @@ describe("/rollup.js", () => { 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") ], @@ -247,8 +232,6 @@ describe("/rollup.js", () => { 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"), diff --git a/packages/test-utils/rollup-code-snapshot.js b/packages/test-utils/rollup-code-snapshot.js new file mode 100644 index 000000000..2c991bcf2 --- /dev/null +++ b/packages/test-utils/rollup-code-snapshot.js @@ -0,0 +1,22 @@ +"use strict"; + +const { toMatchSnapshot } = require("jest-snapshot"); + +expect.extend({ + toMatchRollupCodeSnapshot({ output }) { + const out = Object.create(null); + + output.forEach((chunk) => { + if(chunk.isAsset) { + return; + } + + out[chunk.name] = chunk.code; + }); + + return toMatchSnapshot.call( + this, + out, + ); + }, +});