feat(terser): support directory inputs

packages/terser/src/BUILD.bazel

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
+load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")
 
 package(default_visibility = ["//visibility:public"])
 
@@ -38,3 +39,9 @@ filegroup(
         "terser_minified.bzl",
     ],
 )
+
+nodejs_binary(
+    name = "terser",
+    data = ["@npm//terser"],
+    entry_point = "index.js",
+)

packages/terser/src/index.from_src.bzl

@@ -22,7 +22,7 @@ load(
 
 def terser_minified(**kwargs):
     _terser_minified(
-        # Override to point to the one installed by build_bazel_rules_nodejs in the root
-        terser_bin = "@npm//terser/bin:terser",
+        # Override to point to the one we declare locally
+        terser_bin = "@npm_bazel_terser//:terser",
         **kwargs
     )

packages/terser/src/index.js

@@ -1,8 +1,76 @@
 #!/usr/bin/env node
+/**
+ * @fileoverview wraps the terser CLI to support minifying a directory
+ * Terser doesn't support it; see https://github.com/terser/terser/issues/75
+ * TODO: maybe we should generalize this to a package which would be useful outside
+ *       bazel; however we would have to support the full terser CLI and not make
+ *       assumptions about how the argv looks.
+ */
+const fs = require('fs');
+const path = require('path');
+const child_process = require('child_process');
 
-// Pass-through require, ensures that the nodejs_binary will load the version of terser
-// from @bazel/terser package.json, not some other version the user depends on.
-require('terser/bin/uglifyjs');
+// Run Bazel with --define=VERBOSE_LOGS=1 to enable this logging
+const VERBOSE_LOGS = !!process.env['VERBOSE_LOGS'];
 
-// TODO: add support for minifying multiple files (eg. a TreeArtifact) in a single execution
-// Under Node 12 it should use the worker threads API to saturate all local cores
+function log_verbose(...m) {
+  if (VERBOSE_LOGS) 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('--'));
+
+const inputs = argv.slice(0, outputArgIndex);
+const output = argv[outputArgIndex + 1];
+const residual = argv.slice(outputArgIndex + 2);
+
+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) {
+  if (!fs.existsSync(output)) {
+    fs.mkdirSync(output);
+  }
+
+  fs.readdirSync(input).forEach(f => {
+    if (f.endsWith('.js')) {
+      const inputFile = path.join(input, path.basename(f));
+      const outputFile = path.join(output, path.basename(f));
+      // We don't want to implement a command-line parser for terser
+      // so we invoke its CLI as child processes, just altering the input/output
+      // arguments. See discussion: https://github.com/bazelbuild/rules_nodejs/issues/822
+      // FIXME: this causes unlimited concurrency, which will definitely eat all the RAM on your
+      // machine;
+      //        we need to limit the concurrency with something like the p-limit package.
+      // TODO: under Node 12 it should use the worker threads API to saturate all local cores
+      child_process.fork(
+          // __dirname will be the path to npm_bazel_terser (Bazel's name for our src/ directory)
+          // and our node_modules are installed in the ../npm directory since they're external
+          // Note that the fork API doesn't do any module resolution.
+          path.join(__dirname, '../npm/node_modules/terser/bin/uglifyjs'),
+          [inputFile, '--output', outputFile, ...residual]);
+    }
+  });
+}
+
+if (!inputs.find(isDirectory)) {
+  // Inputs were only files
+  // Just use terser CLI exactly as it works outside bazel
+  require('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 {
+  terserDirectory(inputs[0]);
+}

packages/terser/src/terser_minified.bzl

@@ -14,14 +14,30 @@
 
 "Rule to run the terser binary under bazel"
 
+_DOC = """Run the terser minifier.
+
+Typical example:
+```python
+load("@npm_bazel_terser//:index.bzl", "terser_minified")
+
+terser_minified(
+    name = "out.min",
+    src = "input.js",
+    config_file = "terser_config.json",
+)
+```
+
+Note that the `name` attribute determines what the resulting files will be called.
+So the example above will output `out.min.js` and `out.min.js.map` (since `sourcemap` defaults to `true`).
+"""
+
 _TERSER_ATTRS = {
     "src": attr.label(
-        doc = """A JS file, or a rule producing .js as its default output
+        doc = """A JS file, or a rule producing .js files as its default output
 
 Note that you can pass multiple files to terser, which it will bundle together.
 If you want to do this, you can pass a filegroup here.""",
         allow_files = [".js"],
-        mandatory = True,
     ),
     "config_file": attr.label(
         doc = """A JSON file containing Terser minify() options.
@@ -67,6 +83,14 @@ so that it only affects the current build.
         doc = "Whether to produce a .js.map output",
         default = True,
     ),
+    "src_dir": attr.label(
+        doc = """A directory containing some .js files.
+
+Each `.js` file will be run through terser, and the rule will output a directory of minified files.
+The output will be a directory named the same as the "name" attribute.
+Any files not ending in `.js` will be ignored.
+""",
+    ),
     "terser_bin": attr.label(
         doc = "An executable target that runs Terser",
         default = Label("@npm//@bazel/terser/bin:terser"),
@@ -75,7 +99,10 @@ so that it only affects the current build.
     ),
 }
 
-def _terser_outs(sourcemap):
+def _terser_outs(src_dir, sourcemap):
+    if src_dir:
+        # Tree artifact outputs must be declared with ctx.actions.declare_directory
+        return {}
     result = {"minified": "%{name}.js"}
     if sourcemap:
         result["sourcemap"] = "%{name}.js.map"
@@ -86,10 +113,28 @@ def _terser(ctx):
 
     # CLI arguments; see https://www.npmjs.com/package/terser#command-line-usage
     args = ctx.actions.args()
-    args.add_all([src.path for src in ctx.files.src])
-
-    outputs = [ctx.outputs.minified]
-    args.add_all(["--output", ctx.outputs.minified.path])
+    inputs = []
+    outputs = [getattr(ctx.outputs, o) for o in dir(ctx.outputs)]
+
+    if ctx.attr.src and ctx.attr.src_dir:
+        fail("Only one of src and src_dir attributes should be specified")
+    if not ctx.attr.src and not ctx.attr.src_dir:
+        fail("Either src or src_dir is required")
+    if ctx.attr.src:
+        for src in ctx.files.src:
+            if src.is_directory:
+                fail("Directories should be specified in the src_dir attribute, not src")
+            args.add(src.path)
+        inputs.extend(ctx.files.src)
+    else:
+        for src in ctx.files.src_dir:
+            if not src.is_directory:
+                fail("Individual files should be specifed in the src attribute, not src_dir")
+            args.add(src.path)
+        inputs.extend(ctx.files.src_dir)
+        outputs.append(ctx.actions.declare_directory(ctx.label.name))
+
+    args.add_all(["--output", outputs[0].path])
 
     debug = ctx.attr.debug or "DEBUG" in ctx.var.keys()
     if debug:
@@ -111,6 +156,7 @@ def _terser(ctx):
         args.add_all(["--source-map", ",".join(source_map_opts)])
 
     opts = ctx.actions.declare_file("_%s.minify_options.json" % ctx.label.name)
+    inputs.append(opts)
     ctx.actions.expand_template(
         template = ctx.file.config_file,
         output = opts,
@@ -123,30 +169,19 @@ def _terser(ctx):
     args.add_all(["--config-file", opts.path])
 
     ctx.actions.run(
-        inputs = ctx.files.src + [opts],
+        inputs = inputs,
         outputs = outputs,
         executable = ctx.executable.terser_bin,
         arguments = [args],
-        progress_message = "Minifying JavaScript %s [terser]" % (ctx.outputs.minified.short_path),
+        progress_message = "Minifying JavaScript %s [terser]" % (outputs[0].short_path),
     )
 
-terser_minified = rule(
-    doc = """Run the terser minifier.
-    
-Typical example:
-```python
-load("@npm_bazel_terser//:index.bzl", "terser_minified")
+    return [
+        DefaultInfo(files = depset(outputs)),
+    ]
 
-terser_minified(
-    name = "out.min",
-    src = "input.js",
-    config_file = "terser_config.json",
-)
-```
-
-Note that the `name` attribute determines what the resulting files will be called.
-So the example above will output `out.min.js` and `out.min.js.map` (since `sourcemap` defaults to `true`).
-""",
+terser_minified = rule(
+    doc = _DOC,
     implementation = _terser,
     attrs = _TERSER_ATTRS,
     outputs = _terser_outs,

packages/terser/test/directory_input/BUILD.bazel

@@ -0,0 +1,22 @@
+load("@npm_bazel_jasmine//:index.from_src.bzl", "jasmine_node_test")
+load("@npm_bazel_terser//:index.from_src.bzl", "terser_minified")
+load(":rule.bzl", "declare_directory")
+
+# Check that filegroups work
+declare_directory(
+    name = "dir",
+    srcs = glob(["input*.js"]),
+)
+
+terser_minified(
+    name = "out.min",
+    # TODO: support sourcemaps too
+    sourcemap = False,
+    src_dir = "dir",
+)
+
+jasmine_node_test(
+    name = "test",
+    srcs = ["spec.js"],
+    data = [":out.min"],
+)

packages/terser/test/directory_input/input1.js

@@ -0,0 +1,4 @@
+function a() {
+  var somelongname = 'a';
+  console.log(somelongname);
+}

packages/terser/test/directory_input/input2.js

@@ -0,0 +1,4 @@
+function b() {
+  var someotherlongname = 'b';
+  console.log(someotherlongname);
+}

packages/terser/test/directory_input/rule.bzl

@@ -0,0 +1,16 @@
+"Minimal test fixture to create a directory output"
+
+def _impl(ctx):
+    dir = ctx.actions.declare_directory(ctx.label.name)
+    ctx.actions.run_shell(
+        inputs = ctx.files.srcs,
+        outputs = [dir],
+        # RBE requires that we mkdir, but outside RBE it might already exist
+        command = "mkdir -p {0}; cp $@ {0}".format(dir.path),
+        arguments = [s.path for s in ctx.files.srcs],
+    )
+    return [
+        DefaultInfo(files = depset([dir])),
+    ]
+
+declare_directory = rule(_impl, attrs = {"srcs": attr.label_list(allow_files = True)})

packages/terser/test/directory_input/spec.js

@@ -0,0 +1,8 @@
+const fs = require('fs');
+
+describe('terser on a directory', () => {
+  it('should produce an output for each input', () => {
+    expect(fs.existsSync(require.resolve(__dirname + '/out.min/input1.js'))).toBeTruthy();
+    expect(fs.existsSync(require.resolve(__dirname + '/out.min/input2.js'))).toBeTruthy();
+  });
+});