From c8a3c28f3bcf9d127b4731d914295565ba054ae7 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Mon, 4 Oct 2021 13:09:11 -0700 Subject: [PATCH] fix(esbuild): allow whitespace within args Fixes #2997 --- docs/esbuild.md | 8 +-- packages/esbuild/esbuild.bzl | 37 +++++------ packages/esbuild/test/banner/BUILD.bazel | 66 +++++++++++++++++++ .../esbuild/test/banner/banner.golden.txt | 6 ++ .../test/banner/banner.newline.golden.txt | 7 ++ .../test/banner/banner.quotes.golden.txt | 6 ++ .../test/banner/banner.spaces.golden.txt | 6 ++ packages/esbuild/test/banner/main.js | 1 + 8 files changed, 113 insertions(+), 24 deletions(-) create mode 100644 packages/esbuild/test/banner/BUILD.bazel create mode 100644 packages/esbuild/test/banner/banner.golden.txt create mode 100644 packages/esbuild/test/banner/banner.newline.golden.txt create mode 100644 packages/esbuild/test/banner/banner.quotes.golden.txt create mode 100644 packages/esbuild/test/banner/banner.spaces.golden.txt create mode 100644 packages/esbuild/test/banner/main.js diff --git a/docs/esbuild.md b/docs/esbuild.md index 208c69900e..865bc395cf 100755 --- a/docs/esbuild.md +++ b/docs/esbuild.md @@ -98,7 +98,7 @@ This will create an output directory containing all the code split chunks, along **USAGE** 
-esbuild(name, args, args_file, config, define, deps, entry_point, entry_points, external, format,
+esbuild(name, args, args_json, config, define, deps, entry_point, entry_points, external, format,
         launcher, link_workspace_root, max_threads, minify, node_context_data, output, output_css,
         output_dir, output_map, platform, sourcemap, sources_content, splitting, srcs, target)
@@ -123,11 +123,11 @@ Values are subject to $(location ...) expansion Defaults to `{}` -

args_file

+

args_json

-(*Label*): Internal use only +(*String*): Internal use only -Defaults to `None` +Defaults to `""`

config

diff --git a/packages/esbuild/esbuild.bzl b/packages/esbuild/esbuild.bzl index 56090a8d21..28966250ce 100644 --- a/packages/esbuild/esbuild.bzl +++ b/packages/esbuild/esbuild.bzl @@ -2,7 +2,7 @@ esbuild rule """ -load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "params_file") +load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary") load("@build_bazel_rules_nodejs//:providers.bzl", "ExternalNpmPackageInfo", "JSEcmaScriptModuleInfo", "JSModuleInfo", "NODE_CONTEXT_ATTRS", "NodeContextInfo", "node_modules_aspect", "run_node") load("@build_bazel_rules_nodejs//internal/linker:link_node_modules.bzl", "LinkerPackageMappingInfo", "module_mappings_aspect") load("@build_bazel_rules_nodejs//internal/common:expand_variables.bzl", "expand_variables") @@ -151,9 +151,14 @@ def _esbuild_impl(ctx): launcher_args.add("--metafile=%s" % meta_file.path) # add reference to the users args file, these are merged within the launcher - if ctx.attr.args_file: - inputs.append(ctx.file.args_file) - launcher_args.add("--user_args=%s" % ctx.file.args_file.path) + if ctx.attr.args_json: + user_args_file = ctx.actions.declare_file("%s.user.args.json" % ctx.attr.name) + inputs.append(user_args_file) + ctx.actions.write( + output = user_args_file, + content = ctx.expand_location(ctx.attr.args_json), + ) + launcher_args.add("--user_args=%s" % user_args_file.path) if ctx.attr.config: configs = ctx.attr.config[JSEcmaScriptModuleInfo].sources.to_list() @@ -204,8 +209,7 @@ esbuild = rule( doc = """A dict of extra arguments that are included in the call to esbuild, where the key is the argument name. Values are subject to $(location ...) expansion""", ), - "args_file": attr.label( - allow_single_file = True, + "args_json": attr.string( mandatory = False, doc = "Internal use only", ), @@ -393,24 +397,17 @@ def esbuild_macro(name, output_dir = False, splitting = False, **kwargs): deps = kwargs.pop("deps", []) + ["@esbuild_npm//esbuild"] entry_points = kwargs.get("entry_points", None) - # TODO(mattem): remove `args` and `args_file` in 5.x and everything can go via `config` - args_file = kwargs.pop("args_file", None) - if args_file: - fail("Setting 'args_file' is not supported, set 'config' instead") + # TODO(mattem): remove `args` and `args_json` in 5.x and everything can go via `config` + args_json = kwargs.pop("args_json", None) + if args_json: + fail("Setting 'args_json' is not supported, set 'config' instead") args = kwargs.pop("args", {}) if args: if type(args) != type(dict()): fail("Expected 'args' to be of type dict") - args_file = "%s.user.args.json" % name - params_file( - name = "%s_args" % name, - testonly = kwargs.get("testonly", False), - out = args_file, - args = [json.encode(args)], - data = deps + srcs, - ) + args_json = json.encode(args) config = kwargs.pop("config", None) if config: @@ -423,7 +420,7 @@ def esbuild_macro(name, output_dir = False, splitting = False, **kwargs): srcs = srcs, splitting = splitting, output_dir = True, - args_file = args_file, + args_json = args_json, launcher = _launcher, deps = deps, **kwargs @@ -441,7 +438,7 @@ def esbuild_macro(name, output_dir = False, splitting = False, **kwargs): esbuild( name = name, srcs = srcs, - args_file = args_file, + args_json = args_json, output = output, output_map = output_map, launcher = _launcher, diff --git a/packages/esbuild/test/banner/BUILD.bazel b/packages/esbuild/test/banner/BUILD.bazel new file mode 100644 index 0000000000..44156a54f1 --- /dev/null +++ b/packages/esbuild/test/banner/BUILD.bazel @@ -0,0 +1,66 @@ +load("//:index.bzl", "generated_file_test") +load("//packages/esbuild:index.bzl", "esbuild") + +esbuild( + name = "banner", + args = { + "banner": { + "js": "//hello", + }, + }, + entry_point = "main.js", +) + +esbuild( + name = "banner_newline", + args = { + "banner": { + "js": "//header\n//comments", + }, + }, + entry_point = "main.js", +) + +esbuild( + name = "banner_quotes", + args = { + "banner": { + "js": "\"use strict\"", + }, + }, + entry_point = "main.js", +) + +esbuild( + name = "banner_spaces", + args = { + "banner": { + "js": "#!/usr/bin/env node", + }, + }, + entry_point = "main.js", +) + +generated_file_test( + name = "banner_test", + src = "banner.golden.txt", + generated = "banner.js", +) + +generated_file_test( + name = "banner_newline_test", + src = "banner.newline.golden.txt", + generated = "banner_newline.js", +) + +generated_file_test( + name = "banner_quotes_test", + src = "banner.quotes.golden.txt", + generated = "banner_quotes.js", +) + +generated_file_test( + name = "banner_spaces_test", + src = "banner.spaces.golden.txt", + generated = "banner_spaces.js", +) diff --git a/packages/esbuild/test/banner/banner.golden.txt b/packages/esbuild/test/banner/banner.golden.txt new file mode 100644 index 0000000000..2df842efa4 --- /dev/null +++ b/packages/esbuild/test/banner/banner.golden.txt @@ -0,0 +1,6 @@ +//hello +(() => { + // packages/esbuild/test/banner/main.js + console.log("a script"); +})(); +//# sourceMappingURL=banner.js.map diff --git a/packages/esbuild/test/banner/banner.newline.golden.txt b/packages/esbuild/test/banner/banner.newline.golden.txt new file mode 100644 index 0000000000..6bb02183bc --- /dev/null +++ b/packages/esbuild/test/banner/banner.newline.golden.txt @@ -0,0 +1,7 @@ +//header +//comments +(() => { + // packages/esbuild/test/banner/main.js + console.log("a script"); +})(); +//# sourceMappingURL=banner_newline.js.map diff --git a/packages/esbuild/test/banner/banner.quotes.golden.txt b/packages/esbuild/test/banner/banner.quotes.golden.txt new file mode 100644 index 0000000000..b4290b52c0 --- /dev/null +++ b/packages/esbuild/test/banner/banner.quotes.golden.txt @@ -0,0 +1,6 @@ +"use strict" +(() => { + // packages/esbuild/test/banner/main.js + console.log("a script"); +})(); +//# sourceMappingURL=banner_quotes.js.map diff --git a/packages/esbuild/test/banner/banner.spaces.golden.txt b/packages/esbuild/test/banner/banner.spaces.golden.txt new file mode 100644 index 0000000000..c4b4a2de8d --- /dev/null +++ b/packages/esbuild/test/banner/banner.spaces.golden.txt @@ -0,0 +1,6 @@ +#!/usr/bin/env node +(() => { + // packages/esbuild/test/banner/main.js + console.log("a script"); +})(); +//# sourceMappingURL=banner_spaces.js.map diff --git a/packages/esbuild/test/banner/main.js b/packages/esbuild/test/banner/main.js new file mode 100644 index 0000000000..0e273ec7f6 --- /dev/null +++ b/packages/esbuild/test/banner/main.js @@ -0,0 +1 @@ +console.log("a script"); \ No newline at end of file