diff --git a/packages/terser/README.md b/packages/terser/README.md index b9f765f82..1cf4f8382 100644 --- a/packages/terser/README.md +++ b/packages/terser/README.md @@ -73,6 +73,8 @@ export default { }; ``` +Note: when a [`nameCache`](https://github.com/terser/terser#minify-options) is provided, chunks are minified sequentially on a single worker regardless of `maxWorkers`, since a shared name cache requires sequential minification to mangle names consistently across chunks. + ## Meta [CONTRIBUTING](/.github/CONTRIBUTING.md) diff --git a/packages/terser/src/module.ts b/packages/terser/src/module.ts index 2877a6e49..cea533a41 100644 --- a/packages/terser/src/module.ts +++ b/packages/terser/src/module.ts @@ -13,6 +13,10 @@ export default function terser(input: Options = {}) { let numOfChunks = 0; let numOfWorkersUsed = 0; + // Chained minify tasks; used only when a shared nameCache is provided, + // since terser requires sequential minification for consistent mangling. + let nameCacheChain: Promise = Promise.resolve(); + return { name: 'terser', @@ -24,6 +28,8 @@ export default function terser(input: Options = {}) { }); } + const pool = workerPool; + numOfChunks += 1; const defaultOptions: Options = { @@ -38,12 +44,12 @@ export default function terser(input: Options = {}) { defaultOptions.toplevel = true; } - try { + const runTask = async () => { const { code: result, nameCache, sourceMap - } = await workerPool.addAsync({ + } = await pool.addAsync({ code, options: merge({}, options || {}, defaultOptions) }); @@ -79,13 +85,28 @@ export default function terser(input: Options = {}) { options.nameCache.props = props; } - if ((!!defaultOptions.sourceMap || !!options.sourceMap) && isObject(sourceMap)) { + return { result, sourceMap }; + }; + + try { + let output; + if (options.nameCache) { + // The options snapshot and its serialization must only happen once + // the previous chunk's nameCache has been merged back. + const task = nameCacheChain.then(runTask); + nameCacheChain = task.catch(() => {}); + output = await task; + } else { + output = await runTask(); + } + + if ((!!defaultOptions.sourceMap || !!options.sourceMap) && isObject(output.sourceMap)) { return { - code: result, - map: sourceMap + code: output.result, + map: output.sourceMap }; } - return result; + return output.result; } catch (e) { return Promise.reject(e); } finally { diff --git a/packages/terser/test/fixtures/chunk-1.js b/packages/terser/test/fixtures/chunk-1.js index daff3a741..c27eb3d83 100644 --- a/packages/terser/test/fixtures/chunk-1.js +++ b/packages/terser/test/fixtures/chunk-1.js @@ -1,2 +1 @@ -const chunk1 = 'chunk-1'; -console.log(chunk1) +console.log({ _name: 'chunk-1', _size: 1 }); diff --git a/packages/terser/test/fixtures/chunk-2.js b/packages/terser/test/fixtures/chunk-2.js index 7c4513c6e..8db5e9338 100644 --- a/packages/terser/test/fixtures/chunk-2.js +++ b/packages/terser/test/fixtures/chunk-2.js @@ -1,2 +1 @@ -const chunk2 = 'chunk-2'; -console.log(chunk2) +console.log({ _size: 2, _name: 'chunk-2' }); diff --git a/packages/terser/test/fixtures/name-cache-entry-1.js b/packages/terser/test/fixtures/name-cache-entry-1.js new file mode 100644 index 000000000..013f534ef --- /dev/null +++ b/packages/terser/test/fixtures/name-cache-entry-1.js @@ -0,0 +1,4 @@ +/* eslint-disable no-underscore-dangle */ +import { Foo } from './name-cache-shared.js'; + +console.log(new Foo()._bar()); diff --git a/packages/terser/test/fixtures/name-cache-entry-2.js b/packages/terser/test/fixtures/name-cache-entry-2.js new file mode 100644 index 000000000..4c7e5b72f --- /dev/null +++ b/packages/terser/test/fixtures/name-cache-entry-2.js @@ -0,0 +1,4 @@ +/* eslint-disable no-underscore-dangle */ +import { Foo } from './name-cache-shared.js'; + +console.log(new Foo()._baz(), new Foo()._bar()); diff --git a/packages/terser/test/fixtures/name-cache-shared.js b/packages/terser/test/fixtures/name-cache-shared.js new file mode 100644 index 000000000..0d7198c87 --- /dev/null +++ b/packages/terser/test/fixtures/name-cache-shared.js @@ -0,0 +1,9 @@ +/* eslint-disable class-methods-use-this, no-underscore-dangle */ +export class Foo { + _bar() { + return 'bar'; + } + _baz() { + return 'baz'; + } +} diff --git a/packages/terser/test/test.js b/packages/terser/test/test.js index 78366dbc3..447ac9c16 100644 --- a/packages/terser/test/test.js +++ b/packages/terser/test/test.js @@ -432,3 +432,144 @@ test.sequential('terser preserve vars in nameCache when provided', async () => { } }); }); + +// https://github.com/rollup/plugins/issues/1970 +// The same property must mangle to the same name in every chunk of a build. +async function buildSharedImportChunks(maxWorkers) { + const nameCache = {}; + const bundle = await rollup({ + input: ['test/fixtures/name-cache-entry-1.js', 'test/fixtures/name-cache-entry-2.js'], + plugins: [ + terser({ + mangle: { + properties: { + regex: /^_/ + } + }, + nameCache, + maxWorkers + }) + ] + }); + const result = await bundle.generate({ + format: 'es', + chunkFileNames: 'chunk-[name].js' + }); + expect(result.output.length).toBe(3); + const bar = nameCache.props.props.$_bar; + const baz = nameCache.props.props.$_baz; + expect(bar).toBeTruthy(); + expect(baz).toBeTruthy(); + expect(bar).not.toBe(baz); + for (const chunk of result.output) { + expect(chunk.code).not.toContain('_bar'); + expect(chunk.code).not.toContain('_baz'); + } + const entry1 = result.output.find((chunk) => chunk.fileName === 'name-cache-entry-1.js'); + const entry2 = result.output.find((chunk) => chunk.fileName === 'name-cache-entry-2.js'); + const shared = result.output.find((chunk) => !chunk.isEntry); + expect(entry1.code).toContain(`.${bar}(`); + expect(entry2.code).toContain(`.${bar}(`); + expect(entry2.code).toContain(`.${baz}(`); + expect(shared.code).toContain(`${bar}(){return"bar"}`); + expect(shared.code).toContain(`${baz}(){return"baz"}`); +} +test.sequential('shares nameCache between chunks with a common import', async () => { + await buildSharedImportChunks(); +}); +test.sequential( + 'shares nameCache between chunks with a common import with only 1 worker', + async () => { + await buildSharedImportChunks(1); + } +); + +async function buildUnrelatedChunks(maxWorkers) { + const nameCache = {}; + const bundle = await rollup({ + input: ['test/fixtures/chunk-1.js', 'test/fixtures/chunk-2.js'], + plugins: [ + terser({ + mangle: { + properties: { + regex: /^_/ + } + }, + nameCache, + maxWorkers + }) + ] + }); + const result = await bundle.generate({ + format: 'es' + }); + expect(result.output.length).toBe(2); + const name = nameCache.props.props.$_name; + const size = nameCache.props.props.$_size; + expect(name).toBeTruthy(); + expect(size).toBeTruthy(); + expect(name).not.toBe(size); + const chunk1 = result.output.find((chunk) => chunk.fileName === 'chunk-1.js'); + const chunk2 = result.output.find((chunk) => chunk.fileName === 'chunk-2.js'); + expect(chunk1.code).toContain(`${name}:"chunk-1"`); + expect(chunk1.code).toContain(`${size}:1`); + expect(chunk2.code).toContain(`${name}:"chunk-2"`); + expect(chunk2.code).toContain(`${size}:2`); +} +test.sequential('shares nameCache between unrelated chunks', async () => { + await buildUnrelatedChunks(); +}); +test.sequential('shares nameCache between unrelated chunks with only 1 worker', async () => { + await buildUnrelatedChunks(1); +}); + +test.sequential('mangles unrelated chunks independently without a nameCache', async () => { + const bundle = await rollup({ + input: ['test/fixtures/chunk-1.js', 'test/fixtures/chunk-2.js'], + plugins: [ + terser({ + mangle: { + properties: { + regex: /^_/ + } + } + }) + ] + }); + const result = await bundle.generate({ + format: 'es' + }); + expect(result.output.length).toBe(2); + const chunk1 = result.output.find((chunk) => chunk.fileName === 'chunk-1.js'); + const chunk2 = result.output.find((chunk) => chunk.fileName === 'chunk-2.js'); + const [, chunk1Name] = chunk1.code.match(/[{,]([$\w]+):"chunk-1"/); + const [, chunk2Name] = chunk2.code.match(/[{,]([$\w]+):"chunk-2"/); + expect(chunk1Name).toBeTruthy(); + expect(chunk2Name).toBeTruthy(); + // Each chunk starts from its own empty cache, so the first `_`-property + // encountered in each file gets the same first mangled name; `_name` is + // declared first in chunk-1 and second in chunk-2, so its name diverges. + expect(chunk1Name).not.toBe(chunk2Name); +}); + +test.sequential('uses a single worker when a nameCache is shared', async () => { + let plugin; + const bundle = await rollup({ + input: ['test/fixtures/name-cache-entry-1.js', 'test/fixtures/name-cache-entry-2.js'], + plugins: [ + (plugin = terser({ + mangle: { + properties: { + regex: /^_/ + } + }, + nameCache: {}, + maxWorkers: 4 + })) + ] + }); + await bundle.generate({ + format: 'es' + }); + expect(plugin.numOfWorkersUsed).toBe(1); +});