Performs a no-op bundle if no client-side JavaScript is provided.

Previously the build would fail if `includeScript` was never called and `bundle_js = True`. This is because Rollup _requires_ at least one input to bundle or it will fail. In this case, we don't actually care and doing nothing is the correct solution which will inject no scripts into the page. There's a bit of wasted build-time effort, but it's not that big a deal. Unfortunately there's no way to check if any files are in a `TreeArtifact` at analysis-time and no way to configure Rollup to allow the "no inputs" case. Instead we wrap the Rollup binary with another tool, called `js_bundle`. This is simply a pass through to Rollup, however it adds one `if` statement to check for the "no inputs" case. If so, then it exits successfully as a no-op. Bazel creates the output directory automatically, so there's nothing we need to do. It bugs me a little to have to introduce another Node process for this one `if` statement. I did try to see if I could run Rollup in-process, however this seemed to require manually parsing the command line arguments and passing them through to Rollup in a structured manner, which I'd rather not deal with. If we find the extra process overhead is significant, we can maybe revisit that optimization.
dgp1130 · Jul 24, 2023 · 18a8811 · 18a8811
1 parent b2baca6
commit 18a8811
Show file tree

Hide file tree

Showing 9 changed files with 165 additions and 15 deletions.
diff --git a/examples/no_scripts/BUILD.bazel b/examples/no_scripts/BUILD.bazel
@@ -0,0 +1,41 @@
+load("//:index.bzl", "prerender_pages", "web_resources_devserver")
+load("//tools/jasmine:defs.bzl", "jasmine_web_test_suite")
+load("//tools/typescript:defs.bzl", "ts_project")
+
+prerender_pages(
+    name = "site",
+    entry_point = "./site.js",
+    prerender = ":prerender",
+)
+
+ts_project(
+    name = "prerender",
+    srcs = ["site.tsx"],
+    deps = [
+        "//:node_modules/@rules_prerender/preact",
+        "//:node_modules/preact",
+    ],
+)
+
+web_resources_devserver(
+    name = "devserver",
+    resources = ":site",
+)
+
+ts_project(
+    name = "test_lib",
+    srcs = ["test.mts"],
+    testonly = True,
+    deps = [
+        "//common/testing:devserver",
+        "//common/testing:webdriver",
+        "//:node_modules/@types/jasmine",
+    ],
+)
+
+jasmine_web_test_suite(
+    name = "test",
+    browsers = ["//tools/browsers:chromium-local"],
+    data = [":devserver"],
+    deps = [":test_lib"],
+)
diff --git a/examples/no_scripts/README.md b/examples/no_scripts/README.md
@@ -0,0 +1,5 @@
+# No scripts
+
+Test case which does _not_ include any client-side JavaScript. This should not
+emit any errors or warnings during the build process. It also should _not_
+inject any `<script>` tag into the page, as this would be wasted bytes.
diff --git a/examples/no_scripts/site.tsx b/examples/no_scripts/site.tsx
@@ -0,0 +1,15 @@
+import { PrerenderResource, renderToHtml } from '@rules_prerender/preact';
+
+export default function*(): Generator<PrerenderResource, void, void> {
+    yield PrerenderResource.fromHtml('/index.html', renderToHtml(
+        <html>
+            <head>
+                <meta charSet="utf8" />
+                <title>No scripts</title>
+            </head>
+            <body>
+                <h2>No scripts</h2>
+            </body>
+        </html>
+    ));
+}
diff --git a/examples/no_scripts/test.mts b/examples/no_scripts/test.mts
@@ -0,0 +1,20 @@
+import { useDevserver } from '../../common/testing/devserver.mjs';
+import { useWebDriver } from '../../common/testing/webdriver.mjs';
+
+describe('no_scripts', () => {
+    const devserver = useDevserver('examples/no_scripts/devserver.sh');
+    const wd = useWebDriver(devserver);
+
+    describe('index page', () => {
+        it('renders without a script', async () => {
+            const browser = wd.get();
+            await browser.url('/');
+
+            expect(await browser.$('h2').getText()).toBe('No scripts');
+
+            // Expect only the live reload script.
+            const scripts = await browser.$$('script');
+            expect(scripts.length).toBe(1);
+        });
+    });
+});
diff --git a/packages/rules_prerender/BUILD.bazel b/packages/rules_prerender/BUILD.bazel
@@ -143,8 +143,8 @@ bzl_library(
     deps = [
         ":multi_inject_resources",
         ":prerender_pages_unbundled",
-        ":scripts_bundle",
         ":web_resources",
+        "//tools/binaries/js_bundler:js_bundle",
     ],
 )
 
@@ -160,11 +160,6 @@ bzl_library(
     ],
 )
 
-bzl_library(
-    name = "scripts_bundle",
-    srcs = ["scripts_bundle.bzl"],
-)
-
 bzl_library(
     name = "visibility_aspect",
     srcs = ["visibility_aspect.bzl"],

diff --git a/packages/rules_prerender/prerender_pages.bzl b/packages/rules_prerender/prerender_pages.bzl
@@ -1,8 +1,8 @@
 """Defines `prerender_pages()` functionality."""
 
+load("//tools/binaries/js_bundler:js_bundle.bzl", "js_bundle")
 load(":multi_inject_resources.bzl", "multi_inject_resources")
 load(":prerender_pages_unbundled.bzl", "prerender_pages_unbundled")
-load(":scripts_bundle.bzl", "scripts_bundle")
 load(":web_resources.bzl", "web_resources")
 
 visibility("public")
@@ -103,7 +103,7 @@ def prerender_pages(
     bundle = "%s_bundle" % name
     if bundle_js:
         # Bundle all client-side scripts in a `TreeArtifact` at `%{name}_bundle`.
-        scripts_bundle(
+        js_bundle(
             name = bundle,
             entry_points = ":%s_scripts_entries" % prerender_name,
             testonly = testonly,

diff --git a/tools/binaries/js_bundler/BUILD.bazel b/tools/binaries/js_bundler/BUILD.bazel
@@ -0,0 +1,23 @@
+load("@aspect_rules_js//js:defs.bzl", "js_binary")
+load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
+load("//tools/typescript:defs.bzl", "ts_project")
+
+bzl_library(
+    name = "js_bundle",
+    srcs = ["js_bundle.bzl"],
+    visibility = ["//packages/rules_prerender:__pkg__"],
+)
+
+js_binary(
+    name = "js_bundler",
+    entry_point = "js_bundler.mjs",
+    data = ["js_bundler_lib"],
+    visibility = ["//visibility:public"],
+)
+
+ts_project(
+    name = "js_bundler_lib",
+    srcs = ["js_bundler.mts"],
+    data = ["//packages/rules_prerender:rollup"],
+    deps = ["//common:binary"],
+)
diff --git a/packages/rules_prerender/scripts_bundle.bzl → tools/binaries/js_bundler/js_bundle.bzl b/packages/rules_prerender/scripts_bundle.bzl → tools/binaries/js_bundler/js_bundle.bzl
@@ -7,9 +7,9 @@ load("@aspect_bazel_lib//lib:paths.bzl", "to_output_relative_path")
 load("@aspect_rules_js//js:providers.bzl", "JsInfo")
 load("//common:paths.bzl", "is_js_file")
 
-visibility("private")
+visibility(["//packages/rules_prerender"])
 
-def _scripts_bundle_impl(ctx):
+def _js_bundle_impl(ctx):
     # Extract the config file.
     config_sources = [src
                       for src in ctx.attr._config[JsInfo].sources.to_list()
@@ -66,7 +66,7 @@ def _scripts_bundle_impl(ctx):
     ctx.actions.run(
         mnemonic = "RollupBundle",
         progress_message = "Bundling JavaScript %{label}",
-        executable = ctx.executable._rollup,
+        executable = ctx.executable._js_bundler,
         arguments = [args],
         inputs = sources.to_list() + node_modules.to_list() + [
             ctx.file.entry_points,
@@ -77,8 +77,8 @@ def _scripts_bundle_impl(ctx):
 
     return DefaultInfo(files = depset([output]))
 
-scripts_bundle = rule(
-    implementation = _scripts_bundle_impl,
+js_bundle = rule(
+    implementation = _js_bundle_impl,
     attrs = {
         "entry_points": attr.label(
             mandatory = True,
@@ -95,8 +95,8 @@ scripts_bundle = rule(
             providers = [JsInfo],
             doc = "Dependencies used by the bundled entry points.",
         ),
-        "_rollup": attr.label(
-            default = "//packages/rules_prerender:rollup",
+        "_js_bundler": attr.label(
+            default = "//tools/binaries/js_bundler",
             executable = True,
             cfg = "exec",
         ),

diff --git a/tools/binaries/js_bundler/js_bundler.mts b/tools/binaries/js_bundler/js_bundler.mts
@@ -0,0 +1,51 @@
+/**
+ * @fileoverview Bundles the given JavaScript entry points into a directory of
+ * bundled output files.
+ * 
+ * The command line arguments for this binary are passed through to Rollup
+ * without modification and use the contract. The only difference that in Rollup
+ * passing no input files is an error, but in `js_bundler` it is a no-op
+ * success.
+ */
+
+import { execFile as execFileCb } from 'child_process';
+import { promisify } from 'util';
+import { main } from '../../../common/binary.mjs';
+
+const execFile = promisify(execFileCb);
+
+const rollup =
+    `${process.env['RUNFILES']}/rules_prerender/packages/rules_prerender/rollup.sh`;
+
+main(async () => {
+    // No files to bundle. This would be an error in Rollup, but we don't care
+    // and just skip running Rollup altogether. Bazel creates the output
+    // directory automatically, so we don't need to do anything, just exit
+    // successfully.
+    if (!process.argv.includes('-i')) return 0;
+
+    // Drop Node parts of the argument string.
+    const rollupArgs = process.argv.slice(2);
+
+    // Execute Rollup as a subprocess.
+    const proc = execFile(rollup, rollupArgs, {});
+
+    // Pipe subprocess stdout/stderr to this process' stdout/stderr.
+    proc.child.stdout?.on('data', (chunk) => {
+        process.stdout.write(chunk);
+    });
+    proc.child.stderr?.on('data', (chunk) => {
+        process.stderr.write(chunk);
+    });
+
+    // Wait for Rollup to complete.
+    try {
+        await proc;
+    } catch {
+        // No need to propagate error message, stderr is already piped to the
+        // process output.
+        return 1;
+    }
+
+    return 0;
+});