refactor(rollup): simplify by removing the .from_src.bzl

Also enable the new rollup worker mode for e2e and packages/rollup tests. This is a first step towards making it the default in rules_nodejs 2.0
jbedard · Mar 27, 2020 · 362ba69 · 362ba69
1 parent 7cc42d6
commit 362ba69
Show file tree

Hide file tree

Showing 19 changed files with 32 additions and 48 deletions.
diff --git a/e2e/webapp/BUILD.bazel b/e2e/webapp/BUILD.bazel
@@ -7,6 +7,8 @@ rollup_bundle(
     srcs = ["strings.js"],
     entry_point = "app.js",
     output_dir = True,
+    # Enable experimental faster builds with Bazel workers
+    supports_workers = True,
 )
 
 terser_minified(

diff --git a/internal/pkg_npm/test/BUILD.bazel b/internal/pkg_npm/test/BUILD.bazel
@@ -1,6 +1,6 @@
 load("@build_bazel_rules_nodejs//:index.bzl", "pkg_npm")
 load("@npm_bazel_jasmine//:index.from_src.bzl", "jasmine_node_test")
-load("@npm_bazel_rollup//:index.from_src.bzl", "rollup_bundle")
+load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
 load("@npm_bazel_typescript//:index.from_src.bzl", "ts_library")
 load("//internal/node:context.bzl", "node_context_data")
 load("//third_party/github.com/bazelbuild/bazel-skylib:rules/write_file.bzl", "write_file")

diff --git a/internal/pkg_web/test/BUILD.bazel b/internal/pkg_web/test/BUILD.bazel
@@ -1,7 +1,7 @@
 load("@build_bazel_rules_nodejs//:index.bzl", "pkg_web")
 load("@npm//@babel/cli:index.bzl", "babel")
 load("@npm_bazel_jasmine//:index.from_src.bzl", "jasmine_node_test")
-load("@npm_bazel_rollup//:index.from_src.bzl", "rollup_bundle")
+load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
 load("@npm_bazel_terser//:index.from_src.bzl", "terser_minified")
 load("@npm_bazel_typescript//:index.from_src.bzl", "ts_library")
 

diff --git a/internal/pkg_web/test2/BUILD.bazel b/internal/pkg_web/test2/BUILD.bazel
@@ -1,7 +1,7 @@
 load("@build_bazel_rules_nodejs//:index.bzl", "pkg_web")
 load("@npm//@babel/cli:index.bzl", "babel")
 load("@npm_bazel_jasmine//:index.from_src.bzl", "jasmine_node_test")
-load("@npm_bazel_rollup//:index.from_src.bzl", "rollup_bundle")
+load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
 load("@npm_bazel_terser//:index.from_src.bzl", "terser_minified")
 
 rollup_bundle(

diff --git a/packages/labs/test/grpc_web/BUILD.bazel b/packages/labs/test/grpc_web/BUILD.bazel
@@ -1,6 +1,6 @@
 load("@npm_bazel_jasmine//:index.from_src.bzl", "jasmine_node_test")
 load("@npm_bazel_karma//:index.from_src.bzl", "karma_web_test_suite")
-load("@npm_bazel_rollup//:index.from_src.bzl", "rollup_bundle")
+load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
 load("@npm_bazel_typescript//:index.from_src.bzl", "ts_library")
 
 # This test checks that the protos can be resolved in a nodejs environment

diff --git a/packages/node-patches/BUILD.bazel b/packages/node-patches/BUILD.bazel
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test")
-load("@npm_bazel_rollup//:index.from_src.bzl", "rollup_bundle")
+load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
 load("@npm_node_patches//typescript:index.bzl", "tsc")
 
 package(default_visibility = ["//:__subpackages__"])

diff --git a/packages/protractor/test/protractor-2/BUILD.bazel b/packages/protractor/test/protractor-2/BUILD.bazel
@@ -1,6 +1,6 @@
 load("@npm//http-server:index.bzl", "http_server")
 load("@npm_bazel_protractor//:index.from_src.bzl", "protractor_web_test_suite")
-load("@npm_bazel_rollup//:index.from_src.bzl", "rollup_bundle")
+load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
 load("@npm_bazel_terser//:index.from_src.bzl", "terser_minified")
 load("@npm_bazel_typescript//:index.bzl", "ts_config")
 load("@npm_bazel_typescript//:index.from_src.bzl", "ts_devserver", "ts_library")

diff --git a/packages/rollup/BUILD.bazel b/packages/rollup/BUILD.bazel
@@ -59,6 +59,9 @@ pkg_npm(
         "@npm_bazel_rollup//:package_contents",
     ],
     build_file_content = """exports_files(["rollup.config.js"])""",
+    substitutions = {
+        "//:rollup-worker-local": "@npm//@bazel/rollup/bin:rollup-worker",
+    },
     vendor_external = [
         "npm_bazel_rollup",
     ],

diff --git a/packages/rollup/src/BUILD.bazel b/packages/rollup/src/BUILD.bazel
@@ -51,7 +51,7 @@ filegroup(
 )
 
 nodejs_binary(
-    name = "rollup-worker",
+    name = "rollup-worker-local",
     data = [
         # NOTE: we ought to link to the original location instead of copying
         # "@build_bazel_rules_nodejs//packages/worker:npm_package",

diff --git a/packages/rollup/src/index.from_src.bzl b/packages/rollup/src/index.from_src.bzl
diff --git a/packages/rollup/src/index.js b/packages/rollup/src/index.js
@@ -22,7 +22,7 @@ const PID = process.pid;
 
 let worker;
 try {
-  worker = require('@bazel/worker');
+  worker = require('./worker');
 } catch {
   // TODO: rely on the linker to link the first-party package
   const helper = process.env['BAZEL_NODE_RUNFILES_HELPER'];

diff --git a/packages/rollup/src/package.json b/packages/rollup/src/package.json
@@ -25,8 +25,5 @@
         "npm_bazel_rollup": {
             "rootPath": "."
         }
-    },
-    "dependencies": {
-        "@bazel/worker": "0.0.0-PLACEHOLDER"
     }
 }
diff --git a/packages/rollup/src/rollup_bundle.bzl b/packages/rollup/src/rollup_bundle.bzl
@@ -182,7 +182,8 @@ Otherwise, the outputs are assumed to be a single file.
         doc = "Internal use only",
         executable = True,
         cfg = "host",
-        default = "@npm//@bazel/rollup:rollup-worker",
+        # NB: will be substituted with "@npm//@bazel/rollup/bin:rollup-worker" when the pkg_npm target is built
+        default = "//:rollup-worker-local",
     ),
     "silent": attr.bool(
         doc = """Whether to execute the rollup binary with the --silent flag, defaults to False.
@@ -201,11 +202,11 @@ Passed to the [`--sourcemap` option](https://github.com/rollup/rollup/blob/maste
         values = ["inline", "hidden", "true", "false"],
     ),
     "supports_workers": attr.bool(
-        doc = """Intended for internal use only.
+        doc = """Experimental! Use only with caution.
 
-Allows you to disable the Bazel Worker strategy for this library.
-Typically used together with the "rollup_bin" setting when using a
-non-worker aware binary.""",
+Allows you to enable the Bazel Worker strategy for this library.
+When enabled, this rule invokes the "rollup_worker_bin"
+worker aware binary rather than "rollup_bin".""",
         default = False,
     ),
     "deps": attr.label_list(
@@ -383,7 +384,7 @@ def _rollup_bundle(ctx):
 
     if ctx.attr.supports_workers:
         executable = "rollup_worker_bin"
-        execution_requirements = {"supports-workers": str(int(ctx.attr.supports_workers))}
+        execution_requirements["supports-workers"] = str(int(ctx.attr.supports_workers))
 
     run_node(
         ctx,

diff --git a/packages/rollup/test/code_splitting/BUILD.bazel b/packages/rollup/test/code_splitting/BUILD.bazel
@@ -1,5 +1,5 @@
 load("@npm_bazel_jasmine//:index.from_src.bzl", "jasmine_node_test")
-load("@npm_bazel_rollup//:index.from_src.bzl", "rollup_bundle")
+load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
 
 rollup_bundle(
     name = "bundle",
@@ -10,6 +10,7 @@ rollup_bundle(
     # [!] 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 = True,
+    supports_workers = True,
 )
 
 jasmine_node_test(

diff --git a/packages/rollup/test/integration/BUILD.bazel b/packages/rollup/test/integration/BUILD.bazel
@@ -1,6 +1,6 @@
 load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "npm_package_bin")
 load("@build_bazel_rules_nodejs//internal/golden_file_test:golden_file_test.bzl", "golden_file_test")
-load("@npm_bazel_rollup//:index.from_src.bzl", "rollup_bundle")
+load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
 
 _BUNDLE_FORMATS = [
     "amd",
@@ -23,6 +23,7 @@ _BUNDLE_FORMATS = [
         entry_point = "foo.js",
         format = format,
         sourcemap = "true",
+        supports_workers = True,
         deps = [
             "//%s/fum:fumlib" % package_name(),
             "//%s/foo:foo_lib" % package_name(),

diff --git a/packages/rollup/test/multiple_entry_points/BUILD.bazel b/packages/rollup/test/multiple_entry_points/BUILD.bazel
@@ -1,5 +1,5 @@
 load("@npm_bazel_jasmine//:index.from_src.bzl", "jasmine_node_test")
-load("@npm_bazel_rollup//:index.from_src.bzl", "rollup_bundle")
+load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
 
 rollup_bundle(
     name = "chunks",
@@ -8,6 +8,7 @@ rollup_bundle(
         "entry2.js": "two",
     },
     output_dir = True,
+    supports_workers = True,
 )
 
 jasmine_node_test(

diff --git a/packages/rollup/test/plugins/BUILD.bazel b/packages/rollup/test/plugins/BUILD.bazel
@@ -1,12 +1,13 @@
 load("@build_bazel_rules_nodejs//internal/golden_file_test:golden_file_test.bzl", "golden_file_test")
-load("@npm_bazel_rollup//:index.from_src.bzl", "rollup_bundle")
+load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
 
 rollup_bundle(
     name = "plugins",
     srcs = ["some.json"],
     config_file = "rollup.config.js",
     entry_point = "input.js",
     sourcemap = "false",
+    supports_workers = True,
     deps = ["@npm//rollup-plugin-json"],
 )
 

diff --git a/packages/rollup/test/sourcemaps/BUILD.bazel b/packages/rollup/test/sourcemaps/BUILD.bazel
@@ -1,5 +1,5 @@
 load("@npm_bazel_jasmine//:index.from_src.bzl", "jasmine_node_test")
-load("@npm_bazel_rollup//:index.from_src.bzl", "rollup_bundle")
+load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
 
 rollup_bundle(
     name = "bundle",
@@ -12,6 +12,7 @@ rollup_bundle(
     # Use the desugared form to assert that it works
     entry_points = {"input.js": "bundle"},
     sourcemap = "true",
+    supports_workers = True,
 )
 
 jasmine_node_test(

diff --git a/packages/rollup/test/version_stamp/BUILD.bazel b/packages/rollup/test/version_stamp/BUILD.bazel
@@ -1,5 +1,5 @@
 load("@build_bazel_rules_nodejs//internal/golden_file_test:golden_file_test.bzl", "golden_file_test")
-load("@npm_bazel_rollup//:index.from_src.bzl", "rollup_bundle")
+load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
 load("//internal/node:context.bzl", "node_context_data")
 
 # Force stamping behavior even in builds without --stamp config
@@ -15,6 +15,7 @@ rollup_bundle(
     entry_point = "input.js",
     node_context_data = ":force_stamp",
     sourcemap = "false",
+    supports_workers = True,
 )
 
 golden_file_test(