Skip to content

Commit 8d77827

Browse files
authored
feat(builtin): expose @npm//foo__all_files filegroup that includes all files in the npm package (#1600)
Including files with spaces & unicode characters & files that have been filtered out by the `included_files` attribute. This filegroup cannot be used in runfiles because of Bazel issue bazelbuild/bazel#4327, but it can be used for other purposes such as the srcs input to a pkg_tar for generating a tar of an npm package pulled down with yarn_install or npm_install.
1 parent 42bdb9c commit 8d77827

19 files changed

Lines changed: 362 additions & 261 deletions

File tree

WORKSPACE

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,16 @@ yarn_install(
227227

228228
yarn_install(
229229
name = "fine_grained_goldens",
230+
included_files = [
231+
"",
232+
".js",
233+
".jst",
234+
".ts",
235+
".map",
236+
".d.ts",
237+
".json",
238+
".proto",
239+
],
230240
manual_build_file_contents = """
231241
filegroup(
232242
name = "golden_files",

internal/node/node.bzl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,8 @@ def _nodejs_binary_impl(ctx):
148148
node_modules_manifest = write_node_modules_manifest(ctx)
149149
node_modules_depsets = []
150150
node_modules_depsets.append(depset(ctx.files.node_modules))
151+
if NpmPackageInfo in ctx.attr.node_modules:
152+
node_modules_depsets.append(ctx.attr.node_modules[NpmPackageInfo].sources)
151153

152154
# Also include files from npm fine grained deps as inputs.
153155
# These deps are identified by the NpmPackageInfo provider.

internal/npm_install/generate_build_file.js

Lines changed: 85 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -161,17 +161,12 @@ for installation instructions.`);
161161
* Generates the root BUILD file.
162162
*/
163163
function generateRootBuildFile(pkgs) {
164-
let exportsStarlark = '';
165-
pkgs.forEach(pkg => {
166-
pkg._files.forEach(f => {
167-
exportsStarlark += ` "node_modules/${pkg._dir}/${f}",
168-
`;
169-
});
170-
});
171-
let filesStarlark = '';
164+
let pkgFilesStarlark = '';
172165
if (pkgs.length) {
173-
const list = pkgs.map(pkg => `"//${pkg._dir}:${pkg._name}__all_files",`).join('\n ');
174-
filesStarlark = `
166+
const list = pkgs.map(pkg => `"//${pkg._dir}:${pkg._name}__files",
167+
"//${pkg._dir}:${pkg._name}__nested_node_modules",`)
168+
.join('\n ');
169+
pkgFilesStarlark = `
175170
# direct sources listed for strict deps support
176171
srcs = [
177172
${list}
@@ -186,6 +181,13 @@ for installation instructions.`);
186181
${list}
187182
],`;
188183
}
184+
let exportsStarlark = '';
185+
pkgs.forEach(pkg => {
186+
pkg._files.forEach(f => {
187+
exportsStarlark += ` "node_modules/${pkg._dir}/${f}",
188+
`;
189+
});
190+
});
189191
let buildFile = BUILD_FILE_HEADER +
190192
`load("@build_bazel_rules_nodejs//internal/npm_install:node_module_library.bzl", "node_module_library")
191193
@@ -197,7 +199,7 @@ ${exportsStarlark}])
197199
# there are many files in target.
198200
# See https://github.com/bazelbuild/bazel/issues/5153.
199201
node_module_library(
200-
name = "node_modules",${filesStarlark}${depsStarlark}
202+
name = "node_modules",${pkgFilesStarlark}${depsStarlark}
201203
)
202204
203205
`;
@@ -390,9 +392,6 @@ def _maybe(repo_rule, name, **kwargs):
390392
}
391393
return isDirectory ? files.concat(listFiles(rootDir, relPath)) : files.concat(relPath);
392394
}, [])
393-
// Files with spaces (\x20) or unicode characters (<\x20 && >\x7E) are not allowed in
394-
// Bazel runfiles. See https://github.com/bazelbuild/bazel/issues/4327
395-
.filter(f => !/[^\x21-\x7E]/.test(f))
396395
// We return a sorted array so that the order of files
397396
// is the same regardless of platform
398397
.sort();
@@ -480,6 +479,10 @@ def _maybe(repo_rule, name, **kwargs):
480479
pkg._isNested = /\/node_modules\//.test(p);
481480
// List all the files in the npm package for later use
482481
pkg._files = listFiles(p);
482+
// The subset of files that are valid in runfiles.
483+
// Files with spaces (\x20) or unicode characters (<\x20 && >\x7E) are not allowed in
484+
// Bazel runfiles. See https://github.com/bazelbuild/bazel/issues/4327
485+
pkg._runfiles = pkg._files.filter((f) => !/[^\x21-\x7E]/.test(f));
483486
// Initialize _dependencies to an empty array
484487
// which is later filled with the flattened dependency list
485488
pkg._dependencies = [];
@@ -686,10 +689,11 @@ def _maybe(repo_rule, name, **kwargs):
686689
*/
687690
function printJson(pkg) {
688691
// Clone and modify _dependencies to avoid circular issues when JSONifying
689-
// & delete _files array
692+
// & delete _files & _runfiles arrays
690693
const cloned = Object.assign({}, pkg);
691694
cloned._dependencies = pkg._dependencies.map(dep => dep._dir);
692695
delete cloned._files;
696+
delete cloned._runfiles;
693697
return JSON.stringify(cloned, null, 2).split('\n').map(line => `# ${line}`).join('\n');
694698
}
695699
/**
@@ -748,15 +752,6 @@ def _maybe(repo_rule, name, **kwargs):
748752
return false;
749753
});
750754
}
751-
/**
752-
* If the package is in the Angular package format returns list
753-
* of package files that end with `.umd.js`, `.ngfactory.js` and `.ngsummary.js`.
754-
*/
755-
function getNgApfScripts(pkg) {
756-
return isNgApfPackage(pkg) ?
757-
filterFiles(pkg._files, ['.umd.js', '.ngfactory.js', '.ngsummary.js']) :
758-
[];
759-
}
760755
/**
761756
* Looks for a file within a package and returns it if found.
762757
*/
@@ -773,91 +768,98 @@ def _maybe(repo_rule, name, **kwargs):
773768
* Given a pkg, return the skylark `node_module_library` targets for the package.
774769
*/
775770
function printPackage(pkg) {
776-
const sources = filterFiles(pkg._files, INCLUDED_FILES);
777-
const files = sources.filter((f) => !f.startsWith('node_modules/'));
778-
const nestedNodeModules = sources.filter((f) => f.startsWith('node_modules/'));
779-
const dtsSources = filterFiles(pkg._files, ['.d.ts']);
780-
// TODO(gmagolan): add UMD & AMD scripts to scripts even if not an APF package _but_ only if they
781-
// are named?
782-
const namedSources = getNgApfScripts(pkg);
783-
const deps = [pkg].concat(pkg._dependencies.filter(dep => dep !== pkg && !dep._isNested));
784-
let namedSourcesStarlark = '';
785-
if (namedSources.length) {
786-
namedSourcesStarlark = `
787-
# subset of srcs that are javascript named-UMD or named-AMD scripts
788-
named_module_srcs = [
789-
${namedSources.map((f) => `"//:node_modules/${pkg._dir}/${f}",`).join('\n ')}
790-
],`;
791-
}
792-
let filesStarlark = '';
793-
if (files.length) {
794-
filesStarlark = `
795-
# ${pkg._dir} package files (and files in nested node_modules)
796-
srcs = [
771+
function starlarkFiles(attr, files, comment = '') {
772+
return `
773+
${comment ? comment + '\n ' : ''}${attr} = [
797774
${files.map((f) => `"//:node_modules/${pkg._dir}/${f}",`).join('\n ')}
798775
],`;
799776
}
800-
let nestedNodeModulesStarlark = '';
801-
if (nestedNodeModules.length) {
802-
nestedNodeModulesStarlark = `
803-
# ${pkg._dir} package files (and files in nested node_modules)
804-
srcs = [
805-
${nestedNodeModules.map((f) => `"//:node_modules/${pkg._dir}/${f}",`)
806-
.join('\n ')}
807-
],`;
808-
}
809-
let depsStarlark = '';
810-
if (deps.length) {
811-
const list = deps.map(dep => `"//${dep._dir}:${dep._name}__contents",`).join('\n ');
812-
depsStarlark = `
813-
# flattened list of direct and transitive dependencies hoisted to root by the package manager
814-
deps = [
815-
${list}
816-
],`;
817-
}
818-
let dtsStarlark = '';
819-
if (dtsSources.length) {
820-
dtsStarlark = `
821-
# ${pkg._dir} package declaration files (and declaration files in nested node_modules)
822-
srcs = [
823-
${dtsSources.map(f => `"//:node_modules/${pkg._dir}/${f}",`).join('\n ')}
824-
],`;
825-
}
777+
const includedRunfiles = filterFiles(pkg._runfiles, INCLUDED_FILES);
778+
// Files that are part of the npm package not including its nested node_modules
779+
// (filtered by the 'included_files' attribute)
780+
const pkgFiles = includedRunfiles.filter((f) => !f.startsWith('node_modules/'));
781+
const pkgFilesStarlark = pkgFiles.length ? starlarkFiles('srcs', pkgFiles) : '';
782+
// Files that are in the npm package's nested node_modules
783+
// (filtered by the 'included_files' attribute)
784+
const nestedNodeModules = includedRunfiles.filter((f) => f.startsWith('node_modules/'));
785+
const nestedNodeModulesStarlark = nestedNodeModules.length ? starlarkFiles('srcs', nestedNodeModules) : '';
786+
// Files that have been excluded from the ${pkg._name}__files target above because
787+
// they are filtered out by 'included_files' or because they are not valid runfiles
788+
// See https://github.com/bazelbuild/bazel/issues/4327.
789+
const notPkgFiles = pkg._files.filter((f) => !f.startsWith('node_modules/') && !includedRunfiles.includes(f));
790+
const notPkgFilesStarlark = notPkgFiles.length ? starlarkFiles('srcs', notPkgFiles) : '';
791+
// If the package is in the Angular package format returns list
792+
// of package files that end with `.umd.js`, `.ngfactory.js` and `.ngsummary.js`.
793+
// TODO(gmagolan): add UMD & AMD scripts to scripts even if not an APF package _but_ only if they
794+
// are named?
795+
const namedSources = isNgApfPackage(pkg) ?
796+
filterFiles(pkg._runfiles, ['.umd.js', '.ngfactory.js', '.ngsummary.js']) :
797+
[];
798+
const namedSourcesStarlark = namedSources.length ?
799+
starlarkFiles('named_module_srcs', namedSources, '# subset of srcs that are javascript named-UMD or named-AMD scripts') :
800+
'';
801+
// Typings files that are part of the npm package not including nested node_modules
802+
const dtsSources = filterFiles(pkg._runfiles, ['.d.ts']).filter((f) => !f.startsWith('node_modules/'));
803+
const dtsStarlark = dtsSources.length ?
804+
starlarkFiles('srcs', dtsSources, `# ${pkg._dir} package declaration files (and declaration files in nested node_modules)`) :
805+
'';
806+
// Flattened list of direct and transitive dependencies hoisted to root by the package manager
807+
const deps = [pkg].concat(pkg._dependencies.filter(dep => dep !== pkg && !dep._isNested));
808+
const depsStarlark = deps.map(dep => `"//${dep._dir}:${dep._name}__contents",`).join('\n ');
826809
let result = `load("@build_bazel_rules_nodejs//internal/npm_install:node_module_library.bzl", "node_module_library")
827810
828811
# Generated targets for npm package "${pkg._dir}"
829812
${printJson(pkg)}
830813
814+
# Files that are part of the npm package not including its nested node_modules
815+
# (filtered by the 'included_files' attribute)
831816
filegroup(
832-
name = "${pkg._name}__files",${filesStarlark}
817+
name = "${pkg._name}__files",${pkgFilesStarlark}
833818
)
834819
820+
# Files that are in the npm package's nested node_modules
821+
# (filtered by the 'included_files' attribute)
835822
filegroup(
836823
name = "${pkg._name}__nested_node_modules",${nestedNodeModulesStarlark}
824+
visibility = ["//:__subpackages__"],
825+
)
826+
827+
# Files that have been excluded from the ${pkg._name}__files target above because
828+
# they are filtered out by 'included_files' or because they are not valid runfiles
829+
# See https://github.com/bazelbuild/bazel/issues/4327.
830+
filegroup(
831+
name = "${pkg._name}__not_files",${notPkgFilesStarlark}
837832
visibility = ["//visibility:private"],
838833
)
839834
835+
# All of the files in the npm package including files that have been
836+
# filtered out by 'included_files' or because they are not valid runfiles
837+
# but not including nested node_modules.
840838
filegroup(
841839
name = "${pkg._name}__all_files",
842-
srcs = [":${pkg._name}__files", ":${pkg._name}__nested_node_modules"],
843-
visibility = ["//:__subpackages__"],
840+
srcs = [":${pkg._name}__files", ":${pkg._name}__not_files"],
844841
)
845842
843+
# The primary target for this package for use in rule deps
846844
node_module_library(
847845
name = "${pkg._name}",
848846
# direct sources listed for strict deps support
849-
srcs = [":${pkg._name}__all_files"],${depsStarlark}
847+
srcs = [":${pkg._name}__files"],
848+
# nested node_modules for this package plus flattened list of direct and transitive dependencies
849+
# hoisted to root by the package manager
850+
deps = [
851+
${depsStarlark}
852+
],
850853
)
851854
852-
# ${pkg._name}__contents target is used as dep for main targets to prevent
853-
# circular dependencies errors
855+
# Target is used as dep for main targets to prevent circular dependencies errors
854856
node_module_library(
855857
name = "${pkg._name}__contents",
856-
srcs = [":${pkg._name}__all_files"],${namedSourcesStarlark}
858+
srcs = [":${pkg._name}__files", ":${pkg._name}__nested_node_modules"],${namedSourcesStarlark}
857859
visibility = ["//:__subpackages__"],
858860
)
859861
860-
# ${pkg._name}__typings is the subset of ${pkg._name}__contents that are declarations
862+
# Typings files that are part of the npm package not including nested node_modules
861863
node_module_library(
862864
name = "${pkg._name}__typings",${dtsStarlark}
863865
)
@@ -1013,10 +1015,10 @@ def ${name.replace(/-/g, '_')}_test(**kwargs):
10131015
});
10141016
// filter out duplicate deps
10151017
deps = [...pkgs, ...new Set(deps)];
1016-
let filesStarlark = '';
1018+
let pkgFilesStarlark = '';
10171019
if (deps.length) {
1018-
const list = deps.map(dep => `"//${dep._dir}:${dep._name}__all_files",`).join('\n ');
1019-
filesStarlark = `
1020+
const list = deps.map(dep => `"//${dep._dir}:${dep._name}__files",`).join('\n ');
1021+
pkgFilesStarlark = `
10201022
# direct sources listed for strict deps support
10211023
srcs = [
10221024
${list}
@@ -1035,7 +1037,7 @@ def ${name.replace(/-/g, '_')}_test(**kwargs):
10351037
10361038
# Generated target for npm scope ${scope}
10371039
node_module_library(
1038-
name = "${scope}",${filesStarlark}${depsStarlark}
1040+
name = "${scope}",${pkgFilesStarlark}${depsStarlark}
10391041
)
10401042
10411043
`;

0 commit comments

Comments
 (0)