diff --git a/WORKSPACE b/WORKSPACE index 9c6c6392b0..b2871ca9ff 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -217,9 +217,14 @@ yarn_install( yarn_install( name = "fine_grained_goldens", - # 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"}, + # exercise the dynamic_deps feature, even though it doesn't make sense for the targets to + # depend on zone.js or Angular core. This will just inject an extra data[] dependency into + # the generated binary targets. Note that we also ensure that scoped packages can be properly + # modified. + dynamic_deps = { + "@gregmagolan/test-a": "@angular/core", + "jasmine": "zone.js", + }, manual_build_file_contents = """ filegroup( name = "golden_files", diff --git a/internal/npm_install/generate_build_file.js b/internal/npm_install/generate_build_file.js index 275daf0185..28e6df24c8 100644 --- a/internal/npm_install/generate_build_file.js +++ b/internal/npm_install/generate_build_file.js @@ -444,9 +444,9 @@ 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; + if (name.startsWith(`${p._moduleName}-plugin-`)) return true; - const value = dynamic_deps[p._name]; + const value = dynamic_deps[p._moduleName]; if (name === value) return true; // Support wildcard match @@ -456,8 +456,8 @@ function addDynamicDependencies(pkgs, dynamic_deps = DYNAMIC_DEPS) { return false; } - p._dynamicDependencies = - pkgs.filter(x => !!x._name && match(x._name)).map(dyn => `//${dyn._dir}:${dyn._name}`); + p._dynamicDependencies = pkgs.filter(x => + !!x._moduleName && match(x._moduleName)).map(dyn => `//${dyn._dir}:${dyn._name}`); }); } @@ -485,13 +485,13 @@ 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)); scopes.forEach(f => pkgs.push(...findPackages(f))); + addDynamicDependencies(pkgs); + return pkgs; } @@ -532,6 +532,10 @@ function parsePackage(p) { // Stash the package directory name for future use pkg._name = pkg._dir.split('/').pop(); + // Module name of the package. Unlike "_name" this represents the + // full package name (including scope name). + pkg._moduleName = pkg.name || `${pkg._dir}/${pkg._name}`; + // Keep track of whether or not this is a nested package pkg._isNested = /\/node_modules\//.test(p); diff --git a/internal/npm_install/test/generate_build_file.spec.js b/internal/npm_install/test/generate_build_file.spec.js index 65a4ecdee7..db258385bd 100644 --- a/internal/npm_install/test/generate_build_file.spec.js +++ b/internal/npm_install/test/generate_build_file.spec.js @@ -80,24 +80,36 @@ describe('build file generator', () => { 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'}); + const pkgs = [ + {_name: 'foo', bin: 'foobin', _dir: 'some_dir', _moduleName: 'foo'}, + {_name: 'bar', _dir: 'bar', _moduleName: 'bar'}, + {_name: 'typescript', bin: 'tsc_wrapped', _dir: 'a', _moduleName: '@bazel/typescript'}, + {_name: 'tsickle', _dir: 'b', _moduleName: 'tsickle'}, + ]; + addDynamicDependencies(pkgs, {'foo': 'bar', '@bazel/typescript': 'tsickle'}); expect(pkgs[0]._dynamicDependencies).toEqual(['//bar:bar']); + expect(pkgs[2]._dynamicDependencies).toEqual(['//b:tsickle']); expect(printPackageBin(pkgs[0])).toContain('data = ["//some_dir:foo", "//bar:bar"]'); + expect(printPackageBin(pkgs[2])).toContain('data = ["//a:typescript", "//b:tsickle"]'); }); it('should support wildcard', () => { - const pkgs = [{_name: 'foo', bin: 'foobin', _dir: 'some_dir'}, {_name: 'bar', _dir: 'bar'}]; + const pkgs = [ + {_name: 'foo', bin: 'foobin', _dir: 'some_dir', _moduleName: 'foo'}, + {_name: 'bar', _dir: 'bar', _moduleName: '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'}]; + const pkgs = [ + {_name: 'foo', bin: 'foobin', _dir: 'some_dir', _moduleName: 'foo'}, + {_name: 'foo-plugin-bar', _dir: 'bar', _moduleName: 'foo-plugin-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"]'); }); }); -}); \ No newline at end of file +}); diff --git a/internal/npm_install/test/golden/@gregmagolan/test-a/bin/BUILD.bazel.golden b/internal/npm_install/test/golden/@gregmagolan/test-a/bin/BUILD.bazel.golden index d980958a99..0581c9b23e 100644 --- a/internal/npm_install/test/golden/@gregmagolan/test-a/bin/BUILD.bazel.golden +++ b/internal/npm_install/test/golden/@gregmagolan/test-a/bin/BUILD.bazel.golden @@ -5,5 +5,5 @@ nodejs_binary( name = "test", entry_point = "//:node_modules/@gregmagolan/test-a/@bin/test.js", install_source_map_support = False, - data = ["//@gregmagolan/test-a:test-a"], + data = ["//@gregmagolan/test-a:test-a", "//@angular/core:core"], )