fix: do not put sources+types into NpmPackageStoreInfo.files (#1850)
Co-authored-by: Greg Magolan <greg@aspect.dev>
diff --git a/examples/linked_consumer/BUILD.bazel b/examples/linked_consumer/BUILD.bazel
index 5d0c395..662d757 100644
--- a/examples/linked_consumer/BUILD.bazel
+++ b/examples/linked_consumer/BUILD.bazel
@@ -1,4 +1,6 @@
-load("@aspect_rules_js//js:defs.bzl", "js_test")
+load("@aspect_bazel_lib//lib:copy_to_directory.bzl", "copy_to_directory")
+load("@aspect_bazel_lib//lib:diff_test.bzl", "diff_test")
+load("@aspect_rules_js//js:defs.bzl", "js_info_files", "js_test")
load("@npm//:defs.bzl", "npm_link_all_packages")
npm_link_all_packages(name = "node_modules")
@@ -22,3 +24,77 @@
],
entry_point = "test_pkg_deps_linked.js",
)
+
+# Test that sources & test can be pulled from a linked js_library the same
+# as they can be pulled out of an unlinked js_library
+js_info_files(
+ name = "unlinked_sources",
+ srcs = ["//examples/linked_lib:lib"],
+ include_npm_sources = False,
+ include_sources = True,
+ include_transitive_sources = True,
+ include_transitive_types = False,
+ include_types = False,
+)
+
+js_info_files(
+ name = "linked_sources",
+ srcs = [":node_modules/@lib/test2"],
+ include_npm_sources = False,
+ include_sources = True,
+ include_transitive_sources = True,
+ include_transitive_types = False,
+ include_types = False,
+)
+
+copy_to_directory(
+ name = "unlinked_sources_dir",
+ srcs = [":unlinked_sources"],
+)
+
+copy_to_directory(
+ name = "linked_sources_dir",
+ srcs = [":linked_sources"],
+)
+
+diff_test(
+ name = "sources_test",
+ file1 = ":unlinked_sources_dir",
+ file2 = ":linked_sources_dir",
+)
+
+js_info_files(
+ name = "linked_types",
+ srcs = [":node_modules/@lib/test2"],
+ include_npm_sources = False,
+ include_sources = False,
+ include_transitive_sources = False,
+ include_transitive_types = True,
+ include_types = True,
+)
+
+js_info_files(
+ name = "unlinked_types",
+ srcs = ["//examples/linked_lib:lib"],
+ include_npm_sources = False,
+ include_sources = False,
+ include_transitive_sources = False,
+ include_transitive_types = True,
+ include_types = True,
+)
+
+copy_to_directory(
+ name = "unlinked_types_dir",
+ srcs = [":unlinked_types"],
+)
+
+copy_to_directory(
+ name = "linked_types_dir",
+ srcs = [":linked_types"],
+)
+
+diff_test(
+ name = "types_test",
+ file1 = ":unlinked_types_dir",
+ file2 = ":linked_types_dir",
+)
diff --git a/examples/linked_lib/BUILD.bazel b/examples/linked_lib/BUILD.bazel
index 70b9dd1..82045c6 100644
--- a/examples/linked_lib/BUILD.bazel
+++ b/examples/linked_lib/BUILD.bazel
@@ -9,6 +9,8 @@
name = "pkg",
srcs = [
"index.js",
+ "one.d.ts",
+ "one.js",
"package.json",
],
visibility = ["//visibility:public"],
diff --git a/examples/linked_lib/one.d.ts b/examples/linked_lib/one.d.ts
new file mode 100644
index 0000000..b9dc8dd
--- /dev/null
+++ b/examples/linked_lib/one.d.ts
@@ -0,0 +1 @@
+export declare const one = 1
diff --git a/examples/linked_lib/one.js b/examples/linked_lib/one.js
new file mode 100644
index 0000000..e4d42ee
--- /dev/null
+++ b/examples/linked_lib/one.js
@@ -0,0 +1,4 @@
+'use strict'
+exports.__esModule = true
+exports.one = void 0
+exports.one = 1
diff --git a/npm/private/npm_link_package_store.bzl b/npm/private/npm_link_package_store.bzl
index a1798a4..71c9801 100644
--- a/npm/private/npm_link_package_store.bzl
+++ b/npm/private/npm_link_package_store.bzl
@@ -22,7 +22,7 @@
_ATTRS = {
"src": attr.label(
doc = """The npm_package_store target to link as a direct dependency.""",
- providers = [NpmPackageStoreInfo],
+ providers = [NpmPackageStoreInfo, JsInfo],
mandatory = True,
),
"package": attr.string(
@@ -61,6 +61,7 @@
def _npm_link_package_store_impl(ctx):
store_info = ctx.attr.src[NpmPackageStoreInfo]
+ store_js_info = ctx.attr.src[JsInfo]
package_store_directory = store_info.package_store_directory
if not package_store_directory:
@@ -91,11 +92,28 @@
)
files.append(bin_file)
- files_depset = depset(files, transitive = [store_info.files])
+ # All files required to run the package if consumed as `DefaultInfo`
+ files_depset = depset(files, transitive = [
+ store_info.files,
+ store_js_info.npm_sources,
+ store_js_info.sources,
+ ])
+ transitive_files_depset = depset(files, transitive = [
+ store_info.transitive_files,
+ store_js_info.npm_sources,
+ store_js_info.transitive_sources,
+ ])
- transitive_files_depset = depset(files, transitive = [store_info.transitive_files])
+ # Additional npm_sources required to to run the package, in addition to other
+ # data included in JsInfo provider.
+ npm_sources = depset(files, transitive = [
+ store_info.transitive_files,
+ store_js_info.npm_sources,
+ ])
providers = [
+ # Provide default info to allow consuming the package via `data` of rules
+ # not aware of JsInfo such as `sh_binary` etc.
DefaultInfo(
# Only provide direct files in DefaultInfo files
files = files_depset,
@@ -105,7 +123,11 @@
),
js_info(
target = ctx.label,
- npm_sources = transitive_files_depset,
+ sources = store_js_info.sources,
+ transitive_sources = store_js_info.transitive_sources,
+ types = store_js_info.types,
+ transitive_types = store_js_info.transitive_types,
+ npm_sources = npm_sources,
# only propagate non-dev npm dependencies to use as direct dependencies when linking downstream npm_package targets with npm_link_package
npm_package_store_infos = depset([store_info]) if not store_info.dev else depset(),
),
diff --git a/npm/private/npm_package_store.bzl b/npm/private/npm_package_store.bzl
index 862c0f7..8b3e2a5 100644
--- a/npm/private/npm_package_store.bzl
+++ b/npm/private/npm_package_store.bzl
@@ -105,7 +105,7 @@
> In contrast, Bazel makes it possible to make builds hermetic, which means that
> all dependencies of a program must be declared when running in Bazel's sandbox.
""",
- providers = [NpmPackageStoreInfo],
+ providers = [NpmPackageStoreInfo, JsInfo],
),
"package": attr.string(
doc = """The package name to link to.
@@ -180,9 +180,17 @@
package_store_name = utils.package_store_name(package, version)
package_store_directory = None
+ # files required to create the package store entry
files = []
transitive_files_depsets = []
+
+ # JsInfo of the package and all deps required to run
+ js_infos = []
+
+ # NpmPackageStoreInfo of the package and deps
npm_package_store_infos = []
+
+ # Direct references to all dependencies
direct_ref_deps = {}
# the path to the package store location for this package
@@ -277,6 +285,8 @@
# "node_modules/{package_store_root}/{package_store_name}/node_modules/{package}"
dep_symlink_path = "node_modules/{}/{}/node_modules/{}".format(utils.package_store_root, package_store_name, dep_info.package)
files.append(utils.make_symlink(ctx, dep_symlink_path, dep_package_store_directory.path))
+
+ # Include the store info of all linked dependencies
npm_package_store_infos.append(dep_info)
elif ctx.attr.src and JsInfo in ctx.attr.src:
jsinfo = ctx.attr.src[JsInfo]
@@ -287,10 +297,8 @@
else:
symlink_path = "{}/{}".format(ctx.label.package or ".", package_store_directory_path)
- transitive_files_depsets.append(jsinfo.npm_sources)
- transitive_files_depsets.append(jsinfo.transitive_sources)
- transitive_files_depsets.append(jsinfo.transitive_types)
- npm_package_store_infos.extend(jsinfo.npm_package_store_infos.to_list())
+ # The package JsInfo including all direct and transitive sources, store info etc.
+ js_infos.append(jsinfo)
if jsinfo.target.workspace_name:
target_path = "{}/external/{}/{}".format(ctx.bin_dir.path, jsinfo.target.workspace_name, jsinfo.target.package)
@@ -346,10 +354,14 @@
if package_store_directory:
files.append(package_store_directory)
+ # Include the store and js info of all dependencies expected to be linked
for target in ctx.attr.deps:
+ js_infos.append(target[JsInfo])
npm_package_store_infos.append(target[NpmPackageStoreInfo])
if ctx.attr.src:
+ sources_depset = depset(transitive = [jsinfo.transitive_sources for jsinfo in js_infos])
+ types_depset = depset(transitive = [jsinfo.transitive_types for jsinfo in js_infos])
for npm_package_store_info in npm_package_store_infos:
transitive_files_depsets.append(npm_package_store_info.transitive_files)
else:
@@ -360,21 +372,23 @@
# closure of all the entire package store deps, we can safely add just `files` from each of
# these to `transitive_files_depset`; doing so reduces the size of `transitive_files_depset`
# significantly and reduces analysis time and Bazel memory usage during analysis
+ sources_depset = depset(transitive = [jsinfo.sources for jsinfo in js_infos])
+ types_depset = depset(transitive = [jsinfo.types for jsinfo in js_infos])
for npm_package_store_info in npm_package_store_infos:
transitive_files_depsets.append(npm_package_store_info.files)
+ npm_sources = depset(files, transitive = [jsinfo.npm_sources for jsinfo in js_infos])
transitive_files_depset = depset(files, transitive = transitive_files_depsets)
-
files_depset = depset(files)
providers = [
js_info(
target = ctx.label,
- npm_sources = transitive_files_depset,
- ),
- DefaultInfo(
- files = files_depset,
- runfiles = ctx.runfiles(transitive_files = transitive_files_depset),
+ npm_sources = npm_sources,
+ sources = sources_depset,
+ transitive_sources = sources_depset,
+ types = types_depset,
+ transitive_types = types_depset,
),
NpmPackageStoreInfo(
root_package = ctx.label.package,
