Split up npm fine grained deps BUILD file into multiple BUILD files

BREAKING CHANGES:

Fine grained deps targets changed from `@wksp//:pkg` to `@wksp//pkg`

.circleci/bazel.rc

@@ -4,9 +4,6 @@
 # Don't be spammy in the logs
 build --noshow_progress
 
-# Don't run manual tests
-test --test_tag_filters=-manual
-
 # Print all the options that apply to the build.
 # This helps us diagnose which options override others
 # (e.g. /etc/bazel.bazelrc vs. tools/bazel.rc)

.circleci/config.yml

@@ -81,7 +81,8 @@ jobs:
 
       - run: bazel run @nodejs//:yarn
       - run: bazel build ...
-      - run: bazel test ... --define=some_env=some_value
+      - run: bazel test ...
+      - run: bazel test //examples/define_var --define=some_env=some_value
       # TODO(alexeagle): move this into the example proper
       - run: bazel run examples/rollup -- --help
 
@@ -90,6 +91,7 @@ jobs:
       - run: bazel run //internal/node/test:has_deps_legacy
       - run: bazel run //internal/node/test:has_deps
       - run: bazel run //internal/node/test:has_deps_hybrid
+      - run: bazel run @fine_grained_deps_yarn//typescript/bin:tsc
 
       # We should also be able to test targets in a different workspace
       - run: bazel test @program_example//...

README.md

@@ -202,8 +202,8 @@ load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")
 nodejs_binary(
     name = "bar",
     data = [
-      "@npm//:foo",
-      "@npm//:baz",
+      "@npm//foo",
+      "@npm//baz",
     ]
     ...
 )
@@ -220,14 +220,57 @@ jasmine_node_test(
     name = "test",
     ...
     deps = [
-        "@npm//:jasmine",
-        "@npm//:foo",
-        "@npm//:baz",
+        "@npm//jasmine",
+        "@npm//foo",
+        "@npm//baz",
         ...
     ],
 )
 ```
 
+#### Fine-grained npm package nodejs_binary targets
+
+If an npm package lists one of more `bin` entry points in its `package.json`,
+`nodejs_binary` targets will be generated for these.
+
+For example, the `protractor` package has two bin entries in its `package.json`:
+
+```json
+  "bin": {
+    "protractor": "bin/protractor",
+    "webdriver-manager": "bin/webdriver-manager"
+  },
+```
+
+These will result in two generated `nodejs_binary` targets in the `@npm//protractor/bin`
+package (if your npm deps workspace is `@npm`):
+
+```python
+nodejs_binary(
+    name = "protractor",
+    entry_point = "protractor/bin/protractor",
+    data = ["//protractor:protractor"],
+)
+
+nodejs_binary(
+    name = "webdriver-manager",
+    entry_point = "protractor/bin/webdriver-manager",
+    data = ["//protractor:protractor"],
+)
+```
+
+These targets can be used as executables for actions in custom rules or can
+be run by Bazel directly. For example, you can run protractor with the
+following:
+
+```sh
+$ bazel run @npm//protractor/bin:protractor
+```
+
+Note: These targets are in the `protractor/bin` package so they don't
+conflict with the deps targets. For example, `@npm//protractor:protractor` is
+the deps targets while `@npm//protractor/bin:protractor` is the binary target.
+
 #### Coarse node_modules dependencies
 
 Using fine grained npm dependencies is recommended to minimize
@@ -236,15 +279,14 @@ 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` includes all packages under `node_modules` as well as the `.bin` folder
 
 ```python
 load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")
 
 nodejs_binary(
     name = "bar",
-    node_modules = "@npm//:node_modules_lite",
+    node_modules = "@npm//:node_modules",
 )
 ```
 
@@ -260,8 +302,14 @@ filegroup(
     name = "node_modules",
     srcs = glob(
         include = ["node_modules/**/*"],
-        # Files with spaces in the name are not legal Bazel labels
         exclude = [
+          # Files under test & docs may contain file names that
+          # are not legal Bazel labels (e.g.,
+          # node_modules/ecstatic/test/public/中文/檔案.html)
+          "node_modules/test/**",
+          "node_modules/docs/**",
+          # Files with spaces are not allowed in Bazel runfiles
+          # See https://github.com/bazelbuild/bazel/issues/4327
           "node_modules/**/* */**",
           "node_modules/**/* *",
         ],

examples/bazel_managed_deps/BUILD.bazel

@@ -16,7 +16,7 @@ jasmine_node_test(
     name = "fine_grained_test",
     srcs = glob(["*.spec.js"]),
     deps = [
-        "@npm//:jasmine",
-        "@npm//:typescript",
+        "@npm//jasmine",
+        "@npm//typescript",
     ],
 )

examples/bazel_managed_deps/README.md

@@ -9,7 +9,7 @@ Instead, we can declare individual npm packages as dependencies, e.g.:
 ```
 nodejs_binary(
     name = "fast",
-    data = ["@npm//:jasmine"]
+    data = ["@npm//jasmine"]
 )
 ```
 

examples/bazel_managed_deps/WORKSPACE

@@ -14,7 +14,7 @@ node_repositories()
 # 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
+# @npm//jasmine
 yarn_install(
     name = "npm",
     package_json = "//:package.json",

examples/define_var/BUILD.bazel

@@ -7,11 +7,12 @@ jasmine_node_test(
     name = "define_var",
     srcs = glob(["*.spec.js"]),
     # Just here as a smoke test for this attribute.
-    # Test must be run with --define=some_env=some_value.
-    # Use `bazel test ... --define=some_env=some_value` or
-    # `bazel test //examples/rollup:test --define=some_env=some_value`.
+    # This test must be run with --define=some_env=some_value.
+    # Use `bazel test //examples/define_var --define=some_env=some_value`.
+    # Note that use of --define causes the entire build to be non-incremental
+    # since --define can affect the output of any action.
     configuration_env_vars = ["some_env"],
-    data = ["@fine_grained_deps_yarn//:jasmine"],
+    data = ["@fine_grained_deps_yarn//jasmine"],
     # Don't include this test in //... pattern since it requires special
     # --define option
     tags = ["manual"],

internal/e2e/fine_grained_deps/BUILD.bazel

@@ -25,11 +25,11 @@ load("@build_bazel_rules_nodejs//:defs.bzl", "jasmine_node_test")
         "fine.spec.js",
     ],
     deps = [
-        "@fine_grained_deps_%s//:jasmine" % pkgmgr,
-        "@fine_grained_deps_%s//:typescript" % pkgmgr,
+        "@fine_grained_deps_%s//jasmine" % pkgmgr,
+        "@fine_grained_deps_%s//typescript" % pkgmgr,
         # Note, test-b depends on [email protected] which should get
         # hoisted to node_modules/test-b/node_modules/test-a
-        "@fine_grained_deps_%s//:@gregmagolan/test-b" % pkgmgr,
+        "@fine_grained_deps_%s//@gregmagolan/test-b" % pkgmgr,
     ],
 ) for pkgmgr in [
     "yarn",

internal/e2e/rollup_fine_grained_deps/BUILD.bazel

@@ -15,7 +15,7 @@ rollup_bundle(
     srcs = ["has-deps.js"],
     entry_point = "internal/e2e/rollup_fine_grained_deps/has-deps.js",
     deps = [
-        "@fine_grained_deps_yarn//:@gregmagolan/test-b",
+        "@fine_grained_deps_yarn//@gregmagolan/test-b",
     ],
 )
 
@@ -25,7 +25,7 @@ rollup_bundle(
     name = "bundle_legacy",
     srcs = ["has-deps.js"],
     entry_point = "internal/e2e/rollup_fine_grained_deps/has-deps.js",
-    node_modules = "@fine_grained_deps_yarn//:node_modules_lite",
+    node_modules = "@fine_grained_deps_yarn//:node_modules",
 )
 
 # You can have a rollup_bundle with both a node_modules attribute
@@ -34,9 +34,9 @@ rollup_bundle(
     name = "bundle_hybrid",
     srcs = ["has-deps.js"],
     entry_point = "internal/e2e/rollup_fine_grained_deps/has-deps.js",
-    node_modules = "@fine_grained_deps_yarn//:node_modules_lite",
+    node_modules = "@fine_grained_deps_yarn//:node_modules",
     deps = [
-        "@fine_grained_deps_yarn//:@gregmagolan/test-b",
+        "@fine_grained_deps_yarn//@gregmagolan/test-b",
     ],
 )
 
@@ -60,7 +60,7 @@ jasmine_node_test(
         ":bundle_no_deps.js",
         ":bundle_no_deps.min.js",
         ":bundle_no_deps.min_debug.js",
-        "@fine_grained_deps_yarn//:@gregmagolan/test-a",
-        "@fine_grained_deps_yarn//:jasmine",
+        "@fine_grained_deps_yarn//@gregmagolan/test-a",
+        "@fine_grained_deps_yarn//jasmine",
     ],
 )

internal/jasmine_node_test/jasmine_runner.js

@@ -30,8 +30,8 @@ function main(args) {
       .filter(l => l.length > 0)
       // Filter here so that only files ending in `spec.js` and `test.js`
       // are added to jasmine as spec files. This is important as other
-      // deps such as "@npm//:typescript" if executed may cause the test to
-      // fail or have unexpected side-effects. "@npm//:typescript" would
+      // deps such as "@npm//typescript" if executed may cause the test to
+      // fail or have unexpected side-effects. "@npm//typescript" would
       // try to execute tsc, print its help, and process.exit(1)
       .filter(f => /[^a-zA-Z0-9](spec|test)\.js$/i.test(f))
       // Filter out files from node_modules that match test.js or spec.js

internal/node/node.bzl

@@ -198,8 +198,8 @@ _NODEJS_EXECUTABLE_ATTRS = {
           name = "my_binary",
           ...
           data = [
-              "@npm//:foo",
-              "@npm//:bar",
+              "@npm//foo",
+              "@npm//bar",
               ...
           ],
         )
@@ -228,9 +228,9 @@ _NODEJS_EXECUTABLE_ATTRS = {
             name = "my_test",
             ...
             deps = [
-                "@npm//:jasmine",
-                "@npm//:foo",
-                "@npm//:bar",
+                "@npm//jasmine",
+                "@npm//foo",
+                "@npm//bar",
                 ...
             ],
         )

internal/node/test/BUILD.bazel

@@ -14,7 +14,7 @@ nodejs_binary(
     name = "has_deps_legacy",
     data = ["has-deps.js"],
     entry_point = "build_bazel_rules_nodejs/internal/node/test/has-deps",
-    node_modules = "@fine_grained_deps_yarn//:node_modules_lite",
+    node_modules = "@fine_grained_deps_yarn//:node_modules",
 )
 
 # You can have a nodejs_binary with no node_modules attribute
@@ -23,7 +23,7 @@ nodejs_binary(
     name = "has_deps",
     data = [
         "has-deps.js",
-        "@fine_grained_deps_yarn//:typescript",
+        "@fine_grained_deps_yarn//typescript",
     ],
     entry_point = "build_bazel_rules_nodejs/internal/node/test/has-deps",
 )
@@ -34,8 +34,8 @@ nodejs_binary(
     name = "has_deps_hybrid",
     data = [
         "has-deps.js",
-        "@fine_grained_deps_yarn//:typescript",
+        "@fine_grained_deps_yarn//typescript",
     ],
     entry_point = "build_bazel_rules_nodejs/internal/node/test/has-deps",
-    node_modules = "@fine_grained_deps_yarn//:node_modules_lite",
+    node_modules = "@fine_grained_deps_yarn//:node_modules",
 )