From 6b2b019342a5989a8c03389ea7372ce7606f0b69 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Mon, 29 Apr 2019 15:26:40 -0700 Subject: [PATCH] fix(web_package): normalize root paths consistently with ts_devserver Fixes #728 --- internal/web_package/test-exports/BUILD.bazel | 31 +++++++++++++ internal/web_package/test-exports/file.css | 0 internal/web_package/test-exports/file.js | 0 internal/web_package/test/BUILD.bazel | 2 + internal/web_package/test/spec.js | 2 +- internal/web_package/test2/BUILD.bazel | 46 +++++++++++++++++++ internal/web_package/test2/index.html | 1 + internal/web_package/test2/index_golden.html_ | 2 + .../web_package/test2/rel-exports/BUILD.bazel | 31 +++++++++++++ .../test2/rel-exports/rel-file.css | 0 .../web_package/test2/rel-exports/rel-file.js | 0 internal/web_package/test2/script.js | 3 ++ internal/web_package/test2/spec.js | 15 ++++++ internal/web_package/web_package.bzl | 16 +++++-- .../internal/devserver/ts_devserver.bzl | 7 +-- 15 files changed, 147 insertions(+), 9 deletions(-) create mode 100644 internal/web_package/test-exports/BUILD.bazel create mode 100644 internal/web_package/test-exports/file.css create mode 100644 internal/web_package/test-exports/file.js create mode 100644 internal/web_package/test2/BUILD.bazel create mode 100644 internal/web_package/test2/index.html create mode 100644 internal/web_package/test2/index_golden.html_ create mode 100644 internal/web_package/test2/rel-exports/BUILD.bazel create mode 100644 internal/web_package/test2/rel-exports/rel-file.css create mode 100644 internal/web_package/test2/rel-exports/rel-file.js create mode 100644 internal/web_package/test2/script.js create mode 100644 internal/web_package/test2/spec.js diff --git a/internal/web_package/test-exports/BUILD.bazel b/internal/web_package/test-exports/BUILD.bazel new file mode 100644 index 0000000000..89ec41712a --- /dev/null +++ b/internal/web_package/test-exports/BUILD.bazel @@ -0,0 +1,31 @@ +package(default_visibility = ["//visibility:public"]) + +EXTS = [ + "js", + "css", +] + +[ + genrule( + name = "bin-" + e, + outs = ["bin." + e], + cmd = "echo '' > $@", + output_to_bindir = True, + ) + for e in EXTS +] + +[ + genrule( + name = "gen-" + e, + outs = ["gen." + e], + cmd = "echo '' > $@", + output_to_bindir = False, + ) + for e in EXTS +] + +exports_files([ + "file.css", + "file.js", +]) diff --git a/internal/web_package/test-exports/file.css b/internal/web_package/test-exports/file.css new file mode 100644 index 0000000000..e69de29bb2 diff --git a/internal/web_package/test-exports/file.js b/internal/web_package/test-exports/file.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/internal/web_package/test/BUILD.bazel b/internal/web_package/test/BUILD.bazel index 595359c0c6..0de61a34e2 100644 --- a/internal/web_package/test/BUILD.bazel +++ b/internal/web_package/test/BUILD.bazel @@ -1,6 +1,8 @@ load("@build_bazel_rules_nodejs//:defs.bzl", "jasmine_node_test", "rollup_bundle") load("@build_bazel_rules_nodejs//internal/web_package:web_package.bzl", "web_package") +package(default_visibility = ["//visibility:public"]) + rollup_bundle( name = "bundle", srcs = glob(["*.js"]), diff --git a/internal/web_package/test/spec.js b/internal/web_package/test/spec.js index 1a53406711..fc115a3a4a 100644 --- a/internal/web_package/test/spec.js +++ b/internal/web_package/test/spec.js @@ -10,6 +10,6 @@ describe('web_package', () => { const actual = fs.readFileSync(require.resolve(output), {encoding: 'utf-8'}); const expected = fs.readFileSync(require.resolve(golden), {encoding: 'utf-8'}); // make the input hermetic by replacing the cache-buster timestamp - expect(actual.replace(/\?v=\d+/, '?v=123')).toBe(expected); + expect(actual.replace(/\?v=\d+/g, '?v=123')).toBe(expected); }); }); diff --git a/internal/web_package/test2/BUILD.bazel b/internal/web_package/test2/BUILD.bazel new file mode 100644 index 0000000000..b07cda25ed --- /dev/null +++ b/internal/web_package/test2/BUILD.bazel @@ -0,0 +1,46 @@ +load("@build_bazel_rules_nodejs//:defs.bzl", "jasmine_node_test", "rollup_bundle") +load("@build_bazel_rules_nodejs//internal/web_package:web_package.bzl", "web_package") + +rollup_bundle( + name = "local_bundle", + srcs = glob(["*.js"]), + entry_point = "internal/web_package/test2/script.js", +) + +# Same exts as //internal/web_package/test-exports, //internal/web_package/test2/rel-exports +EXTS = [ + "js", + "css", +] + +web_package( + name = "pkg", + assets = [ + # a bundle from a different directory + "//internal/web_package/test:bundle", + # a bundle in the current path + ":local_bundle", + ] + + # bin + gen + exported files from a different directory + ["//internal/web_package/test-exports:bin-" + e for e in EXTS] + + ["//internal/web_package/test-exports:gen-" + e for e in EXTS] + + ["//internal/web_package/test-exports:file." + e for e in EXTS] + + # bin + gen + exported files from a sub directory + ["//internal/web_package/test2/rel-exports:rel-bin-" + e for e in EXTS] + + ["//internal/web_package/test2/rel-exports:rel-gen-" + e for e in EXTS] + + ["//internal/web_package/test2/rel-exports:rel-file." + e for e in EXTS], + index_html = "index.html", +) + +jasmine_node_test( + name = "test", + srcs = ["spec.js"], + data = [ + "index_golden.html_", + ":pkg", + ], + node_modules = "@build_bazel_rules_nodejs_web_package_deps//:node_modules", + tags = [ + "fix-windows", + ], +) diff --git a/internal/web_package/test2/index.html b/internal/web_package/test2/index.html new file mode 100644 index 0000000000..18ecdcb795 --- /dev/null +++ b/internal/web_package/test2/index.html @@ -0,0 +1 @@ + diff --git a/internal/web_package/test2/index_golden.html_ b/internal/web_package/test2/index_golden.html_ new file mode 100644 index 0000000000..dc9093f5c9 --- /dev/null +++ b/internal/web_package/test2/index_golden.html_ @@ -0,0 +1,2 @@ + + \ No newline at end of file diff --git a/internal/web_package/test2/rel-exports/BUILD.bazel b/internal/web_package/test2/rel-exports/BUILD.bazel new file mode 100644 index 0000000000..dc82bd2ce6 --- /dev/null +++ b/internal/web_package/test2/rel-exports/BUILD.bazel @@ -0,0 +1,31 @@ +package(default_visibility = ["//visibility:public"]) + +EXTS = [ + "js", + "css", +] + +[ + genrule( + name = "rel-bin-" + e, + outs = ["rel-bin." + e], + cmd = "echo '' > $@", + output_to_bindir = True, + ) + for e in EXTS +] + +[ + genrule( + name = "rel-gen-" + e, + outs = ["rel-gen." + e], + cmd = "echo '' > $@", + output_to_bindir = False, + ) + for e in EXTS +] + +exports_files([ + "rel-file.css", + "rel-file.js", +]) diff --git a/internal/web_package/test2/rel-exports/rel-file.css b/internal/web_package/test2/rel-exports/rel-file.css new file mode 100644 index 0000000000..e69de29bb2 diff --git a/internal/web_package/test2/rel-exports/rel-file.js b/internal/web_package/test2/rel-exports/rel-file.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/internal/web_package/test2/script.js b/internal/web_package/test2/script.js new file mode 100644 index 0000000000..e2881b8f1a --- /dev/null +++ b/internal/web_package/test2/script.js @@ -0,0 +1,3 @@ +const el = document.createElement('div'); +el.innerText = 'Hello, World'; +document.body.appendChild(el); diff --git a/internal/web_package/test2/spec.js b/internal/web_package/test2/spec.js new file mode 100644 index 0000000000..2f6b2bbda4 --- /dev/null +++ b/internal/web_package/test2/spec.js @@ -0,0 +1,15 @@ +const fs = require('fs'); +const path = require('path'); + +process.chdir(path.join(process.env['TEST_SRCDIR'], 'build_bazel_rules_nodejs')); +console.error(fs.readdirSync('.')); +describe('web_package paths', () => { + it('should match the golden file', () => { + const output = 'build_bazel_rules_nodejs/internal/web_package/test2/pkg/index.html'; + const golden = 'build_bazel_rules_nodejs/internal/web_package/test2/index_golden.html_'; + const actual = fs.readFileSync(require.resolve(output), {encoding: 'utf-8'}); + const expected = fs.readFileSync(require.resolve(golden), {encoding: 'utf-8'}); + // make the input hermetic by replacing the cache-buster timestamp + expect(actual.replace(/\?v=\d+/g, '?v=123')).toBe(expected); + }); +}); diff --git a/internal/web_package/web_package.bzl b/internal/web_package/web_package.bzl index 4fa2dc718b..599ef01b3a 100644 --- a/internal/web_package/web_package.bzl +++ b/internal/web_package/web_package.bzl @@ -73,18 +73,28 @@ def move_files(output_name, files, action_factory, assembler, root_paths): ) return depset([www_dir]) -def _web_package(ctx): - root_paths = ctx.attr.additional_root_paths + [ +def additional_root_paths(ctx): + return ctx.attr.additional_root_paths + [ + # package path is the root, including in bin/gen ctx.label.package, "/".join([ctx.bin_dir.path, ctx.label.package]), "/".join([ctx.genfiles_dir.path, ctx.label.package]), + + # bazel-bin/gen dirs to absolute paths + ctx.genfiles_dir.path, + ctx.bin_dir.path, + + # package re-rooted subdirectory + "/".join([p for p in [ctx.bin_dir.path, ctx.label.package, "_" + ctx.label.name, ctx.label.package] if p]), ] +def _web_package(ctx): + root_paths = additional_root_paths(ctx) + # Create the output file in a re-rooted subdirectory so it doesn't collide with the input file html = ctx.actions.declare_file("_%s/%s" % (ctx.label.name, ctx.file.index_html.path)) # Move that index file back into place inside the package - root_paths.append("/".join([p for p in [ctx.bin_dir.path, ctx.label.package, "_" + ctx.label.name, ctx.label.package] if p])) populated_index = html_asset_inject( ctx.file.index_html, ctx.actions, diff --git a/packages/typescript/internal/devserver/ts_devserver.bzl b/packages/typescript/internal/devserver/ts_devserver.bzl index 58fdb0ea0e..a2d4f77fb4 100644 --- a/packages/typescript/internal/devserver/ts_devserver.bzl +++ b/packages/typescript/internal/devserver/ts_devserver.bzl @@ -21,6 +21,7 @@ load( ) load( "@build_bazel_rules_nodejs//internal/web_package:web_package.bzl", + "additional_root_paths", "html_asset_inject", ) @@ -99,11 +100,7 @@ def _ts_devserver(ctx): ctx.file.index_html, ctx.actions, ctx.executable._injector, - ctx.attr.additional_root_paths + [ - ctx.label.package, - "/".join([ctx.bin_dir.path, ctx.label.package]), - "/".join([ctx.genfiles_dir.path, ctx.label.package]), - ], + additional_root_paths(ctx), [_short_path_to_manifest_path(ctx, f.short_path) for f in ctx.files.static_files] + [bundle_script], injected_index, )