refactor: distribute esbuild_repositories function in the built-in, n…

…ot the @bazel/esbuild package

Avoids eager fetching of the npm_install/yarn_install by not load'ing from the @npm package

BUILD.bazel

@@ -53,6 +53,7 @@ bzl_library(
         "//internal/pkg_npm:bzl",
         "//internal/pkg_web:bzl",
         "//internal/providers:bzl",
+        "//toolchains/esbuild:bzl",
         "//toolchains/node:bzl",
     ],
 )
@@ -69,9 +70,13 @@ codeowners(
 
 pkg_npm(
     name = "rules_nodejs_package",
-    srcs = glob(["*.bzl"]) + [
+    srcs = [
         "BUILD.bazel",
         "LICENSE",
+        "index.bzl",
+        "package.bzl",
+        "providers.bzl",
+        "version.bzl",
     ],
     substitutions = COMMON_REPLACEMENTS,
     deps = [
@@ -104,6 +109,7 @@ pkg_npm(
         "//third_party/npm/node_modules/browserify:package_contents",
         "//third_party/npm/node_modules/ieee754:package_contents",
         "//third_party/npm/node_modules/named-amd:package_contents",
+        "//toolchains/esbuild:package_contents",
         "//toolchains/node:package_contents",
     ],
 )

WORKSPACE

@@ -131,7 +131,7 @@ browser_repositories(
 )
 
 # Setup esbuild dependencies
-load("//packages/esbuild:esbuild_repositories.bzl", "esbuild_repositories")
+load("//toolchains/esbuild:esbuild_repositories.bzl", "esbuild_repositories")
 
 esbuild_repositories()
 

docs/esbuild.md

@@ -22,31 +22,30 @@ or using yarn
 yarn add -D @bazel/esbuild
 ```
 
-The esbuild binary is fetched from npm automatically and exposed via toolchains. Add the `esbuild_repositories` rule to the `WORKSPACE`:
+The esbuild binary is fetched automatically for your platform and is exposed via Bazel toolchains.
+To do this, add the `esbuild_repositories` rule to your `WORKSPACE`.
+You'll need to point it to the repository created by npm_install or yarn_install where the `@bazel/esbuild`
+package is fetched. (Typically, this is `npm`).
+Set the `npm_repository` attribute to the name of that repository.
 
 ```python
-load("@npm//@bazel/esbuild:esbuild_repositories.bzl", "esbuild_repositories")
-
-esbuild_repositories()
-```
-
-As esbuild is being fetched from `npm`, the load statement above can cause eager fetches of the `@npm` external repository.
-To work around this, it's possible to fetch the `@bazel/esbuild` package via an `http_archive`
-
-```python
-http_archive(
-    name = "bazel_esbuild",
-    urls = [
-        "https://registry.npmjs.org/@bazel/esbuild/-/esbuild-4.0.0.tgz",
-    ],
-    strip_prefix = "package",
+npm_install(
+    name = "npm",
+    # @bazel/esbuild is a dependency in this package.json
+    package_json = "//:package.json",
+    package_lock_json = "//:package-lock.json",
 )
 
-load("@bazel_esbuild//:esbuild_repositories.bzl", "esbuild_repositories")
+load("@build_bazel_rules_nodejs//toolchains/esbuild:esbuild_repositories.bzl", "esbuild_repositories")
 
-esbuild_repositories()
+esbuild_repositories(npm_repository = "npm")  # Note, npm is the default value for npm_repository
 ```
 
+> To avoid eagerly fetching all the npm dependencies, this load statement comes from the "Built-in"
+> `@build_bazel_rules_nodejs` repository rather than from `@npm`.
+> In rules_nodejs 5.0 we intend to fix this layering violation by having the whole esbuild support
+> distributed independently of rules_nodejs, and not require any package to be installed from npm.
+
 ## Overview
 
 The `esbuild` rule can take a JS or TS dependency tree and bundle it to a single file, or split across multiple files, outputting a directory. 
@@ -392,7 +391,7 @@ Any other common attributes
 **USAGE**
 
 <pre>
-esbuild_repositories(<a href="#esbuild_repositories-name">name</a>)
+esbuild_repositories(<a href="#esbuild_repositories-name">name</a>, <a href="#esbuild_repositories-npm_repository">npm_repository</a>)
 </pre>
 
 Helper for fetching and setting up the esbuild versions and toolchains
@@ -406,4 +405,11 @@ currently unused
 
 Defaults to `""`
 
+<h4 id="esbuild_repositories-npm_repository">npm_repository</h4>
+
+the name of the repository where the @bazel/esbuild package is installed
+by npm_install or yarn_install.
+
+Defaults to `"npm"`
+
 

examples/esbuild/WORKSPACE

@@ -33,22 +33,6 @@ npm_install(
     package_lock_json = "//:package-lock.json",
 )
 
-# This is the simpler way to load, however it eagerly fetches all of the npm
-# dependencies for every build due to this load statement, whether the build
-# needs them or not.
-# load("@npm//@bazel/esbuild:esbuild_repositories.bzl", "esbuild_repositories")
-
-# We can avoid that eager fetch by loading esbuild_repositories
-# from a separately-fetched npm package.
-http_archive(
-    name = "bazel_esbuild",
-    strip_prefix = "package",
-    urls = [
-        # TODO(alexeagle): how to keep this version up-to-date as we cut releases?
-        "https://registry.npmjs.org/@bazel/esbuild/-/esbuild-4.0.0-rc.0.tgz",
-    ],
-)
-
-load("@bazel_esbuild//:esbuild_repositories.bzl", "esbuild_repositories")
+load("@build_bazel_rules_nodejs//toolchains/esbuild:esbuild_repositories.bzl", "esbuild_repositories")
 
 esbuild_repositories()

internal/node/BUILD.bazel

@@ -52,6 +52,7 @@ filegroup(
     visibility = ["//:__pkg__"],
 )
 
+# BEGIN-INTERNAL
 # To update node_patches.js run:
 #   bazel run //internal/node:checked_in_node_patches.update
 generated_file_test(
@@ -61,3 +62,4 @@ generated_file_test(
     # For some reason rollup produces different, non-matching output with --config=no-runfiles
     tags = ["fix-windows"],
 )
+# END-INTERNAL

packages/esbuild/BUILD.bazel

@@ -31,11 +31,11 @@ bzl_library(
         "@bazel_tools//tools:bzl_srcs",
     ],
     deps = [
-        "//packages/esbuild/toolchain:bzl",
         "@bazel_skylib//lib:paths",
         "@build_bazel_rules_nodejs//:bzl",
         "@build_bazel_rules_nodejs//internal/common:bzl",
         "@build_bazel_rules_nodejs//internal/node:bzl",
+        "@build_bazel_rules_nodejs//toolchains/esbuild:bzl",
     ],
 )
 
@@ -59,8 +59,6 @@ filegroup(
     srcs = [
         "esbuild.bzl",
         "esbuild_config.bzl",
-        "esbuild_packages.bzl",
-        "esbuild_repositories.bzl",
         "helpers.bzl",
         "index.bzl",
         "launcher.js",
@@ -71,15 +69,11 @@ filegroup(
 pkg_npm(
     name = "npm_package",
     package_name = "@bazel/esbuild",
-    srcs = [
-        ":srcs",
-        "//packages/esbuild/toolchain:srcs",
-    ],
+    srcs = [":srcs"],
     build_file_content = """exports_files(["launcher.js"])""",
-    substitutions = dict({
+    substitutions = {
         "@build_bazel_rules_nodejs//packages/esbuild": "//@bazel/esbuild",
-        "package_path = \"packages/esbuild\",": "package_path = \"external/\" + pkg_label.workspace_name + \"/@bazel/esbuild\",",
-    }),
+    },
     deps = [
         ":README.md",
         ":npm_version_check",

packages/esbuild/esbuild.bzl

@@ -6,7 +6,7 @@ load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "params_file")
 load("@build_bazel_rules_nodejs//:providers.bzl", "ExternalNpmPackageInfo", "JSEcmaScriptModuleInfo", "JSModuleInfo", "NODE_CONTEXT_ATTRS", "NodeContextInfo", "node_modules_aspect", "run_node")
 load("@build_bazel_rules_nodejs//internal/linker:link_node_modules.bzl", "MODULE_MAPPINGS_ASPECT_RESULTS_NAME", "module_mappings_aspect")
 load("@build_bazel_rules_nodejs//internal/common:expand_variables.bzl", "expand_variables")
-load("@build_bazel_rules_nodejs//packages/esbuild/toolchain:toolchain.bzl", "TOOLCHAIN")
+load("@build_bazel_rules_nodejs//toolchains/esbuild:toolchain.bzl", "TOOLCHAIN")
 load(":helpers.bzl", "desugar_entry_point_names", "filter_files", "generate_path_mapping", "resolve_entry_point", "write_args_file", "write_jsconfig_file")
 
 def _esbuild_impl(ctx):

packages/esbuild/index.bzl

@@ -24,7 +24,7 @@ load(
     _esbuild_config = "esbuild_config",
 )
 load(
-    "@build_bazel_rules_nodejs//packages/esbuild/toolchain:toolchain.bzl",
+    "@build_bazel_rules_nodejs//toolchains/esbuild:toolchain.bzl",
     _configure_esbuild_toolchain = "configure_esbuild_toolchain",
 )
 

packages/esbuild/index.docs.bzl

@@ -16,31 +16,30 @@ or using yarn
 yarn add -D @bazel/esbuild
 ```
 
-The esbuild binary is fetched from npm automatically and exposed via toolchains. Add the `esbuild_repositories` rule to the `WORKSPACE`:
+The esbuild binary is fetched automatically for your platform and is exposed via Bazel toolchains.
+To do this, add the `esbuild_repositories` rule to your `WORKSPACE`.
+You'll need to point it to the repository created by npm_install or yarn_install where the `@bazel/esbuild`
+package is fetched. (Typically, this is `npm`).
+Set the `npm_repository` attribute to the name of that repository.
 
 ```python
-load("@npm//@bazel/esbuild:esbuild_repositories.bzl", "esbuild_repositories")
-
-esbuild_repositories()
-```
-
-As esbuild is being fetched from `npm`, the load statement above can cause eager fetches of the `@npm` external repository.
-To work around this, it's possible to fetch the `@bazel/esbuild` package via an `http_archive`
-
-```python
-http_archive(
-    name = "bazel_esbuild",
-    urls = [
-        "https://registry.npmjs.org/@bazel/esbuild/-/esbuild-4.0.0.tgz",
-    ],
-    strip_prefix = "package",
+npm_install(
+    name = "npm",
+    # @bazel/esbuild is a dependency in this package.json
+    package_json = "//:package.json",
+    package_lock_json = "//:package-lock.json",
 )
 
-load("@bazel_esbuild//:esbuild_repositories.bzl", "esbuild_repositories")
+load("@build_bazel_rules_nodejs//toolchains/esbuild:esbuild_repositories.bzl", "esbuild_repositories")
 
-esbuild_repositories()
+esbuild_repositories(npm_repository = "npm")  # Note, npm is the default value for npm_repository
 ```
 
+> To avoid eagerly fetching all the npm dependencies, this load statement comes from the "Built-in"
+> `@build_bazel_rules_nodejs` repository rather than from `@npm`.
+> In rules_nodejs 5.0 we intend to fix this layering violation by having the whole esbuild support
+> distributed independently of rules_nodejs, and not require any package to be installed from npm.
+
 ## Overview
 
 The `esbuild` rule can take a JS or TS dependency tree and bundle it to a single file, or split across multiple files, outputting a directory. 
@@ -97,11 +96,11 @@ load(
     _esbuild_config = "esbuild_config",
 )
 load(
-    "@build_bazel_rules_nodejs//packages/esbuild:esbuild_repositories.bzl",
+    "@build_bazel_rules_nodejs//toolchains/esbuild:esbuild_repositories.bzl",
     _esbuild_repositories = "esbuild_repositories",
 )
 load(
-    "@build_bazel_rules_nodejs//packages/esbuild/toolchain:toolchain.bzl",
+    "@build_bazel_rules_nodejs//toolchains/esbuild:toolchain.bzl",
     _configure_esbuild_toolchain = "configure_esbuild_toolchain",
 )
 

scripts/update-esbuild-versions.js

@@ -82,8 +82,8 @@ async function main() {
   replaceFileContent('packages/esbuild/esbuild_packages.bzl', replacements);
 
   // update package.json used for API wrapper
-  replaceFileContent('packages/esbuild/toolchain/package.json', [[/"esbuild": "(.+?)"/, version]]);
-  exec(`npm i --package-lock-only`, {silent: true, cwd: 'packages/esbuild/toolchain'});
+  replaceFileContent('toolchains/esbuild/package.json', [[/"esbuild": "(.+?)"/, version]]);
+  exec(`npm i --package-lock-only`, {silent: true, cwd: 'toolchains/esbuild'});
 }
 
 main();

packages/esbuild/toolchain/BUILD.bazel → toolchains/esbuild/BUILD.bazel

@@ -1,7 +1,13 @@
 load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
-load("@build_bazel_rules_nodejs//packages/esbuild:esbuild_packages.bzl", "ESBUILD_PACKAGES")
+load("@build_bazel_rules_nodejs//toolchains/esbuild:esbuild_packages.bzl", "ESBUILD_PACKAGES")
 load(":toolchain.bzl", "configure_esbuild_toolchains")
 
+filegroup(
+    name = "package_contents",
+    srcs = glob(["*"]),
+    visibility = ["//:__pkg__"],
+)
+
 exports_files([
     "package.json",
     "package-lock.json",
@@ -19,18 +25,9 @@ configure_esbuild_toolchains(
 bzl_library(
     name = "bzl",
     srcs = [
+        "esbuild_packages.bzl",
+        "esbuild_repositories.bzl",
         "toolchain.bzl",
     ],
-    visibility = ["//packages/esbuild:__pkg__"],
-)
-
-filegroup(
-    name = "srcs",
-    srcs = [
-        "BUILD.bazel",
-        "package.json",
-        "package-lock.json",
-        "toolchain.bzl",
-    ],
-    visibility = ["//packages/esbuild:__pkg__"],
+    visibility = ["//visibility:public"],
 )

packages/esbuild/esbuild_repositories.bzl → toolchains/esbuild/esbuild_repositories.bzl

@@ -10,11 +10,13 @@ def _maybe(repo_rule, name, **kwargs):
     if name not in native.existing_rules():
         repo_rule(name = name, **kwargs)
 
-def esbuild_repositories(name = ""):
+def esbuild_repositories(name = "", npm_repository = "npm"):
     """Helper for fetching and setting up the esbuild versions and toolchains
 
     Args:
         name: currently unused
+        npm_repository:  the name of the repository where the @bazel/esbuild package is installed
+            by npm_install or yarn_install.
     """
 
     for name, meta in ESBUILD_PACKAGES.platforms.items():
@@ -27,20 +29,30 @@ def esbuild_repositories(name = ""):
             sha256 = meta.sha,
         )
 
-        toolchain_label = Label("@build_bazel_rules_nodejs//packages/esbuild/toolchain:esbuild_%s_toolchain" % name)
+        toolchain_label = Label("@build_bazel_rules_nodejs//toolchains/esbuild:esbuild_%s_toolchain" % name)
         native.register_toolchains("@%s//%s:%s" % (toolchain_label.workspace_name, toolchain_label.package, toolchain_label.name))
 
-    pkg_label = Label("@build_bazel_rules_nodejs//packages/esbuild/toolchain:package.json")
+    # When used from our distribution, the toolchain in rules_nodejs needs to point out to the
+    # @bazel/esbuild package where it was installed by npm_install so that our launcher.js can
+    # require('esbuild') via the multi-linker.
+    pkg_label = Label("@%s//packages/esbuild:esbuild.bzl" % npm_repository)
+    package_path = "external/" + pkg_label.workspace_name + "/@bazel/esbuild"
+
+    # BEGIN-INTERNAL
+    # But when used within rules_nodejs locally from source, it's linked next to the launcher.js source
+    package_path = "packages/esbuild"
+
+    # END-INTERNAL
     npm_install(
         name = "esbuild_npm",
-        package_json = pkg_label,
-        package_lock_json = Label("@build_bazel_rules_nodejs//packages/esbuild/toolchain:package-lock.json"),
+        package_json = Label("@build_bazel_rules_nodejs//toolchains/esbuild:package.json"),
+        package_lock_json = Label("@build_bazel_rules_nodejs//toolchains/esbuild:package-lock.json"),
         args = [
             # Install is run with ignore scripts so that esbuild's postinstall script does not run,
             # as we never use the downloaded binary anyway and instead set 'ESBUILD_BINARY_PATH' to the toolchains path.
             # This allows us to deal with --platform
             "--ignore-scripts",
         ],
         symlink_node_modules = False,
-        package_path = "packages/esbuild",
+        package_path = package_path,
     )

packages/esbuild/toolchain/toolchain.bzl → toolchains/esbuild/toolchain.bzl

@@ -21,7 +21,7 @@ _esbuild_toolchain = rule(
     },
 )
 
-TOOLCHAIN = Label("@build_bazel_rules_nodejs//packages/esbuild/toolchain:toolchain_type")
+TOOLCHAIN = Label("@build_bazel_rules_nodejs//toolchains/esbuild:toolchain_type")
 
 def configure_esbuild_toolchain(name, binary, exec_compatible_with):
     """Defines a toolchain for esbuild given the binary path and platform constraints