Fine-grained npm_install & yarn_install deps

.bazelci/presubmit.yml

@@ -24,13 +24,14 @@ platforms:
   windows:
     run_targets:
     - "@nodejs//:yarn"
+    build_flags:
+    - "--build_tag_filters=-no_windows"
     build_targets:
     # Escape hyphens on later args
     - "--"
     - "//..."
-    # Cannot build the docsite on Windows, see
-    # https://github.com/bazelbuild/skydoc/issues/58
-    - "-//docs"
+    test_flags:
+    - "--test_tag_filters=-no_windows"
     test_targets:
     - "//..."
   rbe_ubuntu1604:

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",
+)

docs/BUILD.bazel

@@ -30,4 +30,7 @@ skylark_doc(
     format = "html",
     site_root = "/rules_nodejs",
     strip_prefix = "internal/",
+    # Cannot build the docsite on Windows, see
+    # https://github.com/bazelbuild/skydoc/issues/58
+    tags = ["no_windows"],
 )

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"
+  }
+}