Skip to content

Commit a1962af

Browse files
committed
Removes bundle_js flag from prerender_pages.
This flag served only as a minor optimization for skipping the bundling phase when no client-side JavaScript was expected. It was required before because Rollup would throw a hard error if no entry points were given to be bundled. Now the `js_bundle` rule passes when given no inputs and generates an empty output directory as would be expected. As a result, `bundle_js` is no longer required to avoid errors and the optimization benefits are so trivial as to not be worth the trade off. If a user forgets scripts are disabled for a page and then adds scripts to a component, they may be very confused to find those scripts are not loaded. Requiring a bundle step for client-side JavaScript means this mistake can never be made.
1 parent 3ccdabe commit a1962af

3 files changed

Lines changed: 14 additions & 18 deletions

File tree

packages/rules_prerender/multi_inject_resources.bzl

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ def _multi_inject_resources_impl(ctx):
2020
args = ctx.actions.args()
2121
args.add("--input-dir", ctx.file.input_dir.short_path)
2222
args.add("--config", config.short_path)
23-
if ctx.attr.bundles:
24-
args.add("--bundles", ctx.file.bundles.short_path)
23+
args.add("--bundles", ctx.file.bundles.short_path)
2524
args.add("--output-dir", output_dir.short_path)
2625
ctx.actions.run(
2726
mnemonic = "MultiInjectResources",
@@ -49,7 +48,10 @@ multi_inject_resources = rule(
4948
mandatory = True,
5049
allow_single_file = True,
5150
),
52-
"bundles": attr.label(allow_single_file = True),
51+
"bundles": attr.label(
52+
mandatory = True,
53+
allow_single_file = True,
54+
),
5355
"scripts": attr.string_list(),
5456
"styles": attr.label(),
5557
"_injector": attr.label(

packages/rules_prerender/prerender_pages.bzl

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ def prerender_pages(
1414
scripts = None,
1515
styles = None,
1616
resources = None,
17-
bundle_js = True,
1817
testonly = None,
1918
visibility = None,
2019
debug_target = None,
@@ -76,8 +75,6 @@ def prerender_pages(
7675
scripts: Passed through to `prerender_pages_unbundled()`.
7776
styles: Passed through to `prerender_pages_unbundled()`.
7877
resources: Passed through to `prerender_pages_unbundled()`.
79-
bundle_js: Whether or not to bundle and inject JavaScript files.
80-
Defaults to `True`.
8178
testonly: See https://docs.bazel.build/versions/master/be/common-definitions.html.
8279
visibility: See https://docs.bazel.build/versions/master/be/common-definitions.html.
8380
debug_target: The label to check
@@ -100,22 +97,21 @@ def prerender_pages(
10097
debug_target = debug_target or "//%s:%s" % (native.package_name(), name),
10198
)
10299

100+
# Bundle all client-side scripts in a `TreeArtifact` at `%{name}_bundle`.
103101
bundle = "%s_bundle" % name
104-
if bundle_js:
105-
# Bundle all client-side scripts in a `TreeArtifact` at `%{name}_bundle`.
106-
js_bundle(
107-
name = bundle,
108-
entry_points = ":%s_scripts_entries" % prerender_name,
109-
testonly = testonly,
110-
deps = [":%s_scripts" % prerender_name],
111-
)
102+
js_bundle(
103+
name = bundle,
104+
entry_points = ":%s_scripts_entries" % prerender_name,
105+
testonly = testonly,
106+
deps = [":%s_scripts" % prerender_name],
107+
)
112108

113109
# Inject bundled JS and CSS into the HTML.
114110
injected_dir = "%s_injected" % name
115111
multi_inject_resources(
116112
name = injected_dir,
117113
input_dir = ":%s" % prerender_name,
118-
bundles = ":%s" % bundle if bundle_js else None,
114+
bundles = ":%s" % bundle,
119115
styles = ":%s_styles" % prerender_name,
120116
testonly = testonly,
121117
)

packages/rules_prerender/rollup.config.mts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ export default {
1212
if (warning.code === 'EMPTY_BUNDLE') {
1313
// Give a better suggestion when no JavaScript is generated.
1414
console.warn('Generated an empty JavaScript bundle, do you have'
15-
+ ' any JavaScript? If not, consider setting'
16-
+ ' `bundle_js = False` on your `prerender_pages()` to skip'
17-
+ ' this step.\n\n' + warning.message);
15+
+ ' any JavaScript?\n\n' + warning.message);
1816
return;
1917
}
2018

0 commit comments

Comments
 (0)