feat(terser): add source map links

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 <OUTPUT_MAP_FILE> 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('<OUTPUT_MAP_FILE>', `${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();
+}

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='<OUTPUT_MAP_FILE>'")
+
         # This option doesn't work in the config file, only on the CLI
         args.add_all(["--source-map", ",".join(source_map_opts)])
 

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"],
+)

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 <OUTPUT_MAP_FILE> with the basename', () => {
+    const args = ['--source-map', `url='<OUTPUT_MAP_FILE>'`];
+    const output = '/test/file.js';
+    expect(directoryArgs(args, output)).toEqual(['--source-map', `url='file.js.map'`]);
+  });
+})

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(

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));
+  })
 });

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');
 

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));
+  })
 });