fix: update usages of `ExternalNpmPackageInfo.path` to be safe and default to empty string `ExternalNpmPackageInfo` had a `path` attribute introduced and used in 2c2cc6eec42b0f4f805fb3063920107b78f821fd, but the provider was publicly exported beforehand. Even though all internal usages were updated to include a path, this change was shipped in `3.2.0` and introduced an unintentional breaking change for consumers. Most notably, `rules_postcss` manually implements this provider and does not give a `path` failing in versions `3.2.0` and up. https://github.com/bazelbuild/rules_postcss/blob/2bd16fda40cd4bf4fbf0b477b968366ec1602103/internal/plugin.bzl#L30-L41 Solution here is to safely retrieve the `path` attribute and default to empty string at all places it is used. A test is also introduced to ensure that a `nodejs_binary()` can depend on a custom `ExternalNpmPackageInfo` provider without a `path` attribute, which should hopefully prevent against regressions.
diff --git a/internal/linker/link_node_modules.bzl b/internal/linker/link_node_modules.bzl index 5d30429..eb57221 100644 --- a/internal/linker/link_node_modules.bzl +++ b/internal/linker/link_node_modules.bzl
@@ -81,7 +81,7 @@ # we'll symlink local "node_modules" to them for dep in extra_data + getattr(ctx.attr, "data", []) + getattr(ctx.attr, "deps", []): if ExternalNpmPackageInfo in dep: - path = dep[ExternalNpmPackageInfo].path + path = getattr(dep[ExternalNpmPackageInfo], "path", "") workspace = dep[ExternalNpmPackageInfo].workspace if path in node_modules_roots: other_workspace = node_modules_roots[path]
diff --git a/internal/linker/test/default_path/BUILD.bazel b/internal/linker/test/default_path/BUILD.bazel new file mode 100644 index 0000000..146573c --- /dev/null +++ b/internal/linker/test/default_path/BUILD.bazel
@@ -0,0 +1,30 @@ +load("//:index.bzl", "generated_file_test", "nodejs_binary", "npm_package_bin") +load( + ":custom_external_npm_package_info.bzl", + "custom_external_npm_package_info", +) + +package(default_testonly = True) + +# Custom target returning `ExternalNpmPackageInfo` without a `path` field. +custom_external_npm_package_info( + name = "external_npm_package_info", +) + +nodejs_binary( + name = "binary", + data = [":external_npm_package_info"], + entry_point = "default_path.js", +) + +npm_package_bin( + name = "generated", + stdout = "generated.txt", + tool = ":binary", +) + +generated_file_test( + name = "test", + src = "golden.txt", + generated = "generated.txt", +)
diff --git a/internal/linker/test/default_path/custom_external_npm_package_info.bzl b/internal/linker/test/default_path/custom_external_npm_package_info.bzl new file mode 100644 index 0000000..865934c --- /dev/null +++ b/internal/linker/test/default_path/custom_external_npm_package_info.bzl
@@ -0,0 +1,30 @@ +"""Custom `ExternalNpmPackageInfo` target without a `path` attribute.""" + +load( + "//internal/providers:external_npm_package_info.bzl", + "ExternalNpmPackageInfo", +) + +def _custom_external_npm_package_info_impl(ctx): + return ExternalNpmPackageInfo( + direct_sources = depset([], transitive = ctx.files.deps), + sources = depset(ctx.files.srcs, transitive = ctx.files.deps), + has_directories = False, + workspace = "npm", + # `path` is intentionally **not** provided. + # Historical note: `ExternalNpmPackageInfo` was publicly exported prior + # to the `path` attribute's introduction. This means that we cannot + # assume an `ExternalNpmPackageInfo` has a `path`, and must safely check + # it each time. + ) + +custom_external_npm_package_info = rule( + implementation = _custom_external_npm_package_info_impl, + attrs = { + "srcs": attr.label_list( + allow_files = True, + default = [], + ), + "deps": attr.label_list(default = []), + }, +)
diff --git a/internal/linker/test/default_path/default_path.js b/internal/linker/test/default_path/default_path.js new file mode 100644 index 0000000..daeacec --- /dev/null +++ b/internal/linker/test/default_path/default_path.js
@@ -0,0 +1 @@ +console.log('ExternalNpmPackageInfo with no path works!');
diff --git a/internal/linker/test/default_path/golden.txt b/internal/linker/test/default_path/golden.txt new file mode 100644 index 0000000..91291df --- /dev/null +++ b/internal/linker/test/default_path/golden.txt
@@ -0,0 +1 @@ +ExternalNpmPackageInfo with no path works!
diff --git a/internal/node/node.bzl b/internal/node/node.bzl index a2dd51d..30f8485 100644 --- a/internal/node/node.bzl +++ b/internal/node/node.bzl
@@ -48,7 +48,7 @@ # Add in roots from third-party deps for d in ctx.attr.data: if ExternalNpmPackageInfo in d: - path = d[ExternalNpmPackageInfo].path + path = getattr(d[ExternalNpmPackageInfo], "path", "") workspace = d[ExternalNpmPackageInfo].workspace if path in node_modules_roots: other_workspace = node_modules_roots[path]
diff --git a/internal/providers/external_npm_package_info.bzl b/internal/providers/external_npm_package_info.bzl index 43a0580..72aecda 100644 --- a/internal/providers/external_npm_package_info.bzl +++ b/internal/providers/external_npm_package_info.bzl
@@ -45,7 +45,7 @@ for dep in ctx.rule.attr.deps: if ExternalNpmPackageInfo in dep: has_directories = has_directories or dep[ExternalNpmPackageInfo].has_directories - path = dep[ExternalNpmPackageInfo].path + path = getattr(dep[ExternalNpmPackageInfo], "path", "") workspace = dep[ExternalNpmPackageInfo].workspace sources_depsets = [] if path in paths:
diff --git a/internal/providers/node_runtime_deps_info.bzl b/internal/providers/node_runtime_deps_info.bzl index ede3f4a..151978e 100644 --- a/internal/providers/node_runtime_deps_info.bzl +++ b/internal/providers/node_runtime_deps_info.bzl
@@ -50,7 +50,7 @@ deps += ctx.attr.deps for d in deps: if ExternalNpmPackageInfo in d: - path = d[ExternalNpmPackageInfo].path + path = getattr(d[ExternalNpmPackageInfo], "path", "") workspace = d[ExternalNpmPackageInfo].workspace if path in node_modules_roots: other_workspace = node_modules_roots[path]