Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update to Terser v5 #237

Merged
merged 5 commits into from
Sep 4, 2020
Merged

Update to Terser v5 #237

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
16fa059
bump terser from v4.8.0 to v5.2.1
theenadayalank Aug 25, 2020
6e8f316
use async/await
theenadayalank Sep 3, 2020
ff06ff0
remove unwanted return
theenadayalank Sep 3, 2020
a8d945d
test fix
theenadayalank Sep 3, 2020
32a6426
make use of finally in promise
theenadayalank Sep 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 18 additions & 21 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,11 @@ function UglifyWriter(inputNodes, options) {
},
});

// consumers of this plugin can opt-in to async and concurrent behavior
this.async = (this.options.async === true);
// async prop is deprecated since terser.minify() is async by default
if ('async' in this.options) {
throw new Error('\n Passing `async` property inside `options` is deprecated.');
}

this.concurrency = Number(process.env.JOBS) || this.options.concurrency || Math.max(require('os').cpus().length - 1, 1);

// create a worker pool using an external worker script
Expand All @@ -63,10 +66,9 @@ function UglifyWriter(inputNodes, options) {
}
}

UglifyWriter.prototype.build = function() {
UglifyWriter.prototype.build = async function() {
let writer = this;

// when options.async === true, allow processFile() operations to complete asynchronously
let pendingWork = [];

this.inputPaths.forEach(inputPath => {
Expand All @@ -84,11 +86,7 @@ UglifyWriter.prototype.build = function() {
let uglifyOperation = function() {
return writer.processFile(inFile, outFile, relativePath, writer.outputPath);
};
if (writer.async) {
pendingWork.push(uglifyOperation);
return;
}
return uglifyOperation();
pendingWork.push(uglifyOperation);
} else if (relativePath.slice(-4) === '.map') {
if (writer.excludes.match(`${relativePath.slice(0, -4)}.{js,mjs}`)) {
// ensure .map files for excluded JS paths are also copied forward
Expand All @@ -101,17 +99,16 @@ UglifyWriter.prototype.build = function() {
});
});

return queue(worker, pendingWork, writer.concurrency)
.then((/* results */) => {
// files are finished processing, shut down the workers
writer.pool.terminate();
return writer.outputPath;
})
.catch(e => {
// make sure to shut down the workers on error
writer.pool.terminate();
throw e;
});
try {
await queue(worker, pendingWork, writer.concurrency);
// files are finished processing, shut down the workers
writer.pool.terminate();
return writer.outputPath;
} catch (e) {
// make sure to shut down the workers on error
writer.pool.terminate();
throw e;
}
rwjblue marked this conversation as resolved.
Show resolved Hide resolved
};

UglifyWriter.prototype._isJSExt = function(relativePath) {
Expand All @@ -120,7 +117,7 @@ UglifyWriter.prototype._isJSExt = function(relativePath) {

UglifyWriter.prototype.processFile = function(inFile, outFile, relativePath, outDir) {
// don't run this in the workerpool if concurrency is disabled (can set JOBS <= 1)
if (this.async && this.concurrency > 1) {
if (this.concurrency > 1) {
debug('running in workerpool, concurrency=%d', this.concurrency);
// each of these arguments is a string, which can be sent to the worker process as-is
return this.pool.exec('processFileParallel', [inFile, outFile, relativePath, outDir, silent, this.options]);
Expand Down
82 changes: 41 additions & 41 deletions lib/process-file.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@ const defaults = require('lodash.defaultsdeep');
const fs = require('fs');
const mkdirp = require('mkdirp');
const path = require('path');
const getSourceMapContent = require('./get-sourcemap-content');

const terser = require('terser');
const getSourceMapContent = require('./get-sourcemap-content');


module.exports = function processFile(inFile, outFile, relativePath, outDir, silent, _options) {
module.exports = async function processFile(inFile, outFile, relativePath, outDir, silent, _options) {
let src = fs.readFileSync(inFile, 'utf-8');
let mapName = `${path.basename(outFile).replace(/\.js$/, '')}.map`;

Expand Down Expand Up @@ -48,45 +46,47 @@ module.exports = function processFile(inFile, outFile, relativePath, outDir, sil

let start = new Date();
debug('[starting]: %s %dKB', relativePath, (src.length / 1000));
let result = terser.minify(src, options);
let end = new Date();
let total = end - start;
if (total > 20000 && !silent) {
console.warn(`[WARN] (broccoli-uglify-sourcemap) Minifying "${relativePath}" took: ${total}ms (more than 20,000ms)`);
}

if (result.error) {
result.error.filename = relativePath;
throw result.error;
}

debug('[finished]: %s %dKB in %dms', relativePath, (result.code.length / 1000), total);
try {
let result = await terser.minify(src, options);
let end = new Date();
let total = end - start;

if (options.sourceMap) {
let newSourceMap = JSON.parse(result.map);
if (total > 20000 && !silent) {
console.warn(`[WARN] (broccoli-uglify-sourcemap) Minifying "${relativePath}" took: ${total}ms (more than 20,000ms)`);
}

newSourceMap.sources = newSourceMap.sources.map(function(path) {
if (path === relativePath) {
// If out output file has the same name as one of our original
// sources, they will shadow eachother in Dev Tools. So instead we
// alter the reference to the upstream file.
return path.replace(/\.js$/, '-orig.js');
} else if (path === '0') {
// In [terser-js](https://github.com/terser-js/terser#source-map-options),
// sources are always 0 if old sourcemaps are not provided.
// The value passed for `sourceMap.url` is only used to set
// `//# sourceMappingURL=out.js.map` in `result.code`.
// The value of `filename` is only used to set `file` attribute
// in source map file.
// In broccoli-uglify-sourcemap we know in this case we are generating
// sourcemap for the file we are processing, changing 0 to the actual
// file gives us the correct source.
return relativePath;
}
return path;
});
mkdirp.sync(mapDir);
fs.writeFileSync(path.join(mapDir, mapName), JSON.stringify(newSourceMap));
debug('[finished]: %s %dKB in %dms', relativePath, (result.code.length / 1000), total);

if (options.sourceMap) {
let newSourceMap = JSON.parse(result.map);

newSourceMap.sources = newSourceMap.sources.map(function(path) {
if (path === relativePath) {
// If out output file has the same name as one of our original
// sources, they will shadow eachother in Dev Tools. So instead we
// alter the reference to the upstream file.
return path.replace(/\.js$/, '-orig.js');
} else if (path === '0') {
// In [terser-js](https://github.com/terser-js/terser#source-map-options),
// sources are always 0 if old sourcemaps are not provided.
// The value passed for `sourceMap.url` is only used to set
// `//# sourceMappingURL=out.js.map` in `result.code`.
// The value of `filename` is only used to set `file` attribute
// in source map file.
// In broccoli-uglify-sourcemap we know in this case we are generating
// sourcemap for the file we are processing, changing 0 to the actual
// file gives us the correct source.
return relativePath;
}
return path;
});
mkdirp.sync(mapDir);
fs.writeFileSync(path.join(mapDir, mapName), JSON.stringify(newSourceMap));
}
fs.writeFileSync(outFile, result.code);
} catch (e) {
e.filename = relativePath;
throw e;
}
fs.writeFileSync(outFile, result.code);
};
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"mkdirp": "^0.5.0",
"source-map-url": "^0.4.0",
"symlink-or-copy": "^1.3.1",
"terser": "^4.8.0",
"terser": "^5.2.1",
"walk-sync": "^1.1.3",
"workerpool": "^5.0.4"
},
Expand Down
31 changes: 0 additions & 31 deletions test/__snapshots__/test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,35 +105,6 @@ Object {
}
`;

exports[`broccoli-uglify-sourcemap generates expected output async 1`] = `
Object {
"inside": Object {
"with-upstream-sourcemap.js": "function meaningOfLife(){throw new Error(42)}function boom(){throw new Error(\\"boom\\")}function somethingElse(){throw new Error(\\"somethign else\\")}function fourth(){throw new Error(\\"fourth\\")}function third(){throw new Error(\\"oh no\\")}
//# sourceMappingURL=with-upstream-sourcemap.map",
"with-upstream-sourcemap.map": "{\\"version\\":3,\\"sources\\":[\\"/inner/first.js\\",\\"/inner/second.js\\",\\"/other/fourth.js\\",\\"/other/third.js\\"],\\"names\\":[\\"meaningOfLife\\",\\"Error\\",\\"boom\\",\\"somethingElse\\",\\"fourth\\",\\"third\\"],\\"mappings\\":\\"AAAA,SAAAA,gBAEA,MAAA,IAAAC,MADA,IAIA,SAAAC,OACA,MAAA,IAAAD,MAAA,QCNA,SAAAE,gBACA,MAAA,IAAAF,MAAA,kBCEA,SAAAG,SACA,MAAA,IAAAH,MAAA,UCJA,SAAAI,QACA,MAAA,IAAAJ,MAAA\\",\\"file\\":\\"with-upstream-sourcemap.js\\",\\"sourcesContent\\":[\\"function meaningOfLife() {\\\\n var thisIsALongLocal = 42;\\\\n throw new Error(thisIsALongLocal);\\\\n}\\\\n\\\\nfunction boom() {\\\\n throw new Error('boom');\\\\n}\\\\n\\",\\"function somethingElse() {\\\\n throw new Error(\\\\\\"somethign else\\\\\\");\\\\n}\\\\n\\",\\"\\\\n// Hello world\\\\n\\\\nfunction fourth(){\\\\n throw new Error('fourth');\\\\n}\\\\n\\",\\"function third(){\\\\n throw new Error(\\\\\\"oh no\\\\\\");\\\\n}\\\\n\\"]}",
},
"mjs": Object {
"index.mjs": "const MJS_I_GUESS=\\"ok\\";export default\\"ok\\";
//# sourceMappingURL=index.mjs.map",
"index.mjs.map": "{\\"version\\":3,\\"sources\\":[\\"mjs/index.mjs\\"],\\"names\\":[\\"MJS_I_GUESS\\"],\\"mappings\\":\\"AAAA,MAAMA,YAAc,mBAAA\\",\\"file\\":\\"index.mjs\\"}",
},
"multi": Object {
"with-multi-sourcemap.js": "function meaningOfLife(){throw new Error(42)}function boom(){throw new Error(\\"boom\\")}function somethingElse(){throw new Error(\\"somethign else\\")}function fourth(){throw new Error(\\"fourth\\")}function third(){throw new Error(\\"oh no\\")}
//# sourceMappingURL=with-multi-sourcemap.map",
"with-multi-sourcemap.map": "{\\"version\\":3,\\"sources\\":[\\"/inner/first.js\\",\\"/inner/second.js\\",\\"/other/fourth.js\\",\\"/other/third.js\\"],\\"names\\":[\\"meaningOfLife\\",\\"Error\\",\\"boom\\",\\"somethingElse\\",\\"fourth\\",\\"third\\"],\\"mappings\\":\\"AAAA,SAAAA,gBAEA,MAAA,IAAAC,MADA,IAIA,SAAAC,OACA,MAAA,IAAAD,MAAA,QCNA,SAAAE,gBACA,MAAA,IAAAF,MAAA,kBCEA,SAAAG,SACA,MAAA,IAAAH,MAAA,UCJA,SAAAI,QACA,MAAA,IAAAJ,MAAA\\",\\"file\\":\\"with-multi-sourcemap.js\\",\\"sourcesContent\\":[\\"function meaningOfLife() {\\\\n var thisIsALongLocal = 42;\\\\n throw new Error(thisIsALongLocal);\\\\n}\\\\n\\\\nfunction boom() {\\\\n throw new Error('boom');\\\\n}\\\\n\\",\\"function somethingElse() {\\\\n throw new Error(\\\\\\"somethign else\\\\\\");\\\\n}\\\\n\\",\\"\\\\n// Hello world\\\\n\\\\nfunction fourth(){\\\\n throw new Error('fourth');\\\\n}\\\\n\\",\\"function third(){\\\\n throw new Error(\\\\\\"oh no\\\\\\");\\\\n}\\\\n\\"]}",
},
"no-upstream-sourcemap.js": "function meaningOfLife(){throw new Error(42)}function boom(){throw new Error(\\"boom\\")}function somethingElse(){throw new Error(\\"somethign else\\")}function fourth(){throw new Error(\\"fourth\\")}function third(){throw new Error(\\"oh no\\")}
//# sourceMappingURL=no-upstream-sourcemap.map",
"no-upstream-sourcemap.map": "{\\"version\\":3,\\"sources\\":[\\"no-upstream-sourcemap.js\\"],\\"names\\":[\\"meaningOfLife\\",\\"Error\\",\\"boom\\",\\"somethingElse\\",\\"fourth\\",\\"third\\"],\\"mappings\\":\\"AACA,SAASA,gBAEP,MAAM,IAAIC,MADa,IAIzB,SAASC,OACP,MAAM,IAAID,MAAM,QAGlB,SAASE,gBACP,MAAM,IAAIF,MAAM,kBAMlB,SAASG,SACP,MAAM,IAAIH,MAAM,UAGlB,SAASI,QACP,MAAM,IAAIJ,MAAM\\",\\"file\\":\\"no-upstream-sourcemap.js\\"}",
"something.css": "#id {
background: white;
}",
"with-broken-sourcemap.js": "function meaningOfLife(){throw new Error(42)}
//# sourceMappingURL=with-broken-sourcemap.map",
"with-broken-sourcemap.map": "{\\"version\\":3,\\"sources\\":[\\"with-broken-sourcemap.js\\"],\\"names\\":[\\"meaningOfLife\\",\\"Error\\"],\\"mappings\\":\\"AAAA,SAASA,gBAEP,MAAM,IAAIC,MADa\\",\\"file\\":\\"with-broken-sourcemap.js\\"}",
}
`;

exports[`broccoli-uglify-sourcemap mjs can uglify .mjs files 1`] = `
Object {
"inside": Object {
Expand Down Expand Up @@ -165,8 +136,6 @@ Object {

exports[`broccoli-uglify-sourcemap on error rejects with BuildError 1`] = `Object {}`;

exports[`broccoli-uglify-sourcemap on error rejects with BuildError async 1`] = `Object {}`;

exports[`broccoli-uglify-sourcemap on error shuts down the workerpool 1`] = `Object {}`;

exports[`broccoli-uglify-sourcemap shuts down the workerpool 1`] = `
Expand Down
47 changes: 20 additions & 27 deletions test/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,6 @@ describe('broccoli-uglify-sourcemap', function() {
expect(builder.read()).toMatchSnapshot();
});

it('generates expected output async', async function() {
builder = createBuilder(new Uglify(fixtures, { async: true }));

await builder.build();

expect(builder.read()).toMatchSnapshot();
});
rwjblue marked this conversation as resolved.
Show resolved Hide resolved

it('can handle ES6 code', async function() {
input.write({
'es6.js': `class Foo {
Expand Down Expand Up @@ -97,7 +89,7 @@ let { bar } = Foo.prototype;`,
});

it('shuts down the workerpool', async function() {
let testUglify = new Uglify(fixtures, { async: true });
let testUglify = new Uglify(fixtures);
builder = createBuilder(testUglify);

await builder.build();
Expand All @@ -108,20 +100,7 @@ let { bar } = Foo.prototype;`,

describe('on error', function() {
it('rejects with BuildError', async function() {
builder = createBuilder(new Uglify(fixturesError, {}));

let shouldError;
await builder.build()
.catch(err => {
shouldError = err;
});
expect(shouldError.name).toEqual('BuildError');

expect(builder.read()).toMatchSnapshot();
});

it('rejects with BuildError async', async function() {
builder = createBuilder(new Uglify(fixturesError, { async: true }));
builder = createBuilder(new Uglify(fixturesError));

let shouldError;
await builder.build()
Expand All @@ -134,7 +113,7 @@ let { bar } = Foo.prototype;`,
});

it('shuts down the workerpool', async function() {
let testUglify = new Uglify(fixturesError, { async: true });
let testUglify = new Uglify(fixturesError);
builder = createBuilder(testUglify);

await builder.build().catch(() => {});
Expand All @@ -150,20 +129,20 @@ let { bar } = Foo.prototype;`,
});

it('defaults to CPUs-1 workers', async function() {
let testUglify = new Uglify(fixturesError, { async: true });
let testUglify = new Uglify(fixturesError);

expect(testUglify.concurrency).toEqual(require('os').cpus().length - 1);
});

it('sets concurrency using the option', async function() {
let testUglify = new Uglify(fixturesError, { async: true, concurrency: 145 });
let testUglify = new Uglify(fixturesError, { concurrency: 145 });

expect(testUglify.concurrency).toEqual(145);
});

it('overrides concurrency with JOBS env variable', async function() {
process.env.JOBS = '7';
let testUglify = new Uglify(fixturesError, { async: true, concurrency: 145 });
let testUglify = new Uglify(fixturesError, { concurrency: 145 });

expect(testUglify.concurrency).toEqual(7);
});
Expand All @@ -179,6 +158,20 @@ let { bar } = Foo.prototype;`,
});
});

describe('deprecation', function() {
it('deprecated async option', async function() {
let shouldError;
try {
builder = createBuilder(new Uglify(fixtures, { async: true }));
} catch (err) {
shouldError = err;
}

expect(shouldError.name).toEqual('Error');
expect(shouldError.message).toEqual('\n Passing `async` property inside `options` is deprecated.');
});
});

afterEach(async function() {
if (input) {
await input.dispose();
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5063,10 +5063,10 @@ tar@^4:
safe-buffer "^5.1.2"
yallist "^3.0.2"

terser@^4.8.0:
version "4.8.0"
resolved "https://registry.yarnpkg.com/terser/-/terser-4.8.0.tgz#63056343d7c70bb29f3af665865a46fe03a0df17"
integrity sha512-EAPipTNeWsb/3wLPeup1tVPaXfIaU68xMnVdPafIL1TV05OhASArYyIfFvnvJCNrR2NIOvDVNNTFRa+Re2MWyw==
terser@^5.2.1:
version "5.2.1"
resolved "https://registry.yarnpkg.com/terser/-/terser-5.2.1.tgz#40b971b8d28b4fe98c9e8c0d073ab48e7bb96cd8"
integrity sha512-/AOtjRtAMNGO0fIF6m8HfcvXTw/2AKpsOzDn36tA5RfhRdeXyb4RvHxJ5Pah7iL6dFkLk+gOnCaNHGwJPl6TrQ==
dependencies:
commander "^2.20.0"
source-map "~0.6.1"
Expand Down