diff --git a/docs/js_run_devserver.md b/docs/js_run_devserver.md index 97ec5d531..5d5954fa3 100755 --- a/docs/js_run_devserver.md +++ b/docs/js_run_devserver.md @@ -77,11 +77,14 @@ The devserver specified by either `tool` or `command` is run in a custom sandbox compatible with devserver watch modes in Node.js tools such as Webpack and Next.js. The custom sandbox is populated with the default outputs of all targets in `data` -as well as transitive npm links. - -rules_js npm package link targets such as `//:node_modules/next` are handled efficiently. -Since these targets are symlinks in the output tree, they are recreated as symlinks -in the custom sandbox and do not incur a fully copy of the underlying npm packages. +as well as transitive sources & npm links. + +An an optimization, virtual store files are explicitly excluded from the sandbox since the npm +links will point to the virtual store in the execroot and Node.js will follow those links as it +does within the execroot. As a result, rules_js npm package link targets such as +`//:node_modules/next` are handled efficiently. Since these targets are symlinks in the output +tree, they are recreated as symlinks in the custom sandbox and do not incur a fully copy of the +underlying npm packages. Supports running with [ibazel](https://github.com/bazelbuild/bazel-watcher). Only `data` files that change on incremental builds are synchronized when running with ibazel. diff --git a/e2e/js_run_devserver/multirun_test.sh b/e2e/js_run_devserver/multirun_test.sh index ab5947a0b..386669ca0 100755 --- a/e2e/js_run_devserver/multirun_test.sh +++ b/e2e/js_run_devserver/multirun_test.sh @@ -93,7 +93,7 @@ if ! curl http://localhost:8081/index.html --fail 2>/dev/null | grep "A second l fi echo "
A new file
" > src/new.html -_sedi 's#"other.html",#"other.html", "new.html",#' src/BUILD.bazel +_sedi 's#"other.html"#"other.html", "new.html"#' src/BUILD.bazel echo "Waiting 10 seconds for ibazel rebuild after change to src/BUILD.bazel..." sleep 10 diff --git a/e2e/js_run_devserver/serve_test.sh b/e2e/js_run_devserver/serve_test.sh index ecd7db528..236e105af 100755 --- a/e2e/js_run_devserver/serve_test.sh +++ b/e2e/js_run_devserver/serve_test.sh @@ -63,7 +63,7 @@ if ! curl http://localhost:8080/index.html --fail 2>/dev/null | grep "A second l fi echo "
A new file
" > src/new.html -_sedi 's#"other.html",#"other.html", "new.html",#' src/BUILD.bazel +_sedi 's#"other.html"#"other.html", "new.html"#' src/BUILD.bazel echo "Waiting 10 seconds for ibazel rebuild after change to src/BUILD.bazel..." sleep 10 diff --git a/e2e/js_run_devserver/src/BUILD.bazel b/e2e/js_run_devserver/src/BUILD.bazel index 1ae3282a9..92bdec734 100644 --- a/e2e/js_run_devserver/src/BUILD.bazel +++ b/e2e/js_run_devserver/src/BUILD.bazel @@ -7,15 +7,25 @@ http_server_bin.http_server_binary( name = "http_server", ) +# A trivial graph of sources to include in js_run_devserver data to test +# that transitive sources are include +js_library( + name = "transitive_web_files", + srcs = ["other.html"], +) + +js_library( + name = "web_files", + srcs = ["index.html"], + deps = [":transitive_web_files"], +) + # Simple js_run_devserver rule that syncs two html files and takes an http-server js_binary # as the tool js_run_devserver( name = "devserver", chdir = package_name(), - data = [ - "index.html", - "other.html", - ], + data = [":web_files"], log_level = "debug", tool = ":http_server", ) @@ -47,6 +57,7 @@ js_run_devserver( ":http_root", "//:node_modules/http-server", ], + log_level = "debug", ) # Use command to bake-in the arguments into the runnable binary so that the target @@ -87,8 +98,8 @@ js_run_devserver( chdir = package_name(), command = "./simple.js", data = [ - "index.html", - "other.html", ":simple", + ":web_files", ], + log_level = "debug", ) diff --git a/js/private/BUILD.bazel b/js/private/BUILD.bazel index 86276955c..d118467ff 100644 --- a/js/private/BUILD.bazel +++ b/js/private/BUILD.bazel @@ -127,7 +127,6 @@ bzl_library( deps = [ ":js_binary", ":js_binary_helpers", - ":js_library_helpers", "@bazel_skylib//lib:dicts", ], ) diff --git a/js/private/js_run_devserver.bzl b/js/private/js_run_devserver.bzl index 61df862b8..d93590537 100644 --- a/js/private/js_run_devserver.bzl +++ b/js/private/js_run_devserver.bzl @@ -2,7 +2,6 @@ load(":js_binary.bzl", "js_binary_lib") load(":js_binary_helpers.bzl", _gather_files_from_js_providers = "gather_files_from_js_providers") -load(":js_library_helpers.bzl", _gather_npm_linked_packages = "gather_npm_linked_packages") load("@bazel_skylib//lib:dicts.bzl", "dicts") _DOC = """Runs a devserver via binary target or command. @@ -69,11 +68,14 @@ The devserver specified by either `tool` or `command` is run in a custom sandbox compatible with devserver watch modes in Node.js tools such as Webpack and Next.js. The custom sandbox is populated with the default outputs of all targets in `data` -as well as transitive npm links. +as well as transitive sources & npm links. -rules_js npm package link targets such as `//:node_modules/next` are handled efficiently. -Since these targets are symlinks in the output tree, they are recreated as symlinks -in the custom sandbox and do not incur a fully copy of the underlying npm packages. +An an optimization, virtual store files are explicitly excluded from the sandbox since the npm +links will point to the virtual store in the execroot and Node.js will follow those links as it +does within the execroot. As a result, rules_js npm package link targets such as +`//:node_modules/next` are handled efficiently. Since these targets are symlinks in the output +tree, they are recreated as symlinks in the custom sandbox and do not incur a fully copy of the +underlying npm packages. Supports running with [ibazel](https://github.com/bazelbuild/bazel-watcher). Only `data` files that change on incremental builds are synchronized when running with ibazel. @@ -116,18 +118,24 @@ def _impl(ctx): if ctx.attr.tool and ctx.attr.command: fail("Only one of tool or command may be specified") + transitive_runfiles = [_gather_files_from_js_providers( + targets = ctx.attr.data, + include_transitive_sources = ctx.attr.include_transitive_sources, + include_declarations = ctx.attr.include_declarations, + include_npm_linked_packages = ctx.attr.include_npm_linked_packages, + )] + # The .to_list() calls here are intentional and cannot be avoided; they should be small sets of # files as they only include direct npm links (node_modules/foo) and the virtual store tree # artifacts those symlinks point to (node_modules/.aspect_rules_js/foo@1.2.3/node_modules/foo) - transitive_npm_links_files = [] - transitive_npm_links = depset(transitive = [p.files for p in _gather_npm_linked_packages(srcs = [], deps = ctx.attr.data).transitive.to_list()]) - for f in transitive_npm_links.to_list(): + data_files = [] + for f in depset(transitive = transitive_runfiles + [dep.files for dep in ctx.attr.data]).to_list(): # don't include the virtual store tree artifact; only the node_module link is needed if not "/.aspect_rules_js/" in f.path: - transitive_npm_links_files.append(f) + data_files.append(f) config = { - "data_files": [f.short_path for f in ctx.files.data + transitive_npm_links_files], + "data_files": [f.short_path for f in data_files], } if ctx.attr.tool: config["tool"] = ctx.executable.tool.short_path @@ -135,15 +143,8 @@ def _impl(ctx): config["command"] = ctx.attr.command ctx.actions.write(config_file, json.encode(config)) - transitive_runfiles = [_gather_files_from_js_providers( - targets = ctx.attr.data, - include_transitive_sources = ctx.attr.include_transitive_sources, - include_declarations = ctx.attr.include_declarations, - include_npm_linked_packages = ctx.attr.include_npm_linked_packages, - )] runfiles_merge_targets = ctx.attr.data[:] if ctx.attr.tool: - transitive_runfiles.append(ctx.attr.tool.files) runfiles_merge_targets.append(ctx.attr.tool) runfiles = ctx.runfiles(