refactor: remove JS_LIBRARY_DATA_ATTR and DOWNSTREAM_LINKED_NPM_DEPS_…

…DOCSTRING from js_helpers.bzl It is akward exporting attributes & docstrings. Downstream rules can instead refer to js_library docstrings.
aspect-build · May 20, 2024 · 9cb0241 · 9cb0241
1 parent dead969
commit 9cb0241
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 40 deletions.
diff --git a/docs/js_library.md b/docs/js_library.md
diff --git a/js/libs.bzl b/js/libs.bzl
@@ -10,8 +10,6 @@ load(
 )
 load(
     "//js/private:js_helpers.bzl",
-    _DOWNSTREAM_LINKED_NPM_DEPS_DOCSTRING = "DOWNSTREAM_LINKED_NPM_DEPS_DOCSTRING",
-    _JS_LIBRARY_DATA_ATTR = "JS_LIBRARY_DATA_ATTR",
     _LOG_LEVELS = "LOG_LEVELS",
     _envs_for_log_level = "envs_for_log_level",
     _gather_files_from_js_info = "gather_files_from_js_info",
@@ -33,8 +31,6 @@ js_lib_helpers = struct(
     gather_runfiles = _gather_runfiles,
     gather_transitive_types = _gather_transitive_types,
     gather_transitive_sources = _gather_transitive_sources,
-    JS_LIBRARY_DATA_ATTR = _JS_LIBRARY_DATA_ATTR,
-    DOWNSTREAM_LINKED_NPM_DEPS_DOCSTRING = _DOWNSTREAM_LINKED_NPM_DEPS_DOCSTRING,
 )
 
 js_lib_constants = struct(

diff --git a/js/private/js_helpers.bzl b/js/private/js_helpers.bzl
@@ -5,36 +5,6 @@ load("@aspect_bazel_lib//lib:copy_to_bin.bzl", "copy_file_to_bin_action")
 load("//npm:providers.bzl", "NpmPackageStoreInfo")
 load(":js_info.bzl", "JsInfo")
 
-DOWNSTREAM_LINKED_NPM_DEPS_DOCSTRING = """If this list contains linked npm packages, npm package store targets or other targets that provide
-`JsInfo`, `NpmPackageStoreInfo` providers are gathered from `JsInfo`. This is done directly from
-the `npm_package_store_infos` field of these. For linked npm package targets, the underlying
-`npm_package_store` target(s) that back the links is used. Gathered `NpmPackageStoreInfo`
-providers are propagated to the direct dependencies of downstream linked `npm_package` targets.
-
-NB: Linked npm package targets that are "dev" dependencies do not forward their underlying
-`npm_package_store` target(s) through `npm_package_store_infos` and will therefore not be
-propagated to the direct dependencies of downstream linked `npm_package` targets. npm packages
-that come in from `npm_translate_lock` are considered "dev" dependencies if they are have
-`dev: true` set in the pnpm lock file. This should be all packages that are only listed as
-"devDependencies" in all `package.json` files within the pnpm workspace. This behavior is
-intentional to mimic how `devDependencies` work in published npm packages.
-"""
-
-# This attribute is exposed in //js:libs.bzl so that downstream build rules can use it
-JS_LIBRARY_DATA_ATTR = attr.label_list(
-    doc = """Runtime dependencies to include in binaries/tests that depend on this target.
-
-The transitive npm dependencies, transitive sources, default outputs and runfiles of targets in the `data` attribute
-are added to the runfiles of this target. They should appear in the '*.runfiles' area of any executable which has
-a runtime dependency on this target.
-
-{downstream_linked_npm_deps}
-""".format(
-        downstream_linked_npm_deps = DOWNSTREAM_LINKED_NPM_DEPS_DOCSTRING,
-    ),
-    allow_files = True,
-)
-
 def gather_transitive_sources(sources, targets):
     """Gathers transitive sources from a list of direct sources and targets
 

diff --git a/js/private/js_library.bzl b/js/private/js_library.bzl
@@ -24,7 +24,7 @@ js_library(
 """
 
 load(":js_info.bzl", "JsInfo", "js_info")
-load(":js_helpers.bzl", "DOWNSTREAM_LINKED_NPM_DEPS_DOCSTRING", "JS_LIBRARY_DATA_ATTR", "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_npm_package_store_infos", "gather_npm_sources", "gather_runfiles", "gather_transitive_sources", "gather_transitive_types")
 load("@aspect_bazel_lib//lib:copy_to_bin.bzl", "COPY_FILE_TO_BIN_TOOLCHAINS")
 
 _DOC = """A library of JavaScript sources. Provides JsInfo, the primary provider used in rules_js
@@ -42,6 +42,21 @@ NB: `js_library` copies all source files to the output tree before providing the
 https://github.com/aspect-build/rules_js/tree/dbb5af0d2a9a2bb50e4cf4a96dbc582b27567155/docs#javascript
 for more context on why we do this."""
 
+_LINKED_NPM_DEPS_DOCSTRING = """If this list contains linked npm packages, npm package store targets or other targets that provide
+`JsInfo`, `NpmPackageStoreInfo` providers are gathered from `JsInfo`. This is done directly from
+the `npm_package_store_infos` field of these. For linked npm package targets, the underlying
+`npm_package_store` target(s) that back the links are used. Gathered `NpmPackageStoreInfo`
+providers are propagated to the direct dependencies of downstream linked targets.
+
+NB: Linked npm package targets that are "dev" dependencies do not forward their underlying
+`npm_package_store` target(s) through `npm_package_store_infos` and will therefore not be
+propagated to the direct dependencies of downstream linked targets. npm packages
+that come in from `npm_translate_lock` are considered "dev" dependencies if they are have
+`dev: true` set in the pnpm lock file. This should be all packages that are only listed as
+"devDependencies" in all `package.json` files within the pnpm workspace. This behavior is
+intentional to mimic how `devDependencies` work in published npm packages.
+"""
+
 _ATTRS = {
     "srcs": attr.label_list(
         doc = """Source files that are included in this library.
@@ -83,11 +98,23 @@ The transitive npm dependencies, transitive sources & runfiles of targets in the
 runfiles of this target. They should appear in the '*.runfiles' area of any executable which is output by or has a
 runtime dependency on this target.
 
-{downstream_linked_npm_deps}
-""".format(downstream_linked_npm_deps = DOWNSTREAM_LINKED_NPM_DEPS_DOCSTRING),
+{linked_npm_deps}
+""".format(linked_npm_deps = _LINKED_NPM_DEPS_DOCSTRING),
         providers = [JsInfo],
     ),
-    "data": JS_LIBRARY_DATA_ATTR,
+    "data": attr.label_list(
+        doc = """Runtime dependencies to include in binaries/tests that depend on this target.
+
+The transitive npm dependencies, transitive sources, default outputs and runfiles of targets in the `data` attribute
+are added to the runfiles of this target. They should appear in the '*.runfiles' area of any executable which has
+a runtime dependency on this target.
+
+{linked_npm_deps}
+""".format(
+            linked_npm_deps = _LINKED_NPM_DEPS_DOCSTRING,
+        ),
+        allow_files = True,
+    ),
     "no_copy_to_bin": attr.label_list(
         allow_files = True,
         doc = """List of files to not copy to the Bazel output tree when `copy_data_to_bin` is True.