Move scripts to NodeModuleSources provider & remove ScriptsProvider

internal/common/node_module_info.bzl

@@ -25,6 +25,7 @@ NodeModuleInfo = provider(
 NodeModuleSources = provider(
     doc = "Provides sources for npm dependencies installed with yarn_install and npm_install rules",
     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",
         "workspace": "The workspace name that the npm dependencies are provided from",
     },

internal/common/sources_aspect.bzl

@@ -15,8 +15,7 @@
 """Aspect to collect es5 js sources and scripts from deps.
 """
 
-load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleInfo")
-load("@build_bazel_rules_nodejs//internal/common:providers.bzl", "ScriptsProvider")
+load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleInfo", "NodeModuleSources")
 
 def _sources_aspect_impl(target, ctx):
     # TODO(kyliau): node_sources here is a misnomer because it implies that
@@ -27,17 +26,15 @@ def _sources_aspect_impl(target, ctx):
     node_sources = depset()
 
     # dev_scripts is a collection of "scripts" from "node-module-like" targets
-    # such as `node_module_library`. Note that nothing is collected from the default
-    # filegroup target for generic node modules because it does not have the
-    # `scripts` provider nor does it have the `deps` attribute.
+    # such as `node_module_library`
     dev_scripts = depset()
 
     # Note layering: until we have JS interop providers, this needs to know how to
     # get TypeScript outputs.
     if hasattr(target, "typescript"):
         node_sources = depset(transitive = [node_sources, target.typescript.es5_sources])
-    elif ScriptsProvider in target:
-        dev_scripts = depset(transitive = [dev_scripts, target[ScriptsProvider].scripts])
+    elif NodeModuleSources in target:
+        dev_scripts = depset(transitive = [dev_scripts, target[NodeModuleSources].scripts])
     elif hasattr(target, "files") and not NodeModuleInfo in target:
         # Sources from npm fine grained deps should not be included
         node_sources = depset(

internal/npm_install/generate_build_file.js

@@ -631,11 +631,11 @@ function printJson(pkg) {
 }
 
 /**
- * A filter function for files in an npm package.
+ * A filter function for files in an npm package. Comparison is case-insensitive.
  * @param files array of files to filter
- * @param exts list of white listed extensions; if empty, no filter is done on extensions;
- *             '' empty string denotes to allow files with no extensions, other extensions
- *             are listed with '.ext' notation such as '.d.ts'.
+ * @param exts list of white listed case-insensitive extensions; if empty, no filter is
+ *             done on extensions; '' empty string denotes to allow files with no extensions,
+ *             other extensions are listed with '.ext' notation such as '.d.ts'.
  */
 function filterFiles(files, exts = []) {
   // Files with spaces (\x20) or unicode characters (<\x20 && >\x7E) are not allowed in
@@ -647,8 +647,9 @@ function filterFiles(files, exts = []) {
       // include files with no extensions if noExt is true
       if (allowNoExts && !path.extname(f)) return true;
       // filter files in exts
+      const lc = f.toLowerCase();
       for (const e of exts) {
-        if (e && f.endsWith(e)) {
+        if (e && lc.endsWith(e.toLowerCase())) {
           return true;
         }
       }
@@ -677,12 +678,26 @@ function isNgApfPackage(pkg) {
   });
 }
 
+/**
+ * If the package is in the Angular package format returns list
+ * of package files that end with `.umd.js`, `.ngfactory.js` and `.ngsummary.js`.
+ */
+function getNgApfScripts(pkg) {
+  return isNgApfPackage(pkg) ?
+      filterFiles(pkg._files, ['.umd.js', '.ngfactory.js', '.ngsummary.js']) :
+      [];
+}
+
 /**
  * Given a pkg, return the skylark `node_module_library` targets for the package.
  */
 function printPackage(pkg) {
   const sources = filterFiles(pkg._files, INCLUDED_FILES);
   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 pkgDeps = pkg._dependencies.filter(dep => dep !== pkg && !dep._isNested);
 
   let result = `load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")
@@ -702,15 +717,18 @@ node_module_library(
         ${
       pkgDeps.map(dep => `"//node_modules/${dep._dir}:${dep._name}__files",`).join('\n        ')}
     ],
-    is_apf = ${isNgApfPackage(pkg) ? 'True' : 'False'},
 )
 
+# ${pkg._name}__files target is used as dep for other package targets to prevent
+# circular dependencies errors
 node_module_library(
     name = "${pkg._name}__files",
     srcs = [
         ${sources.map(f => `":${f}",`).join('\n        ')}
     ],
-    is_apf = ${isNgApfPackage(pkg) ? 'True' : 'False'},
+    scripts = [
+        ${scripts.map(f => `":${f}",`).join('\n        ')}
+    ],
 )
 
 node_module_library(

internal/npm_install/node_module_library.bzl

@@ -13,27 +13,15 @@
 # limitations under the License.
 
 load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleInfo", "NodeModuleSources")
-load("@build_bazel_rules_nodejs//internal/common:providers.bzl", "ScriptsProvider")
 
 def _node_module_library_impl(ctx):
-    apf_umds = []
-    apf_factories = []
-    apf_summaries = []
-
-    # If this npm package is in the Angular package format then collect
-    # umd bundles, ngfactory files & ngsummary files and provide them
-    # via the ScriptsProvider
-    if ctx.attr.is_apf:
-        for file in ctx.files.srcs:
-            if file.basename.endswith(".umd.js"):
-                apf_umds.append(file)
-            elif file.basename.endswith(".ngfactory.js"):
-                apf_factories.append(file)
-            elif file.basename.endswith(".ngsummary.js"):
-                apf_summaries.append(file)
-
-    sources = depset(transitive = [src.files for src in ctx.attr.srcs] + [dep.files for dep in ctx.attr.deps])
     workspace = ctx.label.workspace_root.split("/")[1] if ctx.label.workspace_root else ctx.workspace_name
+    sources = depset(ctx.files.srcs, transitive = [dep.files for dep in ctx.attr.deps])
+
+    scripts = depset()
+    for dep in ctx.attr.scripts:
+        scripts = depset(transitive = [scripts, dep.files])
+    scripts = depset(ctx.files.scripts, transitive = [scripts])
 
     return [
         DefaultInfo(
@@ -44,11 +32,9 @@ def _node_module_library_impl(ctx):
         ),
         NodeModuleSources(
             sources = sources,
+            scripts = scripts,
             workspace = workspace,
         ),
-        ScriptsProvider(
-            scripts = depset(apf_umds + apf_factories + apf_summaries),
-        ),
     ]
 
 node_module_library = rule(
@@ -58,9 +44,9 @@ node_module_library = rule(
             doc = "The list of files that comprise the package",
             allow_files = True,
         ),
-        "is_apf": attr.bool(
-            default = False,
-            doc = "True if this npm package is in the Angular package format",
+        "scripts": attr.label_list(
+            doc = "A subset of srcs that are javascript named-UMD or named-AMD scripts for use in rules such as ts_devserver",
+            allow_files = True,
         ),
         "deps": attr.label_list(
             doc = "Transitive dependencies of the package",