From 36cea379b2086d47bd061867185371ed54a69e57 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 31 Mar 2020 22:41:43 +0200 Subject: [PATCH] Store locations for removed comments in cache (#3476) * add tests for merging sourcemaps * Store locations for removed comments in cache * Fix tests * Fix tests Co-authored-by: Jason Bedard --- src/Module.ts | 30 ++++++++----- src/rollup/types.d.ts | 4 +- src/utils/transform.ts | 2 +- test/misc/misc.js | 44 ++++++++++++++++--- .../_config.js | 8 ++++ .../main.js | 4 ++ .../main.js.map | 1 + .../other.js | 3 ++ .../other.js.map | 1 + .../_config.js | 8 ++++ .../main.js | 4 ++ .../other.js | 3 ++ 12 files changed, 93 insertions(+), 19 deletions(-) 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/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' + ); + }); }); 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..41b40422986 --- /dev/null +++ b/test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js @@ -0,0 +1,8 @@ +const assert = require('assert'); + +module.exports = { + description: 'removes sourcemap comments', + async test(code) { + assert.strictEqual(code.indexOf('sourceMappingURL'), -1); + } +}; 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..2457b5690f0 --- /dev/null +++ b/test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js @@ -0,0 +1,8 @@ +const assert = require('assert'); + +module.exports = { + description: 'removes existing inline sourcemaps', + async test(code) { + assert.strictEqual(code.indexOf('sourceMappingURL'), -1); + } +}; 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