feat: add support for linking js_library as 1p npm deps (#1646)

.aspect/rules/external_repository_action_cache/npm_translate_lock_LTE4Nzc1MDcwNjU=

@@ -1,14 +1,14 @@
 # @generated
-# Input hashes for repository rule npm_translate_lock(name = "npm", pnpm_lock = "//:pnpm-lock.yaml").
+# Input hashes for repository rule npm_translate_lock(name = "npm", pnpm_lock = "@@//:pnpm-lock.yaml").
 # This file should be checked into version control along with the pnpm-lock.yaml file.
 .npmrc=-2065072158
-pnpm-lock.yaml=915811237
+pnpm-lock.yaml=1142601305
 examples/npm_deps/patches/meaning-of-life@1.0.0-pnpm.patch=-442666336
 package.json=-275319675
-pnpm-workspace.yaml=-871530930
+pnpm-workspace.yaml=-1178830835
 examples/js_binary/package.json=-41174383
 examples/macro/package.json=857146175
-examples/npm_deps/package.json=283109008
+examples/npm_deps/package.json=-1377141392
 examples/npm_package/libs/lib_a/package.json=-1377103079
 examples/npm_package/packages/pkg_a/package.json=1006424040
 examples/npm_package/packages/pkg_b/package.json=1041247977
@@ -21,6 +21,7 @@ npm/private/test/npm_package/package.json=-1991705133
 npm/private/test/vendored/is-odd/package.json=1041695223
 npm/private/test/vendored/semver-max/package.json=578664053
 examples/linked_empty_node_modules/package.json=-1039372825
+examples/npm_package/packages/pkg_d/package.json=1110895851
 js/private/image/package.json=-1260474848
 js/private/test/image/package.json=1286417612
 js/private/test/js_run_devserver/package.json=-260856079

.bazelignore

@@ -6,6 +6,8 @@ examples/npm_deps/node_modules/
 examples/npm_package/libs/lib_a/node_modules/
 examples/npm_package/packages/pkg_a/node_modules/
 examples/npm_package/packages/pkg_b/node_modules/
+examples/npm_package/packages/pkg_c/node_modules/
+examples/npm_package/packages/pkg_d/node_modules/
 examples/webpack_cli/node_modules/
 js/private/coverage/bundle/node_modules
 js/private/image/node_modules

examples/npm_deps/BUILD.bazel

@@ -262,3 +262,20 @@ js_test(
     ],
     entry_point = "patched-dependencies-test.js",
 )
+
+#######################################
+# Case 9: use a first-party npm package within our Bazel monorepo workspace from a js_library target
+
+write_file(
+    name = "write9",
+    out = "case9.js",
+    content = ["require('@mycorp/pkg-d')"],
+)
+
+js_test(
+    name = "test9",
+    data = [
+        ":node_modules/@mycorp/pkg-d",
+    ],
+    entry_point = "case9.js",
+)

examples/npm_deps/package.json

@@ -5,7 +5,9 @@
         "@aspect-test/c": "2.0.2",
         "@gregmagolan/test-b": "0.0.2",
         "@mycorp/pkg-a": "workspace:*",
+        "@mycorp/pkg-d": "workspace:*",
         "@rollup/plugin-commonjs": "21.1.0",
+        "acorn": "8.7.1",
         "debug": "3.2.7",
         "meaning-of-life": "1.0.0",
         "mobx-react": "7.3.0",

examples/npm_package/packages/pkg_d/BUILD.bazel

@@ -0,0 +1,19 @@
+load("@aspect_rules_js//js:defs.bzl", "js_library")
+load("@npm//:defs.bzl", "npm_link_all_packages")
+
+npm_link_all_packages(name = "node_modules")
+
+js_library(
+    name = "pkg_d",
+    srcs = [
+        "index.js",
+        "package.json",
+    ],
+    visibility = ["//visibility:public"],
+    # because we're linking this js_library, we must explictly add our npm dependendies to `deps` so
+    # they are picked up my the linker. npm dependendies in `data` are not propogated through the
+    # linker when linking a js_libary.
+    deps = [
+        ":node_modules",
+    ],
+)

examples/npm_package/packages/pkg_d/README.md

@@ -0,0 +1,3 @@
+# @mycorp/pkg-d package
+
+'@mycorp/pkg-d' is an example of a first party package that is linked as a js_library

examples/npm_package/packages/pkg_d/index.js

@@ -0,0 +1,19 @@
+/**
+ * @fileoverview minimal test program that requires a third-party package from npm
+ */
+const acorn = require('acorn')
+const { v4: uuid } = require('uuid')
+
+function toAst(program) {
+    return JSON.stringify(acorn.parse(program, { ecmaVersion: 2020 })) + '\n'
+}
+
+function getAcornVersion() {
+    return acorn.version
+}
+
+module.exports = {
+    toAst,
+    getAcornVersion,
+    uuid,
+}

examples/npm_package/packages/pkg_d/package.json

@@ -0,0 +1,10 @@
+{
+    "name": "@mycorp/pkg-d",
+    "private": true,
+    "dependencies": {
+        "uuid": "8.3.2"
+    },
+    "peerDependencies": {
+        "acorn": "8.x.x"
+    }
+}

js/private/js_info.bzl

@@ -3,6 +3,7 @@
 JsInfo = provider(
     doc = "Encapsulates information provided by rules in rules_js and derivative rule sets",
     fields = {
+        "target": "The label of target that created this JsInfo",
         "sources": "A depset of source files produced by the target",
         "types": "A depset of typings files produced by the target",
         "transitive_sources": "A depset of source files produced by the target and the target's transitive deps",
@@ -13,6 +14,7 @@ JsInfo = provider(
 )
 
 def js_info(
+        target,
         sources = depset(),
         types = depset(),
         transitive_sources = depset(),
@@ -22,6 +24,7 @@ def js_info(
     """Construct a JsInfo.
 
     Args:
+        target: See JsInfo documentation
         sources: See JsInfo documentation
         types: See JsInfo documentation
         transitive_sources: See JsInfo documentation
@@ -32,6 +35,9 @@ def js_info(
     Returns:
         A JsInfo provider
     """
+    if type(target) != "Label":
+        msg = "Expected target to be a Label but got {}".format(type(target))
+        fail(msg)
     if type(sources) != "depset":
         msg = "Expected sources to be a depset but got {}".format(type(sources))
         fail(msg)
@@ -52,6 +58,7 @@ def js_info(
         fail(msg)
 
     return JsInfo(
+        target = target,
         sources = sources,
         types = types,
         transitive_sources = transitive_sources,

js/private/js_library.bzl

@@ -214,6 +214,7 @@ def _js_library_impl(ctx):
 
     return [
         js_info(
+            target = ctx.label,
             sources = sources,
             types = types,
             transitive_sources = transitive_sources,

npm/private/npm_link_package_store.bzl

@@ -105,6 +105,7 @@ def _npm_link_package_store_impl(ctx):
             runfiles = ctx.runfiles(transitive_files = transitive_files_depset),
         ),
         js_info(
+            target = ctx.label,
             npm_sources = transitive_files_depset,
             # 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(),

npm/private/npm_package_store.bzl

@@ -6,6 +6,9 @@ load(":utils.bzl", "utils")
 load(":npm_package_info.bzl", "NpmPackageInfo")
 load(":npm_package_store_info.bzl", "NpmPackageStoreInfo")
 
+# buildifier: disable=bzl-visibility
+load("//js/private:js_info.bzl", "JsInfo")
+
 _DOC = """Defines a npm package that is linked into a node_modules tree.
 
 The npm package is linked with a pnpm style symlinked node_modules output tree.
@@ -24,7 +27,6 @@ _ATTRS = {
     "src": attr.label(
         doc = """A npm_package target or or any other target that provides a NpmPackageInfo.
         """,
-        providers = [NpmPackageInfo],
         mandatory = True,
     ),
     "deps": attr.label_keyed_string_dict(
@@ -150,28 +152,47 @@ If set, takes precendance over the package version in the NpmPackageInfo src.
 }
 
 def _npm_package_store_impl(ctx):
-    package = ctx.attr.package if ctx.attr.package else ctx.attr.src[NpmPackageInfo].package
-    version = ctx.attr.version if ctx.attr.version else ctx.attr.src[NpmPackageInfo].version
+    if ctx.attr.src:
+        if NpmPackageInfo in ctx.attr.src:
+            package = ctx.attr.package if ctx.attr.package else ctx.attr.src[NpmPackageInfo].package
+            version = ctx.attr.version if ctx.attr.version else ctx.attr.src[NpmPackageInfo].version
+        elif JsInfo in ctx.attr.src:
+            if not ctx.attr.package:
+                msg = "Expected package to be specified in '{}' when src '{}' provides a JsInfo".format(ctx.label, ctx.attr.src[JsInfo].target)
+                fail(msg)
+            package = ctx.attr.package
+            version = ctx.attr.version if ctx.attr.version else "0.0.0"
+        else:
+            msg = "Expected src of '{}' to provide either NpmPackageInfo or JsInfo".format(ctx.label)
+            fail(msg)
+    else:
+        # ctx.attr.src can be unspecified when the rule is a npm_package_store_internal; when it is _not_
+        # set, this is a terminal 3p package with ctx.attr.deps being the transitive closure of
+        # deps; this pattern is used to break circular dependencies between 3rd party npm deps; it
+        # is not used for 1st party deps
+        package = ctx.attr.package
+        version = ctx.attr.version
 
     if not package:
         fail("No package name specified to link to. Package name must either be specified explicitly via 'package' attribute or come from the 'src' 'NpmPackageInfo', typically a 'npm_package' target")
     if not version:
         fail("No package version specified to link to. Package version must either be specified explicitly via 'version' attribute or come from the 'src' 'NpmPackageInfo', typically a 'npm_package' target")
 
     package_store_name = utils.package_store_name(package, version)
-
-    src = None
     package_store_directory = None
+
     files = []
+    transitive_files_depsets = []
+    transitive_package_store_infos_depsets = []
+    npm_package_store_infos = []
     direct_ref_deps = {}
 
-    npm_package_store_infos = []
+    # the path to the package store location for this package
+    # "node_modules/{package_store_root}/{package_store_name}/node_modules/{package}"
+    package_store_directory_path = paths.join("node_modules", utils.package_store_root, package_store_name, "node_modules", package)
 
-    if ctx.attr.src:
+    if ctx.attr.src and NpmPackageInfo in ctx.attr.src:
         # output the package as a TreeArtifact to its package store location
-        # "node_modules/{package_store_root}/{package_store_name}/node_modules/{package}"
-        package_store_directory_path = paths.join("node_modules", utils.package_store_root, package_store_name, "node_modules", package)
-
         if ctx.label.workspace_name:
             expected_short_path = paths.join("..", ctx.label.workspace_name, ctx.label.package, package_store_directory_path)
         else:
@@ -253,7 +274,22 @@ deps of npm_package_store must be in the same package.""" % (ctx.label.package,
                 dep_symlink_path = paths.join("node_modules", utils.package_store_root, package_store_name, "node_modules", dep_package)
                 files.append(utils.make_symlink(ctx, dep_symlink_path, dep_package_store_directory.path))
                 npm_package_store_infos.append(store)
-    else:
+    elif ctx.attr.src and JsInfo in ctx.attr.src:
+        # Symlink to the directory of the target that created this JsInfo
+        if ctx.label.workspace_name:
+            symlink_path = paths.join("external", ctx.label.workspace_name, ctx.label.package, package_store_directory_path)
+        else:
+            symlink_path = paths.join(ctx.label.package, package_store_directory_path)
+        transitive_files_depsets.append(ctx.attr.src[JsInfo].transitive_sources)
+        transitive_files_depsets.append(ctx.attr.src[JsInfo].transitive_types)
+        transitive_package_store_infos_depsets.append(ctx.attr.src[JsInfo].npm_package_store_infos)
+        if ctx.attr.src[JsInfo].target.workspace_name:
+            target_path = paths.join(ctx.bin_dir.path, "external", ctx.attr.src[JsInfo].target.workspace_name, ctx.attr.src[JsInfo].target.package)
+            package_store_directory = utils.make_symlink(ctx, symlink_path, target_path)
+        else:
+            target_path = paths.join(ctx.bin_dir.path, ctx.attr.src[JsInfo].target.package)
+            package_store_directory = utils.make_symlink(ctx, symlink_path, target_path)
+    elif not ctx.attr.src:
         # ctx.attr.src can be unspecified when the rule is a npm_package_store_internal; when it is _not_
         # set, this is a terminal 3p package with ctx.attr.deps being the transitive closure of
         # deps; this pattern is used to break circular dependencies between 3rd party npm deps; it
@@ -292,6 +328,9 @@ deps of npm_package_store must be in the same package.""" % (ctx.label.package,
                         # "node_modules/{package_store_root}/{package_store_name}/node_modules/{package}"
                         dep_ref_dep_symlink_path = paths.join("node_modules", utils.package_store_root, dep_package_store_name, "node_modules", dep_ref_dep_alias)
                         files.append(utils.make_symlink(ctx, dep_ref_dep_symlink_path, dep_ref_def_package_store_directory.path))
+    else:
+        # We should _never_ get here
+        fail("Internal error")
 
     if package_store_directory:
         files.append(package_store_directory)
@@ -303,10 +342,14 @@ deps of npm_package_store must be in the same package.""" % (ctx.label.package,
 
     files_depset = depset(files)
 
+    for transitive_package_store_infos_depset in transitive_package_store_infos_depsets:
+        for npm_package_store_info in transitive_package_store_infos_depset.to_list():
+            npm_package_store_infos.append(npm_package_store_info)
+
     if ctx.attr.src:
-        transitive_files_depset = depset(files, transitive = [
-            npm_package_store.transitive_files
-            for npm_package_store in npm_package_store_infos
+        transitive_files_depset = depset(files, transitive = transitive_files_depsets + [
+            npm_package_store_info.transitive_files
+            for npm_package_store_info in npm_package_store_infos
         ])
     else:
         # ctx.attr.src can be unspecified when the rule is a npm_package_store_internal; when ctx.attr.src is
@@ -316,9 +359,9 @@ deps of npm_package_store must be in the same package.""" % (ctx.label.package,
         # 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
-        transitive_files_depset = depset(files, transitive = [
-            npm_package_store.files
-            for npm_package_store in npm_package_store_infos
+        transitive_files_depset = depset(files, transitive = transitive_files_depsets + [
+            npm_package_store_info.files
+            for npm_package_store_info in npm_package_store_infos
         ])
 
     providers = [
@@ -336,7 +379,7 @@ deps of npm_package_store must be in the same package.""" % (ctx.label.package,
             dev = ctx.attr.dev,
         ),
     ]
-    if package_store_directory:
+    if package_store_directory and package_store_directory.is_directory:
         # Provide an output group that provides a single file which is the
         # package directory for use in $(execpath) and $(rootpath).
         # Output group name must match utils.package_directory_output_group

npm/private/npm_package_store_internal.bzl

@@ -2,7 +2,6 @@
 
 load("@bazel_skylib//lib:dicts.bzl", "dicts")
 load(":npm_package_store.bzl", _npm_package_store_lib = "npm_package_store_lib")
-load(":npm_package_info.bzl", "NpmPackageInfo")
 
 _INTERNAL_ATTRS_STORE = dicts.add(_npm_package_store_lib.attrs, {
     "src": attr.label(
@@ -15,7 +14,6 @@ _INTERNAL_ATTRS_STORE = dicts.add(_npm_package_store_lib.attrs, {
         complication. Outside our `npm_import` you should structure you `npm_link_package` targets in
         a DAG (without cycles).
         """,
-        providers = [NpmPackageInfo],
     ),
     "package": attr.string(
         doc = """The package name to link to.