feat(typescript): add JSNamedModuleInfo provider to ts_library outputs (#1215)

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.

Some related non-breaking changes:

* node_module_library scripts attribute renamed to named_module_srcs. node_module_library is currently not part of the public API and in user code the npm targets are generated by generate_build_file.ts by yarn_install & npm_install.

BREAKING CHANGES:

The following breaking changes are from internal details so they should not affect most users. Some some downstream projects, however, such as Angular rely on these internal details 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 NpmPackageInfo; sources in the removed scripts field are provided by the JSNamedModuleInfo provider which node_module_library now provides
* collect_node_modules_aspect renamed to just node_modules_aspect

fix: nodejs_binary doesn't need declaration files

Author: gregmagolan

WORKSPACE

@@ -67,11 +67,10 @@ yarn_install(
     name = "npm",
     # The @npm//:node_modules_filegroup generated by manual_build_file_contents
     # is used in the //packages/typescript/test/reference_types_directive:tsconfig_types
-    # test. For now we're still supporting node_modules as a filegroup tho this
-    # may change in the future. The default generated //:node_modules target is a
-    # node_module_library rule which provides NodeModuleInfo and NodeModuleSources
-    # but that rule is not yet in the public API and we have not yet dropped support
-    # for filegroup based node_modules target.
+    # test. For now we're still supporting node_modules as a filegroup tho this may
+    # change in the future. The default generated //:node_modules target is a node_module_library
+    # rule which provides NpmPackageInfo but that rule is not yet in the public API and we
+    # have not yet dropped support for filegroup based node_modules target.
     manual_build_file_contents = """
 filegroup(
   name = "node_modules_filegroup",

examples/angular/patches/@angular+bazel+9.0.0-next.8.patch

@@ -1,16 +1,30 @@
-diff --git a/node_modules/@angular/bazel/src/esm5.bzl b/node_modules/@angular/bazel/src/esm5.bzl
-index 667f601..7907c12 100755
---- a/node_modules/@angular/bazel/src/esm5.bzl
-+++ b/node_modules/@angular/bazel/src/esm5.bzl
-@@ -30,7 +30,7 @@ ESM5Info = provider(
+diff --git a/node_modules/@angular/bazel/src/external.bzl b/node_modules/@angular/bazel/src/external.bzl
+index 4959f22..43f1c96 100755
+--- a/node_modules/@angular/bazel/src/external.bzl
++++ b/node_modules/@angular/bazel/src/external.bzl
+@@ -14,17 +14,17 @@ load(
+     _ts_providers_dict_to_struct = "ts_providers_dict_to_struct",
+ )
+ load(
+-    "@build_bazel_rules_nodejs//internal/common:node_module_info.bzl",
+-    _NodeModuleSources = "NodeModuleSources",
+-    _collect_node_modules_aspect = "collect_node_modules_aspect",
++    "@build_bazel_rules_nodejs//internal/common:npm_package_info.bzl",
++    _NpmPackageInfo = "NpmPackageInfo",
++    _node_modules_aspect = "node_modules_aspect",
+ )
+ load(
+     "@npm_bazel_typescript//internal:ts_config.bzl",
+     _TsConfigInfo = "TsConfigInfo",
  )
 
- def _map_closure_path(file):
--    result = file.short_path[:-len(".closure.js")]
-+    result = file.short_path[:-len(".mjs")]
+-NodeModuleSources = _NodeModuleSources
+-collect_node_modules_aspect = _collect_node_modules_aspect
++NpmPackageInfo = _NpmPackageInfo
++node_modules_aspect = _node_modules_aspect
 
-     # short_path is meant to be used when accessing runfiles in a binary, where
-     # the CWD is inside the current repo. Therefore files in external repo have a
+ tsc_wrapped_tsconfig = _tsc_wrapped_tsconfig
+ COMMON_ATTRIBUTES = _COMMON_ATTRIBUTES
 diff --git a/node_modules/@angular/bazel/src/modify_tsconfig.js b/node_modules/@angular/bazel/src/modify_tsconfig.js
 index 9d3c491..3c99746 100755
 --- a/node_modules/@angular/bazel/src/modify_tsconfig.js
@@ -25,14 +39,23 @@ index 9d3c491..3c99746 100755
    fs.writeFileSync(output, JSON.stringify(data));
  }
 diff --git a/node_modules/@angular/bazel/src/ng_module.bzl b/node_modules/@angular/bazel/src/ng_module.bzl
-index 9b88fbb..68217d0 100755
+index 9b88fbb..54bf2c7 100755
 --- a/node_modules/@angular/bazel/src/ng_module.bzl
 +++ b/node_modules/@angular/bazel/src/ng_module.bzl
-@@ -20,6 +20,7 @@ load(
+@@ -13,13 +13,14 @@ load(
+     "DEFAULT_NG_COMPILER",
+     "DEFAULT_NG_XI18N",
+     "DEPS_ASPECTS",
+-    "NodeModuleSources",
++    "NpmPackageInfo",
+     "TsConfigInfo",
+-    "collect_node_modules_aspect",
++    "node_modules_aspect",
+     "compile_ts",
      "ts_providers_dict_to_struct",
      "tsc_wrapped_tsconfig",
  )
-+load("@build_bazel_rules_nodejs//:providers.bzl", "transitive_js_ecma_script_module_info")
++load("@build_bazel_rules_nodejs//:providers.bzl", "js_ecma_script_module_info", "js_named_module_info")
 
  _FLAT_DTS_FILE_SUFFIX = ".bundle.d.ts"
  _R3_SYMBOLS_DTS_FILE = "src/r3_symbols.d.ts"
@@ -54,22 +77,96 @@ index 9b88fbb..68217d0 100755
          bundle_index_typings = ctx.actions.declare_file("%s.d.ts" % flat_module_out)
          declaration_files.append(bundle_index_typings)
          if is_legacy_ngc:
-@@ -603,7 +604,17 @@ def ng_module_impl(ctx, ts_compile_actions):
+@@ -517,11 +518,11 @@ def _compile_action(ctx, inputs, outputs, dts_bundles_out, messages_out, tsconfi
+             file_inputs += ctx.attr.tsconfig[TsConfigInfo].deps
+ 
+     # Also include files from npm fine grained deps as action_inputs.
+-    # These deps are identified by the NodeModuleSources provider.
++    # These deps are identified by the NpmPackageInfo provider.
+     for d in ctx.attr.deps:
+-        if NodeModuleSources in d:
++        if NpmPackageInfo in d:
+             # Note: we can't avoid calling .to_list() on sources
+-            file_inputs.extend(_filter_ts_inputs(d[NodeModuleSources].sources.to_list()))
++            file_inputs.extend(_filter_ts_inputs(d[NpmPackageInfo].sources.to_list()))
+ 
+     # Collect the inputs and summary files from our deps
+     action_inputs = depset(
+@@ -603,9 +604,23 @@ def ng_module_impl(ctx, ts_compile_actions):
      return providers
 
  def _ng_module_impl(ctx):
 -    return ts_providers_dict_to_struct(ng_module_impl(ctx, compile_ts))
+-
+-local_deps_aspects = [collect_node_modules_aspect, _collect_summaries_aspect]
 +    ts_providers = ng_module_impl(ctx, compile_ts)
 +
 +    # Add in new JS providers
 +    ts_providers["providers"].extend([
-+        transitive_js_ecma_script_module_info(
++        js_named_module_info(
++            sources = ts_providers["typescript"]["es5_sources"],
++            deps = ctx.attr.deps,
++        ),
++        js_ecma_script_module_info(
 +            sources = ts_providers["typescript"]["es6_sources"],
 +            deps = ctx.attr.deps,
 +        ),
 +    ])
 +
 +    return ts_providers_dict_to_struct(ts_providers)
++
++local_deps_aspects = [node_modules_aspect, _collect_summaries_aspect]
+ 
+ # Workaround skydoc bug which assumes DEPS_ASPECTS is a str type
+ [local_deps_aspects.append(a) for a in DEPS_ASPECTS]
+diff --git a/node_modules/@angular/bazel/src/ng_package/ng_package.bzl b/node_modules/@angular/bazel/src/ng_package/ng_package.bzl
+index 48d7619..ea635bb 100755
+--- a/node_modules/@angular/bazel/src/ng_package/ng_package.bzl
++++ b/node_modules/@angular/bazel/src/ng_package/ng_package.bzl
+@@ -13,9 +13,8 @@ It packages your library following the Angular Package Format, see the
+ specification of this format at https://goo.gl/jB3GVv
+ """
+ 
+-load("@build_bazel_rules_nodejs//internal/common:collect_es6_sources.bzl", "collect_es6_sources")
+-load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleSources")
+-load("@build_bazel_rules_nodejs//internal/common:sources_aspect.bzl", "sources_aspect")
++load("@build_bazel_rules_nodejs//:providers.bzl", "JSEcmaScriptModuleInfo", "JSNamedModuleInfo")
++load("@build_bazel_rules_nodejs//internal/common:npm_package_info.bzl", "NpmPackageInfo")
+ load(
+     "@build_bazel_rules_nodejs//internal/rollup:rollup_bundle.bzl",
+     "ROLLUP_ATTRS",
+@@ -207,7 +206,12 @@ def _filter_js_inputs(all_inputs):
+ def _ng_package_impl(ctx):
+     npm_package_directory = ctx.actions.declare_directory("%s.ng_pkg" % ctx.label.name)
+ 
+-    esm_2015_files = _filter_out_generated_files(collect_es6_sources(ctx), "js")
++    esm_2015_files = []
++    for dep in ctx.attr.deps:
++        if JSEcmaScriptModuleInfo in dep:
++            esm_2015_files += dep[JSEcmaScriptModuleInfo].sources.to_list()
++
++    esm_2015_files = _filter_out_generated_files(esm_2015_files, "js")
+     esm5_sources = _filter_out_generated_files(flatten_esm5(ctx), "js")
+ 
+     # These accumulators match the directory names where the files live in the
+@@ -347,9 +351,9 @@ def _ng_package_impl(ctx):
+         node_modules_files = _filter_js_inputs(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 NpmPackageInfo provider.
+         for d in ctx.attr.deps:
+-            if NodeModuleSources in d:
++            if NpmPackageInfo in d:
+                 node_modules_files += _filter_js_inputs(d.files)
+         esm5_rollup_inputs = depset(node_modules_files, transitive = [esm5_sources])
+ 
+@@ -469,7 +473,7 @@ def _ng_package_impl(ctx):
+         files = depset([package_dir]),
+     )]
 
- local_deps_aspects = [collect_node_modules_aspect, _collect_summaries_aspect]
+-DEPS_ASPECTS = [esm5_outputs_aspect, sources_aspect]
++DEPS_ASPECTS = [esm5_outputs_aspect]
 
+ # Workaround skydoc bug which assumes ROLLUP_DEPS_ASPECTS is a str type
+ [DEPS_ASPECTS.append(a) for a in ROLLUP_DEPS_ASPECTS]

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,27 @@ 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)]
 
 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

@@ -0,0 +1,52 @@
+# Copyright 2017 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""NpmPackageInfo providers and apsect to collect node_modules from deps.
+"""
+
+# NpmPackageInfo 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 `node_modules_aspect` below.
+NpmPackageInfo = provider(
+    doc = "Provides information about npm dependencies",
+    fields = {
+        "direct_sources": "Depset of direct source files in this npm package",
+        "sources": "Depset of direct & transitive source files in this npm package and in its dependencies",
+        "workspace": "The workspace name that this npm package is provided from",
+    },
+)
+
+def _node_modules_aspect_impl(target, ctx):
+    providers = []
+
+    # provide NpmPackageInfo if it is not already provided there are NpmPackageInfo deps
+    if not NpmPackageInfo in target:
+        sources_depsets = []
+        workspace = None
+        if hasattr(ctx.rule.attr, "deps"):
+            for dep in ctx.rule.attr.deps:
+                if NpmPackageInfo in dep:
+                    if workspace and dep[NpmPackageInfo].workspace != workspace:
+                        fail("All npm dependencies need to come from a single workspace. Found '%s' and '%s'." % (workspace, dep[NpmPackageInfo].workspace))
+                    workspace = dep[NpmPackageInfo].workspace
+                    sources_depsets.append(dep[NpmPackageInfo].sources)
+            if workspace:
+                providers.extend([NpmPackageInfo(direct_sources = depset(), sources = depset(transitive = sources_depsets), workspace = workspace)])
+
+    return providers
+
+node_modules_aspect = aspect(
+    _node_modules_aspect_impl,
+    attr_aspects = ["deps"],
+)