perf: change js_helpers gather_* utils to accept list, not depset (#1787

)

js/private/js_binary.bzl

@@ -525,10 +525,8 @@ def _create_launcher(ctx, log_prefix_rule_set, log_prefix_rule, fixed_args = [],
 
     runfiles = gather_runfiles(
         ctx = ctx,
-        sources = [],
         data = ctx.attr.data,
         data_files = [entry_point] + ctx.files.data,
-        deps = [],
         copy_data_files_to_bin = ctx.attr.copy_data_to_bin,
         no_copy_to_bin = ctx.files.no_copy_to_bin,
         include_sources = ctx.attr.include_sources,

js/private/js_helpers.bzl

@@ -8,39 +8,35 @@ def gather_transitive_sources(sources, targets):
     """Gathers transitive sources from a list of direct sources and targets
 
     Args:
-        sources: list or depset of direct sources which should be included in `transitive_sources`
+        sources: list of direct sources which should be included in `transitive_sources`
         targets: list of targets to gather `transitive_sources` from `JsInfo`
 
     Returns:
         A depset of transitive sources
     """
-    if type(sources) == "list":
-        sources = depset(sources)
     transitive = [
         target[JsInfo].transitive_sources
         for target in targets
         if JsInfo in target
     ]
-    return depset(transitive = [sources] + transitive)
+    return depset(sources, transitive = transitive)
 
 def gather_transitive_types(types, targets):
     """Gathers transitive types from a list of direct types and targets
 
     Args:
-        types: list or depset of direct sources which should be included in `transitive_types`
+        types: list of direct sources which should be included in `transitive_types`
         targets: list of targets to gather `transitive_types` from `JsInfo`
 
     Returns:
         A depset of transitive sources
     """
-    if type(types) == "list":
-        types = depset(types)
     transitive = [
         target[JsInfo].transitive_types
         for target in targets
         if JsInfo in target
     ]
-    return depset(transitive = [types] + transitive)
+    return depset(types, transitive = transitive)
 
 def gather_npm_sources(srcs, deps):
     """Gathers npm sources from a list of srcs and deps targets
@@ -116,9 +112,9 @@ this option is not needed.
 
 def gather_runfiles(
         ctx,
-        sources,
-        data,
-        deps,
+        sources = None,
+        data = [],
+        deps = [],
         data_files = [],
         copy_data_files_to_bin = False,
         no_copy_to_bin = [],
@@ -138,7 +134,7 @@ def gather_runfiles(
     Args:
         ctx: the rule context
 
-        sources: list or depset of files which should be included in runfiles
+        sources: depset which should be included in runfiles
 
         deps: list of dependency targets; only transitive runfiles are gather from these targets
 
@@ -176,10 +172,11 @@ def gather_runfiles(
         A [runfiles](https://bazel.build/rules/lib/runfiles) object created with [ctx.runfiles](https://bazel.build/rules/lib/ctx#runfiles).
     """
 
+    transitive_files_depsets = []
+
     # Includes sources
-    if type(sources) == "list":
-        sources = depset(sources)
-    transitive_files_depsets = [sources]
+    if sources:
+        transitive_files_depsets.append(sources)
 
     # Gather the default outputs of data targets
     for target in data:

js/private/js_library.bzl

@@ -24,7 +24,7 @@ js_library(
 """
 
 load("@aspect_bazel_lib//lib:copy_to_bin.bzl", "COPY_FILE_TO_BIN_TOOLCHAINS")
-load(":js_helpers.bzl", "copy_js_file_to_bin_action", "gather_npm_package_store_infos", "gather_npm_sources", "gather_runfiles", "gather_transitive_sources", "gather_transitive_types")
+load(":js_helpers.bzl", "copy_js_file_to_bin_action", "gather_runfiles")
 load(":js_info.bzl", "JsInfo", "js_info")
 
 _DOC = """A library of JavaScript sources. Provides JsInfo, the primary provider used in rules_js
@@ -204,33 +204,45 @@ def _js_library_impl(ctx):
         files = ctx.files.types,
     )
 
+    # Direct sources and types
     sources = depset(transitive = [sources, additional_sources])
     types = depset(transitive = [types, additional_sources, additional_types])
 
-    transitive_sources = gather_transitive_sources(
-        sources = sources,
-        targets = ctx.attr.srcs + ctx.attr.types + ctx.attr.deps,
-    )
+    # Transitive sources and types
+    transitive_sources = [sources]
+    transitive_types = [types]
 
-    transitive_types = gather_transitive_types(
-        types = types,
-        targets = ctx.attr.srcs + ctx.attr.types + ctx.attr.deps,
-    )
+    # npm providers
+    npm_sources = []
+    npm_package_store_infos = []
 
-    npm_sources = gather_npm_sources(
-        srcs = ctx.attr.srcs + ctx.attr.types,
-        deps = ctx.attr.deps,
-    )
+    # Concat the srcs+types+deps once
+    srcs_types_deps = ctx.attr.srcs + ctx.attr.types + ctx.attr.deps
 
-    npm_package_store_infos = gather_npm_package_store_infos(
-        targets = ctx.attr.srcs + ctx.attr.data + ctx.attr.deps,
-    )
+    # Collect transitive sources, types and npm providers from srcs+types+deps
+    for target in srcs_types_deps:
+        if JsInfo in target:
+            jsinfo = target[JsInfo]
+            transitive_sources.append(jsinfo.transitive_sources)
+            transitive_types.append(jsinfo.transitive_types)
+            npm_sources.append(jsinfo.npm_sources)
+            npm_package_store_infos.append(jsinfo.npm_package_store_infos)
+
+    # Also add npm_package_store_infos from ctx.attr.data
+    for target in ctx.attr.data:
+        if JsInfo in target:
+            npm_package_store_infos.append(target[JsInfo].npm_package_store_infos)
+
+    transitive_sources = depset(transitive = transitive_sources)
+    transitive_types = depset(transitive = transitive_types)
+    npm_sources = depset(transitive = npm_sources)
+    npm_package_store_infos = depset(transitive = npm_package_store_infos)
 
     runfiles = gather_runfiles(
         ctx = ctx,
         sources = transitive_sources,
         data = ctx.attr.data,
-        deps = ctx.attr.srcs + ctx.attr.types + ctx.attr.deps,
+        deps = srcs_types_deps,
         data_files = ctx.files.data,
         copy_data_files_to_bin = ctx.attr.copy_data_to_bin,
         no_copy_to_bin = ctx.files.no_copy_to_bin,

npm/private/npm_package.bzl

@@ -16,7 +16,6 @@ load("@bazel_skylib//lib:dicts.bzl", "dicts")
 load("@bazel_skylib//lib:versions.bzl", "versions")
 load("//js:defs.bzl", "js_binary")
 load("//js:libs.bzl", "js_lib_helpers")
-load("//js:providers.bzl", "JsInfo")
 load(":npm_package_info.bzl", "NpmPackageInfo")
 
 # Pull in all copy_to_directory attributes except for exclude_prefixes
@@ -70,14 +69,9 @@ def _npm_package_impl(ctx):
     dst = ctx.actions.declare_directory(ctx.attr.out if ctx.attr.out else ctx.attr.name)
 
     # forward all npm_package_store_infos
-    npm_package_store_infos = [
-        target[JsInfo].npm_package_store_infos
-        for target in ctx.attr.srcs
-        if JsInfo in target
-    ]
-    npm_package_store_infos.append(js_lib_helpers.gather_npm_package_store_infos(
-        targets = ctx.attr.data,
-    ))
+    npm_package_store_infos = js_lib_helpers.gather_npm_package_store_infos(
+        targets = ctx.attr.srcs + ctx.attr.data,
+    )
 
     copy_to_directory_bin_action(
         ctx,
@@ -109,7 +103,7 @@ def _npm_package_impl(ctx):
             package = ctx.attr.package,
             version = ctx.attr.version,
             src = dst,
-            npm_package_store_infos = depset(transitive = npm_package_store_infos),
+            npm_package_store_infos = npm_package_store_infos,
         ),
     ]