feat(typescript): add JSNamedModuleInfo provider to ts_library outputs

All rules that require devmode named sources are updated to use JSNamedModuleInfo. The rules affected are karma, protractor, ts_devserver, ts_proto_library (in labs) & npm_package.

BREAKING CHANGES:
The following breaking changes are from internal details so they should not affect most users, however, some downstream projects such as Angular rely on these and will need to be updated accordingly when updating to the next release.
* `sources_aspect` from `/internal/node/node.bzl` and `/internal/common/sources_aspect.bzl` is removed; its functionality was duplicate to what JSNamedModuleInfo providers
* NodeModuleSources is removed and its `sources` field is moved to NodeModuleInfo; sources in the `scripts` field are now provided by JSNamedModuleInfo
* node_module_library `scripts` attribute renamed to `named_sources`
* `collect_node_modules_aspect` renamed to just `node_modules_aspect`

internal/common/devmode_js_sources.bzl

@@ -17,7 +17,7 @@
 Outputs a manifest file with the sources listed.
 """
 
-load(":sources_aspect.bzl", "sources_aspect")
+load("@build_bazel_rules_nodejs//:providers.bzl", "JSNamedModuleInfo")
 
 # Avoid using non-normalized paths (workspace/../other_workspace/path)
 def _to_manifest_path(ctx, file):
@@ -27,26 +27,30 @@ def _to_manifest_path(ctx, file):
         return ctx.workspace_name + "/" + file.short_path
 
 def _devmode_js_sources_impl(ctx):
-    files = depset()
-
-    for d in ctx.attr.deps:
-        if hasattr(d, "node_sources"):
-            files = depset(transitive = [files, d.node_sources])
-        elif hasattr(d, "files"):
-            files = depset(transitive = [files, d.files])
+    sources_depsets = []
+    for dep in ctx.attr.deps:
+        if JSNamedModuleInfo in dep:
+            sources_depsets.append(dep[JSNamedModuleInfo].sources)
+        if hasattr(dep, "files"):
+            sources_depsets.append(dep.files)
+    sources = depset(transitive = sources_depsets)
 
     ctx.actions.write(ctx.outputs.manifest, "".join([
         _to_manifest_path(ctx, f) + "\n"
-        for f in files.to_list()
+        for f in sources.to_list()
+        if f.path.endswith(".js") or f.path.endswith(".mjs")
     ]))
-    return [DefaultInfo(files = files)]
+
+    return [DefaultInfo(
+        files = sources,
+        runfiles = ctx.runfiles(transitive_files = sources),
+    )]
 
 devmode_js_sources = rule(
     implementation = _devmode_js_sources_impl,
     attrs = {
         "deps": attr.label_list(
             allow_files = True,
-            aspects = [sources_aspect],
         ),
     },
     outputs = {

internal/common/node_module_info.bzl

@@ -12,63 +12,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""NodeModuleInfo & NodeModuleSources providers and apsect to collect node_modules from deps.
+"""NodeModuleInfo providers and apsect to collect node_modules from deps.
 """
 
-# NodeModuleInfo provider is only provided by targets that are npm dependencies by the
-# `node_module_library` rule. This provider is currently used by different rules to filter out
-# npm dependencies such as
-# ```
-# [d for d in ctx.attr.deps if not NodeModuleInfo in d]
-# ```
-# in `packages/typescript/internal/build_defs.bzl` or
-# ```
-# hasattr(target, "files") and not NodeModuleInfo in target:
-# ```
-# in `internal/common/sources_aspect.bzl`.
-# Similar filtering is done in downstream repositories such as angular/angular so this provider
-# needs to go through a deprecation period before it can be phased out.
-NodeModuleInfo = provider(
-    doc = "Provides information about npm dependencies installed with yarn_install and npm_install rules",
-    fields = {
-        "workspace": "The workspace name that the npm dependencies are provided from",
-    },
-)
-
-# NodeModuleSources provider is provided by targets that are npm dependencies by the
+# NodeModuleInfo provider is provided by targets that are npm dependencies by the
 # `node_module_library` rule as well as other targets that have direct or transitive deps on
-# `node_module_library` targets via the `collect_node_modules_aspect` below.
-# TODO: rename to NodeModuleSourcesInfo so name doesn't trigger name-conventions warning
-# buildozer: disable=name-conventions
-NodeModuleSources = provider(
-    doc = "Provides sources for npm dependencies installed with yarn_install and npm_install rules",
+# `node_module_library` targets via the `node_modules_aspect` below.
+NodeModuleInfo = provider(
+    doc = "Provides information about npm dependencies",
     fields = {
-        "scripts": "Source files that are javascript named-UMD or named-AMD modules for use in rules such as ts_devserver",
-        "sources": "Source files that are npm dependencies",
+        "sources": "Source files that are direct & transitive npm depedendencies",
         "workspace": "The workspace name that the npm dependencies are provided from",
     },
 )
 
-def _collect_node_modules_aspect_impl(target, ctx):
-    nm_wksp = None
-
-    if NodeModuleSources in target:
-        return []
+def _node_modules_aspect_impl(target, ctx):
+    providers = []
 
-    if hasattr(ctx.rule.attr, "deps"):
+    # provide NodeModuleInfo if it is not already provided there are NodeModuleInfo deps
+    if not NodeModuleInfo in target:
         sources = depset()
-        for dep in ctx.rule.attr.deps:
-            if NodeModuleSources in dep:
-                if nm_wksp and dep[NodeModuleSources].workspace != nm_wksp:
-                    fail("All npm dependencies need to come from a single workspace. Found '%s' and '%s'." % (nm_wksp, dep[NodeModuleSources].workspace))
-                nm_wksp = dep[NodeModuleSources].workspace
-                sources = depset(transitive = [dep[NodeModuleSources].sources, sources])
-        if sources:
-            return [NodeModuleSources(sources = sources, workspace = nm_wksp)]
-
-    return []
-
-collect_node_modules_aspect = aspect(
-    implementation = _collect_node_modules_aspect_impl,
+        nm_wksp = None
+        if hasattr(ctx.rule.attr, "deps"):
+            for dep in ctx.rule.attr.deps:
+                if NodeModuleInfo in dep:
+                    if nm_wksp and dep[NodeModuleInfo].workspace != nm_wksp:
+                        fail("All npm dependencies need to come from a single workspace. Found '%s' and '%s'." % (nm_wksp, dep[NodeModuleInfo].workspace))
+                    nm_wksp = dep[NodeModuleInfo].workspace
+                    sources = depset(transitive = [dep[NodeModuleInfo].sources, sources])
+            if nm_wksp:
+                providers.extend([NodeModuleInfo(sources = sources, workspace = nm_wksp)])
+
+    return providers
+
+node_modules_aspect = aspect(
+    _node_modules_aspect_impl,
     attr_aspects = ["deps"],
 )

internal/linker/link_node_modules.bzl

@@ -10,7 +10,7 @@ linker, which uses the mappings to link a node_modules directory for
 runtimes to locate all the first-party packages.
 """
 
-load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleSources")
+load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleInfo")
 
 def _debug(vars, *args):
     if "VERBOSE_LOGS" in vars.keys():
@@ -42,8 +42,8 @@ def register_node_modules_linker(ctx, args, inputs):
     # Look through data/deps attributes to find...
     for dep in getattr(ctx.attr, "data", []) + getattr(ctx.attr, "deps", []):
         # ...the root directory for the third-party node_modules; we'll symlink the local "node_modules" to it
-        if NodeModuleSources in dep:
-            possible_root = "/".join([dep[NodeModuleSources].workspace, "node_modules"])
+        if NodeModuleInfo in dep:
+            possible_root = "/".join([dep[NodeModuleInfo].workspace, "node_modules"])
             if not node_modules_root:
                 node_modules_root = possible_root
             elif node_modules_root != possible_root:

internal/node.bzl

@@ -23,7 +23,5 @@ load(
     "//internal/common:expand_into_runfiles.bzl",
     _expand_location_into_runfiles = "expand_location_into_runfiles",
 )
-load("//internal/common:sources_aspect.bzl", _sources_aspect = "sources_aspect")
 
-sources_aspect = _sources_aspect
 expand_location_into_runfiles = _expand_location_into_runfiles

internal/node/node.bzl

@@ -20,10 +20,11 @@ They support module mapping: any targets in the transitive dependencies with
 a `module_name` attribute can be `require`d by that name.
 """
 
-load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleSources", "collect_node_modules_aspect")
+load("@build_bazel_rules_nodejs//:declaration_provider.bzl", "DeclarationInfo")
+load("@build_bazel_rules_nodejs//:providers.bzl", "JSNamedModuleInfo")
+load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleInfo", "node_modules_aspect")
 load("//internal/common:expand_into_runfiles.bzl", "expand_location_into_runfiles")
 load("//internal/common:module_mappings.bzl", "module_mappings_runtime_aspect")
-load("//internal/common:sources_aspect.bzl", "sources_aspect")
 load("//internal/common:windows_utils.bzl", "create_windows_native_launcher_script", "is_windows")
 
 def _trim_package_node_modules(package_name):
@@ -48,8 +49,8 @@ def _compute_node_modules_root(ctx):
     """
     node_modules_root = None
     if ctx.attr.node_modules:
-        if NodeModuleSources in ctx.attr.node_modules:
-            node_modules_root = "/".join([ctx.attr.node_modules[NodeModuleSources].workspace, "node_modules"])
+        if NodeModuleInfo in ctx.attr.node_modules:
+            node_modules_root = "/".join([ctx.attr.node_modules[NodeModuleInfo].workspace, "node_modules"])
         elif ctx.files.node_modules:
             # ctx.files.node_modules is not an empty list
             workspace = ctx.attr.node_modules.label.workspace_root.split("/")[1] if ctx.attr.node_modules.label.workspace_root else ctx.workspace_name
@@ -59,8 +60,8 @@ def _compute_node_modules_root(ctx):
                 "node_modules",
             ] if f])
     for d in ctx.attr.data:
-        if NodeModuleSources in d:
-            possible_root = "/".join([d[NodeModuleSources].workspace, "node_modules"])
+        if NodeModuleInfo in d:
+            possible_root = "/".join([d[NodeModuleInfo].workspace, "node_modules"])
             if not node_modules_root:
                 node_modules_root = possible_root
             elif node_modules_root != possible_root:
@@ -130,19 +131,23 @@ def _nodejs_binary_impl(ctx):
     node_modules = depset(ctx.files.node_modules)
 
     # Also include files from npm fine grained deps as inputs.
-    # These deps are identified by the NodeModuleSources provider.
+    # These deps are identified by the NodeModuleInfo provider.
     for d in ctx.attr.data:
-        if NodeModuleSources in d:
-            node_modules = depset(transitive = [node_modules, d[NodeModuleSources].sources])
+        if NodeModuleInfo in d:
+            node_modules = depset(transitive = [node_modules, d[NodeModuleInfo].sources])
 
     # Using a depset will allow us to avoid flattening files and sources
     # inside this loop. This should reduce the performances hits,
     # since we don't need to call .to_list()
     sources = depset()
 
     for d in ctx.attr.data:
-        if hasattr(d, "node_sources"):
-            sources = depset(transitive = [sources, d.node_sources])
+        if DeclarationInfo in d:
+            sources = depset(transitive = [sources, d[DeclarationInfo].transitive_declarations])
+
+        # TODO: switch to JSModuleInfo when it is available
+        if JSNamedModuleInfo in d:
+            sources = depset(transitive = [sources, d[JSNamedModuleInfo].sources])
         if hasattr(d, "files"):
             sources = depset(transitive = [sources, d.files])
 
@@ -262,7 +267,7 @@ _NODEJS_EXECUTABLE_ATTRS = {
     "data": attr.label_list(
         doc = """Runtime dependencies which may be loaded during execution.""",
         allow_files = True,
-        aspects = [sources_aspect, module_mappings_runtime_aspect, collect_node_modules_aspect],
+        aspects = [node_modules_aspect, module_mappings_runtime_aspect],
     ),
     "default_env_vars": attr.string_list(
         doc = """Default environment variables that are added to `configuration_env_vars`.

internal/node/npm_package_bin.bzl

@@ -1,13 +1,14 @@
 "A generic rule to run a tool that appears in node_modules/.bin"
 
+load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleInfo", "node_modules_aspect")
 load("@build_bazel_rules_nodejs//internal/linker:link_node_modules.bzl", "module_mappings_aspect", "register_node_modules_linker")
 
 # Note: this API is chosen to match nodejs_binary
 # so that we can generate macros that act as either an output-producing tool or an executable
 _ATTRS = {
     "outs": attr.output_list(),
     "args": attr.string_list(mandatory = True),
-    "data": attr.label_list(allow_files = True, aspects = [module_mappings_aspect]),
+    "data": attr.label_list(allow_files = True, aspects = [module_mappings_aspect, node_modules_aspect]),
     "output_dir": attr.bool(),
     "tool": attr.label(
         executable = True,
@@ -23,14 +24,25 @@ def _expand_location(ctx, s):
     s = s.replace("$@", "/".join([ctx.bin_dir.path, ctx.label.package, ctx.attr.name]))
     return ctx.expand_location(s, targets = ctx.attr.data)
 
+def _inputs(ctx):
+    inputs_depsets = []
+
+    # Also include files from npm fine grained deps as inputs.
+    # These deps are identified by the NodeModuleInfo provider.
+    for d in ctx.attr.data:
+        if NodeModuleInfo in d:
+            # Note: we can't avoid calling .to_list() on sources
+            inputs_depsets.append(d[NodeModuleInfo].sources)
+    return depset(ctx.files.data, transitive = inputs_depsets).to_list()
+
 def _impl(ctx):
     if ctx.attr.output_dir and ctx.attr.outs:
         fail("Only one of output_dir and outs may be specified")
     if not ctx.attr.output_dir and not ctx.attr.outs:
         fail("One of output_dir and outs must be specified")
 
     args = ctx.actions.args()
-    inputs = ctx.files.data[:]
+    inputs = _inputs(ctx)
     outputs = []
     if ctx.attr.output_dir:
         outputs = [ctx.actions.declare_directory(ctx.attr.name)]

internal/npm_install/generate_build_file.ts

@@ -882,15 +882,15 @@ function printPackage(pkg: Dep) {
   const dtsSources = filterFiles(pkg._files, ['.d.ts']);
   // TODO(gmagolan): add UMD & AMD scripts to scripts even if not an APF package _but_ only if they
   // are named?
-  const scripts = getNgApfScripts(pkg);
+  const namedSources = getNgApfScripts(pkg);
   const deps = [pkg].concat(pkg._dependencies.filter(dep => dep !== pkg && !dep._isNested));
 
-  let scriptStarlark = '';
-  if (scripts.length) {
-    scriptStarlark = `
+  let namedSourcesStarlark = '';
+  if (namedSources.length) {
+    namedSourcesStarlark = `
     # subset of srcs that are javascript named-UMD or named-AMD scripts
-    scripts = [
-        ${scripts.map((f: string) => `"//:node_modules/${pkg._dir}/${f}",`).join('\n        ')}
+    named_sources = [
+        ${namedSources.map((f: string) => `"//:node_modules/${pkg._dir}/${f}",`).join('\n        ')}
     ],`;
   }
 
@@ -942,7 +942,7 @@ node_module_library(
 # circular dependencies errors
 node_module_library(
     name = "${pkg._name}__contents",
-    srcs = [":${pkg._name}__files"],${scriptStarlark}
+    srcs = [":${pkg._name}__files"],${namedSourcesStarlark}
 )
 
 # ${pkg._name}__typings is the subset of ${pkg._name}__contents that are declarations