diff --git a/packages/terser/src/index.js b/packages/terser/src/index.js index 48e1334328..4812031285 100644 --- a/packages/terser/src/index.js +++ b/packages/terser/src/index.js @@ -22,48 +22,48 @@ function log_error(...m) { console.error('[terser/index.js]', ...m); } -// Peek at the arguments to find any directories declared as inputs -let argv = process.argv.slice(2); -// terser_minified.bzl always passes the inputs first, -// then --output [out], then remaining args -// We want to keep those remaining ones to pass to terser -// Avoid a dependency on a library like minimist; keep it simple. -const outputArgIndex = argv.findIndex((arg) => arg.startsWith('--')); - -// We don't want to implement a command-line parser for terser -// so we invoke its CLI as child processes when a directory is provided, just altering the -// input/output arguments. See discussion: https://github.com/bazelbuild/rules_nodejs/issues/822 - -const inputs = argv.slice(0, outputArgIndex); -const output = argv[outputArgIndex + 1]; -const residual = argv.slice(outputArgIndex + 2); - -// user override for the terser binary location. used in testing. -const TERSER_BINARY = process.env.TERSER_BINARY || require.resolve('terser/bin/uglifyjs') -// choose a default concurrency of the number of cores -1 but at least 1. -const TERSER_CONCURENCY = (process.env.TERSER_CONCURRENCY || os.cpus().length - 1) || 1 - -log_verbose(`Running terser/index.js - inputs: ${inputs} - output: ${output} - residual: ${residual}`); - function isDirectory(input) { return fs.lstatSync(path.join(process.cwd(), input)).isDirectory(); } -function terserDirectory(input) { +/** + * Replaces with the outputFile name in the source-map options argument + */ +function directoryArgs(residualArgs, outputFile) { + const sourceMapIndex = residualArgs.indexOf('--source-map'); + if (sourceMapIndex === -1) { + return residualArgs; + } + + // copy args so we don't accidently mutate and process the same set of args twice + const argsCopy = [...residualArgs]; + + // the options for the source map arg is the next one + // if it changes in terser_minified.bzl this needs to be updated + const sourceMapOptsIndex = sourceMapIndex + 1 + const sourceMapOptsStr = argsCopy[sourceMapOptsIndex]; + + argsCopy[sourceMapOptsIndex] = + sourceMapOptsStr.replace('', `${path.basename(outputFile)}.map`); + + return argsCopy; +} + +function terserDirectory(input, output, residual, terserBinary) { if (!fs.existsSync(output)) { fs.mkdirSync(output); } + const TERSER_CONCURENCY = (process.env.TERSER_CONCURRENCY || os.cpus().length - 1) || 1 + let work = []; let active = 0; let errors = []; function exec([inputFile, outputFile]) { active++; - let args = [TERSER_BINARY, inputFile, '--output', outputFile, ...residual]; + let args = + [terserBinary, inputFile, '--output', outputFile, ...directoryArgs(residual, outputFile)]; spawn(process.execPath, args) .then( @@ -116,19 +116,6 @@ function terserDirectory(input) { }); } -if (!inputs.find(isDirectory) && inputs.length) { - // Inputs were only files - // Just use terser CLI exactly as it works outside bazel - require(TERSER_BINARY || 'terser/bin/uglifyjs'); - -} else if (inputs.length > 1) { - // We don't know how to merge multiple input dirs to one output dir - throw new Error('terser_minified only allows a single input when minifying a directory'); - -} else if (inputs[0]) { - terserDirectory(inputs[0]); -} - function spawn(cmd, args) { return new Promise((resolve, reject) => { const err = []; @@ -147,3 +134,50 @@ function spawn(cmd, args) { }); }) } + +function main() { + // Peek at the arguments to find any directories declared as inputs + let argv = process.argv.slice(2); + // terser_minified.bzl always passes the inputs first, + // then --output [out], then remaining args + // We want to keep those remaining ones to pass to terser + // Avoid a dependency on a library like minimist; keep it simple. + const outputArgIndex = argv.findIndex((arg) => arg.startsWith('--')); + + // We don't want to implement a command-line parser for terser + // so we invoke its CLI as child processes when a directory is provided, just altering the + // input/output arguments. See discussion: https://github.com/bazelbuild/rules_nodejs/issues/822 + + const inputs = argv.slice(0, outputArgIndex); + const output = argv[outputArgIndex + 1]; + const residual = argv.slice(outputArgIndex + 2); + + // user override for the terser binary location. used in testing. + const terserBinary = process.env.TERSER_BINARY || require.resolve('terser/bin/uglifyjs') + // choose a default concurrency of the number of cores -1 but at least 1. + + log_verbose(`Running terser/index.js + inputs: ${inputs} + output: ${output} + residual: ${residual}`); + + if (!inputs.find(isDirectory) && inputs.length) { + // Inputs were only files + // Just use terser CLI exactly as it works outside bazel + require(terserBinary || 'terser/bin/uglifyjs'); + + } else if (inputs.length > 1) { + // We don't know how to merge multiple input dirs to one output dir + throw new Error('terser_minified only allows a single input when minifying a directory'); + + } else if (inputs[0]) { + terserDirectory(inputs[0], output, residual, terserBinary); + } +} + +// export this for unit testing purposes +exports.directoryArgs = directoryArgs; + +if (require.main === module) { + main(); +} \ No newline at end of file diff --git a/packages/terser/src/terser_minified.bzl b/packages/terser/src/terser_minified.bzl index 88919cd840..7ece2bdd85 100644 --- a/packages/terser/src/terser_minified.bzl +++ b/packages/terser/src/terser_minified.bzl @@ -138,6 +138,14 @@ def _terser(ctx): else: fail("When sourcemap is True, there should only be one or none input sourcemaps") + if len(directory_srcs) == 0: + source_map_opts.append("url='%s.js.map'" % ctx.label.name) + else: + # since the input here is a dir, we don't know the names of the + # source map files yet, so we use this key to do a replacement + # when we expand the dir into specific files + source_map_opts.append("url=''") + # This option doesn't work in the config file, only on the CLI args.add_all(["--source-map", ",".join(source_map_opts)]) diff --git a/packages/terser/test/BUILD.bazel b/packages/terser/test/BUILD.bazel new file mode 100644 index 0000000000..159d07169b --- /dev/null +++ b/packages/terser/test/BUILD.bazel @@ -0,0 +1,7 @@ +load("@npm_bazel_jasmine//:index.from_src.bzl", "jasmine_node_test") + +jasmine_node_test( + name = "test", + srcs = ["directory-args.spec.js"], + deps = ["@npm_bazel_terser//:index.js"], +) diff --git a/packages/terser/test/directory-args.spec.js b/packages/terser/test/directory-args.spec.js new file mode 100644 index 0000000000..eba4ecb843 --- /dev/null +++ b/packages/terser/test/directory-args.spec.js @@ -0,0 +1,25 @@ +const {directoryArgs} = require('npm_bazel_terser/index') + +describe('directoryArgs', () => { + it('return a new array ref', () => { + const args = ['--source-map', '']; + expect(directoryArgs(args, '')).not.toBe(args); + }); + + it('should not return a new array ref with source-maps arg ', () => { + const args = []; + expect(directoryArgs(args)).toBe(args); + }); + + it('not mutate the args when theres no replacement token', () => { + const args = ['--source-map', 'url=index.js.map']; + const output = '/test/file.js'; + expect(directoryArgs(args, output)).toEqual(args); + }); + + it('should replace the with the basename', () => { + const args = ['--source-map', `url=''`]; + const output = '/test/file.js'; + expect(directoryArgs(args, output)).toEqual(['--source-map', `url='file.js.map'`]); + }); +}) \ No newline at end of file diff --git a/packages/terser/test/directory_input/BUILD.bazel b/packages/terser/test/directory_input/BUILD.bazel index 2bfffadde5..fe7c7f93e7 100644 --- a/packages/terser/test/directory_input/BUILD.bazel +++ b/packages/terser/test/directory_input/BUILD.bazel @@ -11,8 +11,6 @@ declare_directory( terser_minified( name = "out.min", src = "dir", - # TODO: support sourcemaps too - sourcemap = False, ) jasmine_node_test( diff --git a/packages/terser/test/directory_input/spec.js b/packages/terser/test/directory_input/spec.js index 5ff4934bfd..489c5f3954 100644 --- a/packages/terser/test/directory_input/spec.js +++ b/packages/terser/test/directory_input/spec.js @@ -1,4 +1,5 @@ const fs = require('fs'); +const path = require('path'); const {runfiles} = require('build_bazel_rules_nodejs/internal/linker'); describe('terser on a directory', () => { @@ -7,4 +8,17 @@ describe('terser on a directory', () => { expect(fs.existsSync(out + '/input1.js')).toBeTruthy(); expect(fs.existsSync(out + '/input2.js')).toBeTruthy(); }); + + it('should link the source to the source map', () => { + const minFile = runfiles.resolvePackageRelative('out.min') + '/input1.js'; + const expectedSourceMapUrl = runfiles.resolvePackageRelative('out.min') + '/input1.js.map'; + const content = fs.readFileSync(minFile, 'utf8'); + const sourceMapLine = content.split(/r?\n/).find(l => l.startsWith('//#')); + + expect(sourceMapLine).toBeDefined(); + + const [_, sourceMapUrl] = sourceMapLine.split('='); + + expect(sourceMapUrl).toEqual(path.basename(expectedSourceMapUrl)); + }) }); diff --git a/packages/terser/test/exec/spec.js b/packages/terser/test/exec/spec.js index cfaa372a18..5fd2f09f6d 100644 --- a/packages/terser/test/exec/spec.js +++ b/packages/terser/test/exec/spec.js @@ -1,6 +1,5 @@ const fs = require('fs'); const cp = require('child_process'); -const path = require('path'); const util = require('util'); const assert = require('assert'); diff --git a/packages/terser/test/inline_sourcemap/spec.js b/packages/terser/test/inline_sourcemap/spec.js index 1729653a44..9d8b2ec076 100644 --- a/packages/terser/test/inline_sourcemap/spec.js +++ b/packages/terser/test/inline_sourcemap/spec.js @@ -1,6 +1,8 @@ const fs = require('fs'); +const path = require('path'); const sm = require('source-map'); const {runfiles} = require('build_bazel_rules_nodejs/internal/linker'); +const os = require('os'); describe('terser sourcemap handling', () => { it('should produce a sourcemap output', async () => { @@ -15,4 +17,17 @@ describe('terser sourcemap handling', () => { expect(pos.column).toBe(14); }); }); + + it('should link the source to the source map', () => { + const minFile = runfiles.resolvePackageRelative('out.min.js'); + const expectedSourceMapUrl = runfiles.resolvePackageRelative('out.min.js.map'); + const content = fs.readFileSync(minFile, 'utf8'); + const sourceMapLine = content.split(/r?\n/).find(l => l.startsWith('//#')); + + expect(sourceMapLine).toBeDefined(); + + const [_, sourceMapUrl] = sourceMapLine.split('='); + + expect(sourceMapUrl).toEqual(path.basename(expectedSourceMapUrl)); + }) });