Skip to content

Commit

Permalink
feat(builtin): add support for stopping propagation of `link_node_mod…
Browse files Browse the repository at this point in the history
…ules` aspect

This commit does two things:

  1. It switches the `link_node_modules` aspect from the deprecated
     struct provider variant to the provider list return value.
  2. The aspect no longer collects aspect results from dependencies
     if the target already provides explicit mapping information for the
     linker. This is an API for advanced use-cases as outlined in
     #2941.
  • Loading branch information
devversion authored and alexeagle committed Sep 17, 2021
1 parent 2c077f7 commit dedc982
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 15 deletions.
34 changes: 24 additions & 10 deletions internal/linker/link_node_modules.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,12 @@ def _debug(vars, *args):
if "VERBOSE_LOGS" in vars.keys():
print("[link_node_modules.bzl]", *args)

# Arbitrary name; must be chosen to globally avoid conflicts with any other aspect
MODULE_MAPPINGS_ASPECT_RESULTS_NAME = "link_node_modules__aspect_result"
LinkerPackageMappingInfo = provider(
doc = """Provider capturing package mappings for the linker to consume.""",
fields = {
"mappings": "Dictionary of mappings. Maps package names to an exec path.",
},
)

# Traverse 'srcs' in addition so that we can go across a genrule
_MODULE_MAPPINGS_DEPS_NAMES = ["data", "deps", "srcs"]
Expand Down Expand Up @@ -79,7 +83,10 @@ def write_node_modules_manifest(ctx, extra_data = [], mnemonic = None, link_work

# Look through data/deps attributes to find first party deps to link
for dep in extra_data + getattr(ctx.attr, "data", []) + getattr(ctx.attr, "deps", []):
for k, v in getattr(dep, MODULE_MAPPINGS_ASPECT_RESULTS_NAME, {}).items():
if not LinkerPackageMappingInfo in dep:
continue

for k, v in dep[LinkerPackageMappingInfo].mappings.items():
map_key_split = k.split(":")
package_name = map_key_split[0]
package_path = map_key_split[1] if len(map_key_split) > 1 else ""
Expand Down Expand Up @@ -135,10 +142,13 @@ def _get_module_mappings(target, ctx):
"""
mappings = {}

# Propogate transitive mappings
# Propagate transitive mappings
for name in _MODULE_MAPPINGS_DEPS_NAMES:
for dep in getattr(ctx.rule.attr, name, []):
for k, v in getattr(dep, MODULE_MAPPINGS_ASPECT_RESULTS_NAME, {}).items():
if not LinkerPackageMappingInfo in dep:
continue

for k, v in dep[LinkerPackageMappingInfo].mappings.items():
if _link_mapping(target.label, mappings, k, v):
_debug(ctx.var, "target %s propagating module mapping %s: %s" % (dep.label, k, v))
mappings[k] = v
Expand Down Expand Up @@ -173,11 +183,15 @@ def _get_module_mappings(target, ctx):
return mappings

def _module_mappings_aspect_impl(target, ctx):
# Use a dictionary to construct the result struct
# so that we can reference the MODULE_MAPPINGS_ASPECT_RESULTS_NAME variable
return struct(**{
MODULE_MAPPINGS_ASPECT_RESULTS_NAME: _get_module_mappings(target, ctx),
})
# If the target explicitly provides mapping information, we will not propagate
# any information. The target already provides explicit mapping information.
# See details on a concrete use-case: https://github.com/bazelbuild/rules_nodejs/issues/2941.
if LinkerPackageMappingInfo in target:
return []

return [
LinkerPackageMappingInfo(mappings = _get_module_mappings(target, ctx)),
]

module_mappings_aspect = aspect(
_module_mappings_aspect_impl,
Expand Down
7 changes: 5 additions & 2 deletions internal/node/node.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ load("//internal/common:module_mappings.bzl", "module_mappings_runtime_aspect")
load("//internal/common:path_utils.bzl", "strip_external")
load("//internal/common:preserve_legacy_templated_args.bzl", "preserve_legacy_templated_args")
load("//internal/common:windows_utils.bzl", "create_windows_native_launcher_script", "is_windows")
load("//internal/linker:link_node_modules.bzl", "MODULE_MAPPINGS_ASPECT_RESULTS_NAME", "module_mappings_aspect", "write_node_modules_manifest")
load("//internal/linker:link_node_modules.bzl", "LinkerPackageMappingInfo", "module_mappings_aspect", "write_node_modules_manifest")
load("//nodejs:repositories.bzl", "BUILT_IN_NODE_PLATFORMS")

def _trim_package_node_modules(package_name):
Expand Down Expand Up @@ -58,7 +58,10 @@ def _compute_node_modules_roots(ctx, data):

# Add in roots for multi-linked first party deps
for dep in data:
for k, v in getattr(dep, MODULE_MAPPINGS_ASPECT_RESULTS_NAME, {}).items():
if not LinkerPackageMappingInfo in dep:
continue

for k, v in dep[LinkerPackageMappingInfo].mappings.items():
map_key_split = k.split(":")
package_name = map_key_split[0]
package_path = map_key_split[1] if len(map_key_split) > 1 else ""
Expand Down
6 changes: 3 additions & 3 deletions packages/esbuild/esbuild.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ esbuild rule

load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "params_file")
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", "MODULE_MAPPINGS_ASPECT_RESULTS_NAME", "module_mappings_aspect")
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")
load("@build_bazel_rules_nodejs//toolchains/esbuild:toolchain.bzl", "TOOLCHAIN")
load(":helpers.bzl", "desugar_entry_point_names", "filter_files", "generate_path_mapping", "resolve_entry_point", "write_args_file", "write_jsconfig_file")
Expand Down Expand Up @@ -34,8 +34,8 @@ def _esbuild_impl(ctx):
deps_depsets.append(dep[ExternalNpmPackageInfo].sources)

# Collect the path alias mapping to resolve packages correctly
if hasattr(dep, MODULE_MAPPINGS_ASPECT_RESULTS_NAME):
for key, value in getattr(dep, MODULE_MAPPINGS_ASPECT_RESULTS_NAME).items():
if LinkerPackageMappingInfo in dep:
for key, value in dep[LinkerPackageMappingInfo].mappings.items():
# key is of format "package_name:package_path"
package_name = key.split(":")[0]
path_alias_mappings.update(generate_path_mapping(package_name, value.replace(ctx.bin_dir.path + "/", "")))
Expand Down

0 comments on commit dedc982

Please sign in to comment.