Fine-grained npm_install & yarn_install deps

.ci/rules_nodejs.json

@@ -8,7 +8,6 @@
         "--host_cpu=x64_windows_msys"
       ],
       "test_tag_filters": [
-        "-no_windows",
         "-slow"
       ],
       "targets": ["//...", "@program_example//...", "@packages_example/..."],

README.md

@@ -145,8 +145,17 @@ then next you will create a `BUILD.bazel` file in your project root containing:
 ```python
 package(default_visibility = ["//visibility:public"])
 
-# NOTE: this may move to node_modules/BUILD in a later release
-filegroup(name = "node_modules", srcs = glob(["node_modules/**/*"]))
+filegroup(
+    name = "node_modules",
+    srcs = glob(
+        include = ["node_modules/**/*"],
+        # Files with spaces in the name are not legal Bazel labels
+        exclude = [
+          "node_modules/**/* */**",
+          "node_modules/**/* *",
+        ],
+    ),
+)
 ```
 
 We recommend using the version of the package management tools installed by
@@ -208,7 +217,7 @@ Using Yarn (preferred):
 load("@build_bazel_rules_nodejs//:defs.bzl", "yarn_install")
 
 yarn_install(
-    name = "foo",
+    name = "npm",
     package_json = "//:package.json",
     yarn_lock = "//:yarn.lock",
 )
@@ -220,26 +229,72 @@ Using NPM:
 load("@build_bazel_rules_nodejs//:defs.bzl", "npm_install")
 
 npm_install(
-    name = "foo",
+    name = "npm",
     package_json = "//:package.json",
     package_lock_json = "//:package-lock.json",
 )
 ```
 
-You can then reference this version of `node_modules` in your `BUILD` rules via:
+#### Fine-grained npm package dependencies
+
+You can then reference individual npm packages in your `BUILD` rules via:
 
 ```python
 load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")
 
 nodejs_binary(
     name = "bar",
-    # Ordinarily this defaults to //:node_modules
-    node_modules = "@foo//:node_modules",
+    data = [
+      "@npm//:foo",
+      "@npm//:baz",
+    ]
+    ...
+)
+```
+
+In this case, the `bar` nodejs_binary depends only the `foo` and `baz` npm packages
+and all of their transitive deps.
+
+For other rules such as `jasmine_node_test`, fine grained
+npm dependencies are specified in the `deps` attribute:
+
+```python
+jasmine_node_test(
+    name = "test",
     ...
+    deps = [
+        "@npm//:jasmine",
+        "@npm//:foo",
+        "@npm//:baz",
+        ...
+    ],
 )
 ```
 
-With this approach, Bazel is responsible for making sure that `node_modules` is
+#### Coarse node_modules dependencies
+
+Using fine grained npm dependencies is recommended to minimize
+the number of inputs to your rules. However, for backward compatibility
+there are also filegroups defined by `yarn_install` and `npm_install`
+that include all packages under `node_modules` and which can be used
+with the `node_modules` attribute of nodejs rules.
+
+* `@npm//:node_modules` is includes **all** files under `node_modules`
+* `@npm//:node_modules_lite` is includes only `.js`, `.d.ts`, `.json` and `./bin/*` under `node_modules` (this reduces the number of input files)
+* `@npm//:node_modules_none` is an empty file group which is useful when a nodejs_binary has no node_module dependencies but it still requires its `node_modules` attribute set
+
+```python
+load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")
+
+nodejs_binary(
+    name = "bar",
+    node_modules = "@npm//:node_modules_lite",
+)
+```
+
+#### Why use Bazel-managed dependencies?
+
+With the Bazel-managed dependencies approach, Bazel is responsible for making sure that `node_modules` is
 up to date with `package[-lock].json` or `yarn.lock`.  This means Bazel will set it up when the
 repo is first cloned, and rebuild it whenever it changes.
 

WORKSPACE

@@ -32,6 +32,11 @@ local_repository(
     path = "examples/packages",
 )
 
+local_repository(
+    name = "bazel_managed_deps",
+    path = "examples/bazel_managed_deps",
+)
+
 local_repository(
     name = "node_loader_e2e_no_preserve_symlinks",
     path = "internal/e2e/node_loader_no_preserve_symlinks",
@@ -47,7 +52,7 @@ local_repository(
     path = "internal/test/node_resolve_dep",
 )
 
-load("//:defs.bzl", "node_repositories")
+load("//:defs.bzl", "node_repositories", "yarn_install", "npm_install")
 
 # Install a hermetic version of node.
 # After this is run, these labels will be available:
@@ -62,7 +67,8 @@ node_repositories(
         "//:package.json",
         "//examples/rollup:package.json",
         "@program_example//:package.json",
-        "//internal/test:package.json"
+        "//internal/test:package.json",
+        "//internal/npm_install/test:package.json",
     ],
     preserve_symlinks = True,
 )
@@ -87,4 +93,20 @@ skydoc_repositories()
 load("@io_bazel_rules_go//go:def.bzl", "go_rules_dependencies", "go_register_toolchains")
 
 go_rules_dependencies()
-go_register_toolchains()
+go_register_toolchains()
+
+#
+# Install npm dependencies for tests
+#
+
+yarn_install(
+    name = "fine_grained_deps_yarn",
+    package_json = "//internal/e2e/fine_grained_deps:package.json",
+    yarn_lock = "//internal/e2e/fine_grained_deps:yarn.lock",
+)
+
+npm_install(
+    name = "fine_grained_deps_npm",
+    package_json = "//internal/e2e/fine_grained_deps:package.json",
+    package_lock_json = "//internal/e2e/fine_grained_deps:package-lock.json",
+)

examples/bazel_managed_deps/BUILD.bazel

@@ -0,0 +1,32 @@
+load("@build_bazel_rules_nodejs//:defs.bzl", "jasmine_node_test")
+
+# Test what happens when we depend on the catch-all "node_modules" rule rather
+# than declare our dependencies on individual npm packages.
+# This is the legacy behavior, so it also proves our backwards-compatibility
+# story.
+jasmine_node_test(
+    name = "test",
+    srcs = [
+        "coarse.spec.js",
+        "common.spec.js",
+    ],
+    node_modules = "@npm//:node_modules",
+)
+
+# Test what happens when only certain NPM packages are in our dependencies.
+# These packages and their dependencies are copied to the execroot, but
+# the rest are not.
+jasmine_node_test(
+    name = "fine_grained_test",
+    srcs = [
+        "common.spec.js",
+        "fine.spec.js",
+    ],
+    deps = [
+        "@npm//:jasmine",
+        "@npm//:typescript",
+        # Note, test-b depends on [email protected] which should get
+        # hoisted to node_modules/test-b/node_modules/test-a
+        "@npm//:@gregmagolan/test-b",
+    ],
+)

examples/bazel_managed_deps/README.md

@@ -0,0 +1,19 @@
+# Fine-grained dependencies
+
+Declaring the entire `node_modules` directory as an input to every nodejs action
+has performance problems. When using local sandboxing, every file is set up in
+the kernel container for the sandboxed disk, this is slow on Mac. With remote
+execution, we guarantee these files all need to be copied to the worker machine.
+
+Instead, we can declare individual npm packages as dependencies, e.g.:
+```
+nodejs_binary(
+    name = "fast",
+    data = ["@my_npm_packages//:jasmine"]
+)
+```
+
+and only the contents of `node_modules/jasmine/` will be copied to workers.
+
+See design doc:
+https://docs.google.com/document/d/1BmQfTKhKMIsd27YKzWIhU7on-OVgRTSnUi1EHtHcceo/edit#

examples/bazel_managed_deps/WORKSPACE

@@ -0,0 +1,22 @@
+# BEGIN BORING BOILERPLATE
+workspace(name = "bazel_managed_deps")
+local_repository(
+    name = "build_bazel_rules_nodejs",
+    path = "../..",
+)
+load("@build_bazel_rules_nodejs//:package.bzl", "rules_nodejs_dependencies")
+rules_nodejs_dependencies()
+
+load("@build_bazel_rules_nodejs//:defs.bzl", "node_repositories", "yarn_install")
+node_repositories()
+# END BORING BOILERPLATE
+
+# This runs yarn install, then our generate_build_file.js to create BUILD files
+# inside the resulting node_modules directory.
+# The name "npm" here means the resulting modules are referenced like
+# @npm//:jasmine
+yarn_install(
+    name = "npm",
+    package_json = "//:package.json",
+    yarn_lock = "//:yarn.lock",
+)

examples/bazel_managed_deps/coarse.spec.js

@@ -0,0 +1,6 @@
+describe('dependencies', () => {
+  it('it should resolve [email protected] which is under node_modules/test-a', () => {
+    const testA = require('@gregmagolan/test-a');
+    expect(testA).toEqual('test-a-0.0.2');
+  });
+});

examples/bazel_managed_deps/common.spec.js

@@ -0,0 +1,23 @@
+const ts = require('typescript');
+
+describe('dependencies', () => {
+  it('should get the typescript library', () => {
+    expect(ts.version).toBe('3.0.1');
+  });
+
+  it(`should resolve transitive dependencies
+  Note that jasmine-core is not listed in our deps[]
+  but it is a transitive dependency of jasmine, which is in our deps.`,
+     () => {
+       require('jasmine-core');
+     });
+
+  it(`should resolve @gregmagolan/test-b to version 0.0.2 with a @gregmagolan/test-a dependency of 0.0.1
+  Note that @gregmagolan/[email protected] is an explicit devDependency of this project,
+  so we are really testing that test-b will get the version it depends on, not
+  the hoisted one.`,
+     () => {
+       const testB = require('@gregmagolan/test-b');
+       expect(testB).toEqual('test-b-0.0.2/test-a-0.0.1');
+     });
+});

examples/bazel_managed_deps/fine.spec.js

@@ -0,0 +1,11 @@
+describe('dependencies', () => {
+  it('should fail to resolve test-a since it has not been specified as a fine-grained direct dependency',
+     () => {
+       try {
+         require('@gregmagolan/test-a');
+         expect(true).toBe(false);
+       } catch (err) {
+         expect(err.code).toBe('MODULE_NOT_FOUND');
+       }
+     });
+});

examples/bazel_managed_deps/package.json

@@ -0,0 +1,13 @@
+{
+  "name": "bazel_managed_deps",
+  "version": "1.0.0",
+  "license": "MIT",
+  "devDependencies": {
+    "jasmine": "^3.2.0",
+    "typescript": "^3.0.1"
+  },
+  "dependencies": {
+    "@gregmagolan/test-a": "^0.0.2",
+    "@gregmagolan/test-b": "^0.0.2"
+  }
+}

examples/bazel_managed_deps/yarn.lock

@@ -0,0 +1,93 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
+# yarn lockfile v1
+
+
+"@gregmagolan/[email protected]":
+  version "0.0.1"
+  resolved "https://registry.yarnpkg.com/@gregmagolan/test-a/-/test-a-0.0.1.tgz#c90ebc0676f13b34400da8d1e55ffe5aa76655b4"
+
+"@gregmagolan/test-a@^0.0.2":
+  version "0.0.2"
+  resolved "https://registry.yarnpkg.com/@gregmagolan/test-a/-/test-a-0.0.2.tgz#86ab79a55f44860b02d8275e22d282a1f82b8768"
+
+"@gregmagolan/test-b@^0.0.2":
+  version "0.0.2"
+  resolved "https://registry.yarnpkg.com/@gregmagolan/test-b/-/test-b-0.0.2.tgz#7549fb9f39895c0d744c2c24c98b0e5817e46bd7"
+  dependencies:
+    "@gregmagolan/test-a" "0.0.1"
+
+balanced-match@^1.0.0:
+  version "1.0.0"
+  resolved "https://registry.yarnpkg.com/balanced-match/-/balanced-match-1.0.0.tgz#89b4d199ab2bee49de164ea02b89ce462d71b767"
+
+brace-expansion@^1.1.7:
+  version "1.1.11"
+  resolved "https://registry.yarnpkg.com/brace-expansion/-/brace-expansion-1.1.11.tgz#3c7fcbf529d87226f3d2f52b966ff5271eb441dd"
+  dependencies:
+    balanced-match "^1.0.0"
+    concat-map "0.0.1"
+
+[email protected]:
+  version "0.0.1"
+  resolved "https://registry.yarnpkg.com/concat-map/-/concat-map-0.0.1.tgz#d8a96bd77fd68df7793a73036a3ba0d5405d477b"
+
+fs.realpath@^1.0.0:
+  version "1.0.0"
+  resolved "https://registry.yarnpkg.com/fs.realpath/-/fs.realpath-1.0.0.tgz#1504ad2523158caa40db4a2787cb01411994ea4f"
+
+glob@^7.0.6:
+  version "7.1.2"
+  resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.2.tgz#c19c9df9a028702d678612384a6552404c636d15"
+  dependencies:
+    fs.realpath "^1.0.0"
+    inflight "^1.0.4"
+    inherits "2"
+    minimatch "^3.0.4"
+    once "^1.3.0"
+    path-is-absolute "^1.0.0"
+
+inflight@^1.0.4:
+  version "1.0.6"
+  resolved "https://registry.yarnpkg.com/inflight/-/inflight-1.0.6.tgz#49bd6331d7d02d0c09bc910a1075ba8165b56df9"
+  dependencies:
+    once "^1.3.0"
+    wrappy "1"
+
+inherits@2:
+  version "2.0.3"
+  resolved "https://registry.yarnpkg.com/inherits/-/inherits-2.0.3.tgz#633c2c83e3da42a502f52466022480f4208261de"
+
+jasmine-core@~3.2.0:
+  version "3.2.1"
+  resolved "https://registry.yarnpkg.com/jasmine-core/-/jasmine-core-3.2.1.tgz#8e4ff5b861603ee83343f2b49eee6a0ffe9650ce"
+
+jasmine@^3.2.0:
+  version "3.2.0"
+  resolved "https://registry.yarnpkg.com/jasmine/-/jasmine-3.2.0.tgz#b3a018454781805650e46578803d08e7cfdd7b3d"
+  dependencies:
+    glob "^7.0.6"
+    jasmine-core "~3.2.0"
+
+minimatch@^3.0.4:
+  version "3.0.4"
+  resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-3.0.4.tgz#5166e286457f03306064be5497e8dbb0c3d32083"
+  dependencies:
+    brace-expansion "^1.1.7"
+
+once@^1.3.0:
+  version "1.4.0"
+  resolved "https://registry.yarnpkg.com/once/-/once-1.4.0.tgz#583b1aa775961d4b113ac17d9c50baef9dd76bd1"
+  dependencies:
+    wrappy "1"
+
+path-is-absolute@^1.0.0:
+  version "1.0.1"
+  resolved "https://registry.yarnpkg.com/path-is-absolute/-/path-is-absolute-1.0.1.tgz#174b9268735534ffbc7ace6bf53a5a9e1b5c5f5f"
+
+typescript@^3.0.1:
+  version "3.0.1"
+  resolved "https://registry.yarnpkg.com/typescript/-/typescript-3.0.1.tgz#43738f29585d3a87575520a4b93ab6026ef11fdb"
+
+wrappy@1:
+  version "1.0.2"
+  resolved "https://registry.yarnpkg.com/wrappy/-/wrappy-1.0.2.tgz#b5243d8f3ec1aa35f1364605bc0d1036e30ab69f"