feat(builtin): introduce dynamic dependencies concept

This lets our generated nodejs_binary targets for npm packages express dynamic runtime-only dependencies
This is needed for example so that rollup binary can require the rollup-plugin-* packages

WORKSPACE

@@ -219,6 +219,9 @@ yarn_install(
 
 yarn_install(
     name = "npm_install_test",
+    # exercise the dynamic_deps feature, even though it doesn't make sense for a real jasmine binary to depend on zone.js
+    # This will just inject an extra data[] dependency into the jasmine_bin generated target.
+    dynamic_deps = {"jasmine": "zone.js"},
     manual_build_file_contents = """
 filegroup(
   name = "test_files",

internal/npm_install/generate_build_file.js

@@ -58,6 +58,7 @@ const RULE_TYPE = args[1];
 const ERROR_ON_BAZEL_FILES = parseInt(args[2]);
 const LOCK_FILE_LABEL = args[3];
 const INCLUDED_FILES = args[4] ? args[4].split(',') : [];
+const DYNAMIC_DEPS = JSON.parse(args[5] || '{}');
 
 if (require.main === module) {
   main();
@@ -103,6 +104,7 @@ function main() {
 module.exports = {
   main,
   printPackageBin,
+  addDynamicDependencies,
 };
 
 /**
@@ -436,6 +438,27 @@ function hasRootBuildFile(pkg, rootPath) {
   return false;
 }
 
+function addDynamicDependencies(pkgs, dynamic_deps = DYNAMIC_DEPS) {
+  pkgs.forEach(p => {
+    function match(name) {
+      // Automatically include dynamic dependency on plugins of the form pkg-plugin-foo
+      if (name.startsWith(`${p._name}-plugin-`)) return true;
+
+      const value = dynamic_deps[p._name];
+      if (name === value) return true;
+
+      // Support wildcard match
+      if (value && value.includes('*') && name.startsWith(value.substring(0, value.indexOf('*')))) {
+        return true;
+      }
+
+      return false;
+    }
+    p._dynamicDependencies =
+        pkgs.filter(x => !!x._name && match(x._name)).map(dyn => `//${dyn._dir}:${dyn._name}`);
+  });
+}
+
 /**
  * Finds and returns an array of all packages under a given path.
  */
@@ -460,6 +483,8 @@ function findPackages(p = 'node_modules') {
   packages.forEach(
       f => pkgs.push(parsePackage(f), ...findPackages(path.posix.join(f, 'node_modules'))));
 
+  addDynamicDependencies(pkgs);
+
   const scopes = listing.filter(f => f.startsWith('@'))
                      .map(f => path.posix.join(p, f))
                      .filter(f => isDirectory(f));
@@ -582,10 +607,10 @@ function cleanupEntryPointPath(p) {
 }
 
 /**
- * Cleans up the given path  
+ * Cleans up the given path
  * Then tries to resolve the path into a file and warns if in DEBUG and the file dosen't exist
- * @param {any} pkg 
- * @param {string} path 
+ * @param {any} pkg
+ * @param {string} path
  * @returns {string | undefined}
  */
 function findEntryFile(pkg, path) {
@@ -631,9 +656,9 @@ function resolveMainFile(pkg, mainFileName) {
 }
 
 /**
- * Tries to resolve the mainFile from a given pkg  
+ * Tries to resolve the mainFile from a given pkg
  * This uses seveal mainFileNames in priority to find a correct usable file
- * @param {any} pkg 
+ * @param {any} pkg
  * @returns {string | undefined}
  */
 function resolvePkgMainFile(pkg) {
@@ -953,6 +978,10 @@ function printPackageBin(pkg) {
     result = `load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")
 
 `;
+    const data = [`//${pkg._dir}:${pkg._name}`];
+    if (pkg._dynamicDependencies) {
+      data.push(...pkg._dynamicDependencies);
+    }
 
     for (const [name, path] of executables.entries()) {
       // Handle additionalAttributes of format:
@@ -977,7 +1006,7 @@ nodejs_binary(
     name = "${name}",
     entry_point = "//:node_modules/${pkg._dir}/${path}",
     install_source_map_support = False,
-    data = ["//${pkg._dir}:${pkg._name}"],${additionalAttributes}
+    data = [${data.map(p => `"${p}"`).join(', ')}],${additionalAttributes}
 )
 
 `;

internal/npm_install/npm_install.bzl

@@ -65,6 +65,19 @@ COMMON_ATTRIBUTES = dict(dict(), **{
         If symlink_node_modules is True, this attribute is ignored since
         the dependency manager will run in the package.json location.""",
     ),
+    "dynamic_deps": attr.string_dict(
+        doc = """Declare implicit dependencies between npm packages.
+        
+        In many cases, an npm package doesn't list a dependency on another package, yet still require()s it.
+        One example is plugins, where a tool like rollup can require rollup-plugin-json if the user installed it.
+        Another example is the tsc_wrapped binary in @bazel/typescript which can require tsickle if its installed.
+        
+        Note, this may sound like "optionalDependencies" but that field in package.json actually means real dependencies
+        which are installed, but failures on installation are ignored.
+
+        We must declare these dependencies so that Bazel includes the maybe-require()d package in the inputs to the program.""",
+        default = {"@bazel/typescript": "tsickle"},
+    ),
     "exclude_packages": attr.string_list(
         doc = """DEPRECATED. This attribute is no longer used.""",
     ),
@@ -141,6 +154,7 @@ def _create_build_files(repository_ctx, rule_type, node, lock_file):
         "1" if error_on_build_files else "0",
         str(lock_file),
         ",".join(repository_ctx.attr.included_files),
+        str(repository_ctx.attr.dynamic_deps),
     ])
     if result.return_code:
         fail("generate_build_file.js failed: \nSTDOUT:\n%s\nSTDERR:\n%s" % (result.stdout, result.stderr))

internal/npm_install/test/generate_build_file.spec.js

@@ -1,5 +1,5 @@
 const {check, files} = require('./check');
-const {printPackageBin} = require('../generate_build_file');
+const {printPackageBin, addDynamicDependencies} = require('../generate_build_file');
 
 describe('build file generator', () => {
   describe('integration test', () => {
@@ -77,4 +77,27 @@ describe('build file generator', () => {
           .toContain('nodejs_binary(');
     });
   });
+
+  describe('dynamic dependencies', () => {
+    it('should include requested dynamic dependencies in nodejs_binary data', () => {
+      const pkgs = [{_name: 'foo', bin: 'foobin', _dir: 'some_dir'}, {_name: 'bar', _dir: 'bar'}];
+      addDynamicDependencies(pkgs, {'foo': 'bar'});
+      expect(pkgs[0]._dynamicDependencies).toEqual(['//bar:bar']);
+      expect(printPackageBin(pkgs[0])).toContain('data = ["//some_dir:foo", "//bar:bar"]');
+    });
+    it('should support wildcard', () => {
+      const pkgs = [{_name: 'foo', bin: 'foobin', _dir: 'some_dir'}, {_name: 'bar', _dir: 'bar'}];
+      addDynamicDependencies(pkgs, {'foo': 'b*'});
+      expect(pkgs[0]._dynamicDependencies).toEqual(['//bar:bar']);
+      expect(printPackageBin(pkgs[0])).toContain('data = ["//some_dir:foo", "//bar:bar"]');
+    });
+    it('should automatically include plugins in nodejs_binary data', () => {
+      const pkgs =
+          [{_name: 'foo', bin: 'foobin', _dir: 'some_dir'}, {_name: 'foo-plugin-bar', _dir: 'bar'}];
+      addDynamicDependencies(pkgs, {});
+      expect(pkgs[0]._dynamicDependencies).toEqual(['//bar:foo-plugin-bar']);
+      expect(printPackageBin(pkgs[0]))
+          .toContain('data = ["//some_dir:foo", "//bar:foo-plugin-bar"]');
+    });
+  });
 });

internal/npm_install/test/golden/jasmine/bin/BUILD.bazel.golden

@@ -5,5 +5,5 @@ nodejs_binary(
     name = "jasmine",
     entry_point = "//:node_modules/jasmine/bin/jasmine.js",
     install_source_map_support = False,
-    data = ["//jasmine:jasmine"],
+    data = ["//jasmine:jasmine", "//zone.js:zone.js"],
 )

packages/typescript/docs/install.md

@@ -104,7 +104,7 @@ managing npm dependencies with Bazel.
 
 ## Customizing the TypeScript compiler binary
 
-An example is needing to increase the NodeJS heap size used for compilations.
+An example use case is needing to increase the NodeJS heap size used for compilations.
 
 Similar to above, you declare your own binary for running tsc_wrapped, e.g.:
 
@@ -127,7 +127,9 @@ nodejs_binary(
 
 then refer to that target in the `compiler` attribute of your `ts_library` rule.
 
-Another use case is to use [tsickle], which is an optional dependency of `tsc_wrapped`. In case it needs `"@npm//tsickle"` added to the `data` attribute above.
+Note that `nodejs_binary` targets generated by `npm_install`/`yarn_install` can include data dependencies
+on packages which aren't declared as dependencies. For example, if you use [tsickle] to generate Closure Compiler-compatible JS, then it needs to be a `data` dependency of `tsc_wrapped` so that it can be loaded at runtime.
+See the `dynamic_deps` attribute of `npm_install`/`yarn_install` to include more such runtime dependencies in the generated `nodejs_binary` targets, rather than needing to define a custom one.
 
 [tsickle]: https://github.com/angular/tsickle