refactor: simplify API a bit

Based on discussion with @fenghaolw

e2e/webapp/BUILD.bazel

@@ -6,12 +6,12 @@ rollup_bundle(
     name = "app",
     srcs = ["strings.js"],
     entry_point = "app.js",
-    output_dir = "bundle",
+    output_dir = True,
 )
 
 terser_minified(
     name = "out.min",
-    src_dir = "app",
+    src = "app",
 )
 
 nodejs_test(

packages/rollup/src/rollup_bundle.bzl

@@ -132,10 +132,11 @@ Passed to the [`--globals` option](https://github.com/rollup/rollup/blob/master/
 Also, the keys from the map are passed to the [`--external` option](https://github.com/rollup/rollup/blob/master/docs/999-big-list-of-options.md#external).
 """,
     ),
-    "output_dir": attr.string(
-        doc = """A directory in which generated chunks are placed.
+    "output_dir": attr.bool(
+        doc = """Whether to produce a directory output.
 
-Passed to the [`--output.dir` option](https://github.com/rollup/rollup/blob/master/docs/999-big-list-of-options.md#outputdir) in rollup.
+We will use the [`--output.dir` option](https://github.com/rollup/rollup/blob/master/docs/999-big-list-of-options.md#outputdir) in rollup
+rather than `--output.file`.
 
 If the program produces multiple chunks, you must specify this attribute.
 Otherwise, the outputs are assumed to be a single file.
@@ -240,7 +241,7 @@ def _rollup_bundle(ctx):
 
     # If user requests an output_dir, then use output.dir rather than output.file
     if ctx.attr.output_dir:
-        outputs.append(ctx.actions.declare_directory(ctx.attr.output_dir))
+        outputs.append(ctx.actions.declare_directory(ctx.label.name))
         for entry_point in entry_points:
             args.add_joined([entry_point[1], _no_ext(entry_point[0])], join_with = "=")
         args.add_all(["--output.dir", outputs[0].path])

packages/rollup/test/code_splitting/BUILD.bazel

@@ -9,7 +9,7 @@ rollup_bundle(
     # TODO: if this isn't here, the error is
     # [!] Error: You must set output.dir instead of output.file when generating multiple chunks.
     # which will confuse users because it references output.dir instead of output_dir
-    output_dir = "chunks",
+    output_dir = True,
 )
 
 jasmine_node_test(

packages/rollup/test/code_splitting/spec.js

@@ -2,7 +2,7 @@ const fs = require('fs');
 
 describe('rollup code splitting', () => {
   it('should produce a chunk for lazy loaded code', () => {
-    expect(fs.existsSync(require.resolve(__dirname + '/chunks/bundle.js'))).toBeTruthy();
-    expect(fs.existsSync(require.resolve(__dirname + '/chunks/chunk.js'))).toBeTruthy();
+    expect(fs.existsSync(require.resolve(__dirname + '/bundle/bundle.js'))).toBeTruthy();
+    expect(fs.existsSync(require.resolve(__dirname + '/bundle/chunk.js'))).toBeTruthy();
   });
 });

packages/rollup/test/multiple_entry_points/BUILD.bazel

@@ -2,17 +2,17 @@ load("@npm_bazel_jasmine//:index.from_src.bzl", "jasmine_node_test")
 load("@npm_bazel_rollup//:index.from_src.bzl", "rollup_bundle")
 
 rollup_bundle(
-    name = "bundle",
+    name = "chunks",
     entry_points = {
         "entry1.js": "one",
         "entry2.js": "two",
     },
-    output_dir = "chunks",
+    output_dir = True,
 )
 
 jasmine_node_test(
     name = "test",
     srcs = ["spec.js"],
     data = ["@npm//source-map"],
-    deps = [":bundle"],
+    deps = [":chunks"],
 )

packages/terser/src/terser_minified.bzl

@@ -29,15 +29,19 @@ terser_minified(
 
 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`).
+If the input is a directory, then the output will also be a directory, named after the `name` attribute.
 """
 
 _TERSER_ATTRS = {
     "src": attr.label(
-        doc = """A JS file, or a rule producing .js files as its default output
+        doc = """File(s) to minify.
+        
+Can be a .js file, a rule producing .js files as its default output, or a rule producing a directory of .js files.
 
 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.
@@ -83,14 +87,6 @@ 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"),
@@ -99,41 +95,25 @@ Any files not ending in `.js` will be ignored.
     ),
 }
 
-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"
-    return result
-
 def _terser(ctx):
     "Generate actions to create terser config run terser"
 
     # CLI arguments; see https://www.npmjs.com/package/terser#command-line-usage
     args = ctx.actions.args()
-    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)
+    inputs = ctx.files.src[:]
+    outputs = []
+
+    directory_srcs = [s for s in ctx.files.src if s.is_directory]
+    if len(directory_srcs) > 0:
+        if len(ctx.files.src) > 1:
+            fail("When directories are passed to terser_minified, there should be only one input")
         outputs.append(ctx.actions.declare_directory(ctx.label.name))
+    else:
+        outputs.append(ctx.actions.declare_file("%s.js" % ctx.label.name))
+        if ctx.attr.sourcemap:
+            outputs.append(ctx.actions.declare_file("%s.js.map" % ctx.label.name))
 
+    args.add_all([s.path for s in ctx.files.src])
     args.add_all(["--output", outputs[0].path])
 
     debug = ctx.attr.debug or "DEBUG" in ctx.var.keys()
@@ -182,5 +162,4 @@ terser_minified = rule(
     doc = _DOC,
     implementation = _terser,
     attrs = _TERSER_ATTRS,
-    outputs = _terser_outs,
 )

packages/terser/test/debug/BUILD.bazel

@@ -11,22 +11,21 @@ terser_minified(
 
 golden_file_test(
     name = "test",
-    # Assert that the default output is a .js file
-    # by omitting the ".js" extension here
     actual = "out.min",
     golden = "output.golden.js_",
 )
 
 terser_minified(
     name = "debug_from_env",
     src = "input.js",
+    sourcemap = False,
     # Don't specify debug = True
     # Instead we'll run the test with --define=DEBUG=1
 )
 
 golden_file_test(
     name = "test_define_DEBUG",
-    actual = "debug_from_env.js",
+    actual = "debug_from_env",
     golden = "output.golden.js_",
     tags = ["manual"],  # Only passes when --define=DEBUG=1 is set
 )

packages/terser/test/directory_input/BUILD.bazel

@@ -10,9 +10,9 @@ declare_directory(
 
 terser_minified(
     name = "out.min",
+    src = "dir",
     # TODO: support sourcemaps too
     sourcemap = False,
-    src_dir = "dir",
 )
 
 jasmine_node_test(

packages/terser/test/user_config/BUILD.bazel

@@ -5,11 +5,12 @@ terser_minified(
     name = "out.min",
     src = "input.js",
     config_file = "terser_config.json",
+    sourcemap = False,
 )
 
 golden_file_test(
     name = "test",
-    actual = "out.min.js",
+    actual = "out.min",
     golden = "output.golden.js_",
     golden_debug = "output.debug.golden.js_",
 )