From f5be0cb8a1db4c395143c069fce7ab33fdf78b3c Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Sun, 29 Mar 2020 20:56:32 -0700 Subject: [PATCH 1/4] add tests for merging sourcemaps --- .../existing-external-sourcemaps-combined/_config.js | 9 +++++++++ .../existing-external-sourcemaps-combined/main.js | 4 ++++ .../existing-external-sourcemaps-combined/main.js.map | 1 + .../existing-external-sourcemaps-combined/other.js | 3 +++ .../existing-external-sourcemaps-combined/other.js.map | 1 + .../existing-inline-sourcemaps-combined/_config.js | 8 ++++++++ .../samples/existing-inline-sourcemaps-combined/main.js | 4 ++++ .../samples/existing-inline-sourcemaps-combined/other.js | 3 +++ 8 files changed, 33 insertions(+) create mode 100644 test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js create mode 100644 test/sourcemaps/samples/existing-external-sourcemaps-combined/main.js create mode 100644 test/sourcemaps/samples/existing-external-sourcemaps-combined/main.js.map create mode 100644 test/sourcemaps/samples/existing-external-sourcemaps-combined/other.js create mode 100644 test/sourcemaps/samples/existing-external-sourcemaps-combined/other.js.map create mode 100644 test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js create mode 100644 test/sourcemaps/samples/existing-inline-sourcemaps-combined/main.js create mode 100644 test/sourcemaps/samples/existing-inline-sourcemaps-combined/other.js diff --git a/test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js b/test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js new file mode 100644 index 00000000000..a377059f3ad --- /dev/null +++ b/test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js @@ -0,0 +1,9 @@ +const assert = require('assert'); + +module.exports = { + description: 'combines existing inline sourcemap comments into one', + async test(code) { + assert.doesNotMatch(code, /sourceMappingURL=main\.js\.map/); + assert.doesNotMatch(code, /sourceMappingURL=other\.js\.map/); + }, +}; diff --git a/test/sourcemaps/samples/existing-external-sourcemaps-combined/main.js b/test/sourcemaps/samples/existing-external-sourcemaps-combined/main.js new file mode 100644 index 00000000000..9f837dbfb00 --- /dev/null +++ b/test/sourcemaps/samples/existing-external-sourcemaps-combined/main.js @@ -0,0 +1,4 @@ +import value from "./other"; +const s = `other: ${value}`; +console.log(s); +//# sourceMappingURL=main.js.map \ No newline at end of file diff --git a/test/sourcemaps/samples/existing-external-sourcemaps-combined/main.js.map b/test/sourcemaps/samples/existing-external-sourcemaps-combined/main.js.map new file mode 100644 index 00000000000..869d5b39321 --- /dev/null +++ b/test/sourcemaps/samples/existing-external-sourcemaps-combined/main.js.map @@ -0,0 +1 @@ +{"version":3,"file":"main.js","sourceRoot":"","sources":["main.ts"],"names":[],"mappings":"AAAA,OAAO,KAAK,MAAM,SAAS,CAAC;AAE5B,MAAM,CAAC,GAAW,UAAU,KAAK,EAAE,CAAC;AAEpC,OAAO,CAAC,GAAG,CAAC,CAAC,CAAC,CAAC"} \ No newline at end of file diff --git a/test/sourcemaps/samples/existing-external-sourcemaps-combined/other.js b/test/sourcemaps/samples/existing-external-sourcemaps-combined/other.js new file mode 100644 index 00000000000..692376e46d9 --- /dev/null +++ b/test/sourcemaps/samples/existing-external-sourcemaps-combined/other.js @@ -0,0 +1,3 @@ +const x = "foo"; +export default x; +//# sourceMappingURL=other.js.map \ No newline at end of file diff --git a/test/sourcemaps/samples/existing-external-sourcemaps-combined/other.js.map b/test/sourcemaps/samples/existing-external-sourcemaps-combined/other.js.map new file mode 100644 index 00000000000..26062a454fa --- /dev/null +++ b/test/sourcemaps/samples/existing-external-sourcemaps-combined/other.js.map @@ -0,0 +1 @@ +{"version":3,"file":"other.js","sourceRoot":"","sources":["other.ts"],"names":[],"mappings":"AAAA,MAAM,CAAC,GAAW,KAAK,CAAC;AACxB,eAAe,CAAC,CAAC"} \ No newline at end of file diff --git a/test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js b/test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js new file mode 100644 index 00000000000..d77fb87c969 --- /dev/null +++ b/test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js @@ -0,0 +1,8 @@ +const assert = require('assert'); + +module.exports = { + description: 'combines existing sourcemap files into one', + async test(code) { + assert.doesNotMatch(code, /sourceMappingURL=data:/); + }, +}; diff --git a/test/sourcemaps/samples/existing-inline-sourcemaps-combined/main.js b/test/sourcemaps/samples/existing-inline-sourcemaps-combined/main.js new file mode 100644 index 00000000000..e6bb890fc98 --- /dev/null +++ b/test/sourcemaps/samples/existing-inline-sourcemaps-combined/main.js @@ -0,0 +1,4 @@ +import value from "./other"; +const s = `other: ${value}`; +console.log(s); +//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoibWFpbi5qcyIsInNvdXJjZVJvb3QiOiIiLCJzb3VyY2VzIjpbIm1haW4udHMiXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IkFBQUEsT0FBTyxLQUFLLE1BQU0sU0FBUyxDQUFDO0FBRTVCLE1BQU0sQ0FBQyxHQUFXLFVBQVUsS0FBSyxFQUFFLENBQUM7QUFFcEMsT0FBTyxDQUFDLEdBQUcsQ0FBQyxDQUFDLENBQUMsQ0FBQyJ9 \ No newline at end of file diff --git a/test/sourcemaps/samples/existing-inline-sourcemaps-combined/other.js b/test/sourcemaps/samples/existing-inline-sourcemaps-combined/other.js new file mode 100644 index 00000000000..d694ffb8a20 --- /dev/null +++ b/test/sourcemaps/samples/existing-inline-sourcemaps-combined/other.js @@ -0,0 +1,3 @@ +const x = "foo"; +export default x; +//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoib3RoZXIuanMiLCJzb3VyY2VSb290IjoiIiwic291cmNlcyI6WyJvdGhlci50cyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQSxNQUFNLENBQUMsR0FBVyxLQUFLLENBQUM7QUFDeEIsZUFBZSxDQUFDLENBQUMifQ== \ No newline at end of file From 0147318d636ea53e23111a1846ab592dc57e4d85 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 31 Mar 2020 20:18:56 +0200 Subject: [PATCH 2/4] Store locations for removed comments in cache --- .prettierrc | 6 ++++-- src/Module.ts | 30 +++++++++++++++++----------- src/rollup/types.d.ts | 4 +++- src/utils/transform.ts | 2 +- test/misc/misc.js | 44 ++++++++++++++++++++++++++++++++++++------ 5 files changed, 65 insertions(+), 21 deletions(-) diff --git a/.prettierrc b/.prettierrc index 2a1740fd7ae..ea138e8454c 100644 --- a/.prettierrc +++ b/.prettierrc @@ -1,5 +1,7 @@ { + "arrowParens": "avoid", + "printWidth": 100, "singleQuote": true, - "useTabs": true, - "printWidth": 100 + "trailingComma": "none", + "useTabs": true } diff --git a/src/Module.ts b/src/Module.ts index a5f04e32c6e..a8b8450fb20 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -236,6 +236,7 @@ export default class Module { usesTopLevelAwait = false; private allExportNames: Set | null = null; + private alwaysRemovedCode!: [number, number][]; private ast!: Program; private astContext!: AstContext; private context: string; @@ -619,6 +620,7 @@ export default class Module { } setSource({ + alwaysRemovedCode, ast, code, customTransformCache, @@ -631,6 +633,7 @@ export default class Module { transformDependencies, transformFiles }: TransformModuleJSON & { + alwaysRemovedCode?: [number, number][]; transformFiles?: EmittedFile[] | undefined; }) { this.code = code; @@ -651,8 +654,18 @@ export default class Module { timeStart('generate ast', 3); - this.esTreeAst = ast || tryParse(this, this.graph.acornParser, this.graph.acornOptions); - markPureCallExpressions(this.comments, this.esTreeAst); + this.alwaysRemovedCode = alwaysRemovedCode || []; + if (ast) { + this.esTreeAst = ast; + } else { + this.esTreeAst = tryParse(this, this.graph.acornParser, this.graph.acornOptions); + for (const comment of this.comments) { + if (!comment.block && SOURCEMAPPING_URL_RE.test(comment.text)) { + this.alwaysRemovedCode.push([comment.start, comment.end]); + } + } + markPureCallExpressions(this.comments, this.esTreeAst); + } timeEnd('generate ast', 3); @@ -666,7 +679,9 @@ export default class Module { filename: (this.excludeFromSourcemap ? null : fileName)!, // don't include plugin helpers in sourcemap indentExclusionRanges: [] }); - this.removeExistingSourceMap(); + for (const [start, end] of this.alwaysRemovedCode) { + this.magicString.remove(start, end); + } timeStart('analyse ast', 3); @@ -720,6 +735,7 @@ export default class Module { toJSON(): ModuleJSON { return { + alwaysRemovedCode: this.alwaysRemovedCode, ast: this.esTreeAst, code: this.code, customTransformCache: this.customTransformCache, @@ -912,14 +928,6 @@ export default class Module { } } - private removeExistingSourceMap() { - for (const comment of this.comments) { - if (!comment.block && SOURCEMAPPING_URL_RE.test(comment.text)) { - this.magicString.remove(comment.start, comment.end); - } - } - } - private shimMissingExport(name: string): void { if (!this.exports[name]) { this.graph.warn({ diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 265a6237b47..1f894b02012 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -94,7 +94,7 @@ export interface SourceDescription { } export interface TransformModuleJSON { - ast: AcornNode; + ast?: AcornNode; code: string; // note if plugins use new this.cache to opt-out auto transform cache customTransformCache: boolean; @@ -108,6 +108,8 @@ export interface TransformModuleJSON { } export interface ModuleJSON extends TransformModuleJSON { + alwaysRemovedCode: [number, number][]; + ast: AcornNode; dependencies: string[]; id: string; transformFiles: EmittedFile[] | undefined; diff --git a/src/utils/transform.ts b/src/utils/transform.ts index c7a0b9b3d7b..297a0dc04b7 100644 --- a/src/utils/transform.ts +++ b/src/utils/transform.ts @@ -160,7 +160,7 @@ export default function transform( } return { - ast: ast!, + ast, code, customTransformCache, moduleSideEffects, diff --git a/test/misc/misc.js b/test/misc/misc.js index 55a0d487da3..3558b8edd42 100644 --- a/test/misc/misc.js +++ b/test/misc/misc.js @@ -108,12 +108,10 @@ describe('misc', () => { .then(bundle => bundle.generate({ format: 'es' })) .then(({ output }) => { assert.equal(warnings.length, 0); - assert.deepEqual(output.map(({ fileName }) => fileName), [ - 'main1.js', - 'main2.js', - 'dep-f8bec8a7.js', - 'dyndep-b0a9ee12.js' - ]); + assert.deepEqual( + output.map(({ fileName }) => fileName), + ['main1.js', 'main2.js', 'dep-f8bec8a7.js', 'dyndep-b0a9ee12.js'] + ); }); }); @@ -175,4 +173,38 @@ describe('misc', () => { assert.strictEqual(output.output[0].code, 'var input = 42;\n\nexport default input;\n') ); }); + + it('consistently handles comments when using the cache', async () => { + const FILES = { + main: `import value from "other"; +console.log(value); +/*#__PURE__*/console.log('removed');`, + other: `var x = "foo"; +export default x; +//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoib3RoZXIuanMiLCJzb3VyY2VSb290IjoiIiwic291cmNlcyI6WyJvdGhlci50cyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQSxJQUFNLENBQUMsR0FBVyxLQUFLLENBQUM7QUFDeEIsZUFBZSxDQUFDLENBQUMifQ==` + }; + const EXPECTED_OUTPUT = `var x = "foo"; + +console.log(x); +`; + const firstBundle = await rollup.rollup({ + input: 'main', + plugins: [loader(FILES)] + }); + assert.strictEqual( + (await firstBundle.generate({ format: 'es' })).output[0].code, + EXPECTED_OUTPUT, + 'first' + ); + const secondBundle = await rollup.rollup({ + cache: firstBundle.cache, + input: 'main', + plugins: [loader(FILES)] + }); + assert.strictEqual( + (await secondBundle.generate({ format: 'es' })).output[0].code, + EXPECTED_OUTPUT, + 'second' + ); + }); }); From 8e841b2c932887c9be6b7b4ee765e929f402b6fc Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 31 Mar 2020 22:08:27 +0200 Subject: [PATCH 3/4] Fix tests --- .../existing-external-sourcemaps-combined/_config.js | 8 ++++---- .../existing-inline-sourcemaps-combined/_config.js | 7 ++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js b/test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js index a377059f3ad..4a078a4d38e 100644 --- a/test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js +++ b/test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js @@ -1,9 +1,9 @@ const assert = require('assert'); module.exports = { - description: 'combines existing inline sourcemap comments into one', + solo: true, + description: 'removes sourcemap comments', async test(code) { - assert.doesNotMatch(code, /sourceMappingURL=main\.js\.map/); - assert.doesNotMatch(code, /sourceMappingURL=other\.js\.map/); - }, + assert.strictEqual(code.indexOf('sourceMappingURL'), -1); + } }; diff --git a/test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js b/test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js index d77fb87c969..09a35337622 100644 --- a/test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js +++ b/test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js @@ -1,8 +1,9 @@ const assert = require('assert'); module.exports = { - description: 'combines existing sourcemap files into one', + solo: true, + description: 'removes existing inline sourcemaps', async test(code) { - assert.doesNotMatch(code, /sourceMappingURL=data:/); - }, + assert.strictEqual(code.indexOf('sourceMappingURL'), -1); + } }; From dee436e7059e95134cb15c29d5b02d3ab805c0b0 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 31 Mar 2020 22:08:27 +0200 Subject: [PATCH 4/4] Fix tests --- .../samples/existing-external-sourcemaps-combined/_config.js | 1 - .../samples/existing-inline-sourcemaps-combined/_config.js | 1 - 2 files changed, 2 deletions(-) diff --git a/test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js b/test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js index 4a078a4d38e..41b40422986 100644 --- a/test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js +++ b/test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js @@ -1,7 +1,6 @@ const assert = require('assert'); module.exports = { - solo: true, description: 'removes sourcemap comments', async test(code) { assert.strictEqual(code.indexOf('sourceMappingURL'), -1); diff --git a/test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js b/test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js index 09a35337622..2457b5690f0 100644 --- a/test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js +++ b/test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js @@ -1,7 +1,6 @@ const assert = require('assert'); module.exports = { - solo: true, description: 'removes existing inline sourcemaps', async test(code) { assert.strictEqual(code.indexOf('sourceMappingURL'), -1);