feat(builtin): npm_package_bin can produce directory output (#1164)

internal/node/npm_package_bin.bzl

@@ -8,26 +8,44 @@ _ATTRS = {
     "outs": attr.output_list(),
     "args": attr.string_list(mandatory = True),
     "data": attr.label_list(allow_files = True, aspects = [module_mappings_aspect]),
+    "out_dir": attr.string(),
     "tool": attr.label(
         executable = True,
         cfg = "host",
         mandatory = True,
     ),
 }
 
+# Need a custom expand_location function
+# because the out_dir is a tree artifact
+# so we weren't able to give it a label
+def _expand_location(ctx, s):
+    s = s.replace("$@", "/".join([ctx.bin_dir.path, ctx.label.package, ctx.attr.out_dir]))
+    return ctx.expand_location(s, targets = ctx.attr.data)
+
 def _impl(ctx):
+    if ctx.attr.out_dir and ctx.attr.outs:
+        fail("Only one of out_dir and outs may be specified")
+    if not ctx.attr.out_dir and not ctx.attr.outs:
+        fail("One of out_dir and outs must be specified")
+
     args = ctx.actions.args()
     inputs = ctx.files.data[:]
-    outputs = ctx.outputs.outs
+    outputs = []
+    if ctx.attr.out_dir:
+        outputs = [ctx.actions.declare_directory(ctx.attr.out_dir)]
+    else:
+        outputs = ctx.outputs.outs
     register_node_modules_linker(ctx, args, inputs)
     for a in ctx.attr.args:
-        args.add(ctx.expand_location(a, targets = ctx.attr.data))
+        args.add(_expand_location(ctx, a))
     ctx.actions.run(
         executable = ctx.executable.tool,
         inputs = inputs,
         outputs = outputs,
         arguments = [args],
     )
+    return [DefaultInfo(files = depset(outputs))]
 
 _npm_package_bin = rule(
     _impl,
@@ -45,17 +63,22 @@ def npm_package_bin(tool = None, package = None, package_bin = None, **kwargs):
     https://docs.bazel.build/versions/master/skylark/macros.html#full-example
 
     Args:
-        data: identical to [genrule.srcs](https://docs.bazel.build/versions/master/be/general.html#genrule.srcs)
+        data: similar to [genrule.srcs](https://docs.bazel.build/versions/master/be/general.html#genrule.srcs)
               may also include targets that produce or reference npm packages which are needed by the tool
-        outs: identical to [genrule.outs](https://docs.bazel.build/versions/master/be/general.html#genrule.outs)
+        outs: similar to [genrule.outs](https://docs.bazel.build/versions/master/be/general.html#genrule.outs)
+        out_dir: use this instead of `outs` if you want the output to be a directory
+                 Exactly one of `outs`, `out_dir` may be used.
+                 If you output a directory, there can only be one output.
         args: Command-line arguments to the tool.
 
             Subject to 'Make variable' substitution.
             Can use $(location) expansion. See https://docs.bazel.build/versions/master/be/make-variables.html
+            You may also refer to the location of the out_dir with the special `$@` replacement, like genrule.
 
         package: an npm package whose binary to run, like "terser". Assumes your node_modules are installed in a workspace called "npm"
         package_bin: the "bin" entry from `package` that should be run. By default package_bin is the same string as `package`
-        tool: a label for a binary to run, like `@npm//terser/bin:terser`. This is the longer form of package/package_bin
+        tool: a label for a binary to run, like `@npm//terser/bin:terser`. This is the longer form of package/package_bin.
+              Note that you can also refer to a binary in your local workspace.
     """
     if not tool:
         if not package:

internal/node/test/BUILD.bazel

@@ -158,8 +158,28 @@ npm_package_bin(
     package = "terser",
 )
 
+nodejs_binary(
+    name = "copy_to_directory",
+    entry_point = "copy_to_directory.js",
+)
+
+npm_package_bin(
+    name = "produces_directory",
+    args = [
+        "--output-dir",
+        "$@",
+        "$(location terser_input.js)",
+    ],
+    data = ["terser_input.js"],
+    out_dir = "dir_output",
+    tool = ":copy_to_directory",
+)
+
 nodejs_test(
     name = "npm_package_bin_test",
-    data = ["minified.js"],
+    data = [
+        "minified.js",
+        "produces_directory",
+    ],
     entry_point = "npm_package_bin.spec.js",
 )

internal/node/test/copy_to_directory.js

@@ -0,0 +1,14 @@
+const fs = require('fs');
+const path = require('path');
+
+// argv[2] is always --output-dir
+const out_dir = process.argv[3];
+
+try {
+  fs.mkdirSync(out_dir);
+} catch {
+}
+
+for (const input of process.argv.slice(4)) {
+  fs.copyFileSync(input, path.join(out_dir, path.basename(input)));
+}

internal/node/test/npm_package_bin.spec.js

@@ -1,8 +1,13 @@
 const fs = require('fs');
 const path = require('path');
-
-const content = fs.readFileSync(path.join(require.resolve(__dirname + '/minified.js')), 'utf-8');
+const min_js = path.join(require.resolve(__dirname + '/minified.js'));
+const content = fs.readFileSync(min_js, 'utf-8');
 if (!content.includes('{console.error("thing")}')) {
   console.error(content);
   process.exitCode = 1;
 }
+
+const dir = fs.readdirSync(path.join(path.dirname(min_js), 'dir_output'));
+if (!dir.includes('terser_input.js')) {
+  console.error(dir), process.exitCode = 1;
+}

internal/npm_install/generate_build_file.js

@@ -1063,7 +1063,7 @@ function printIndexBzl(pkg) {
 
 # Generated helper macro to call ${name}
 def ${name.replace(/-/g, '_')}(**kwargs):
-    if "outs" in kwargs:
+    if "outs" in kwargs or "out_dir" in kwargs:
         npm_package_bin(tool = "@${WORKSPACE}//${pkg._dir}/bin:${name}", **kwargs)
     else:
         nodejs_binary(

internal/npm_install/test/golden/@gregmagolan/test-a/index.bzl.golden

@@ -1,6 +1,6 @@
 load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary", "npm_package_bin")
 def test(**kwargs):
-    if "outs" in kwargs:
+    if "outs" in kwargs or "out_dir" in kwargs:
         npm_package_bin(tool = "@fine_grained_goldens//@gregmagolan/test-a/bin:test", **kwargs)
     else:
         nodejs_binary(

internal/npm_install/test/golden/jasmine/index.bzl.golden

@@ -1,6 +1,6 @@
 load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary", "npm_package_bin")
 def jasmine(**kwargs):
-    if "outs" in kwargs:
+    if "outs" in kwargs or "out_dir" in kwargs:
         npm_package_bin(tool = "@fine_grained_goldens//jasmine/bin:jasmine", **kwargs)
     else:
         nodejs_binary(