Skip to content

Commit 2c2cc6e

Browse files
Greg Magolangregmagolan
authored andcommitted
feat: support for nested node_modules in linker
This PR adds support for nested node_modules folders in the linker, which allows targets to depend on npm packages from multiple yarn_install & npm_install repository rules (max one per directory in the workspace). This is an opt-in feature which is enabled by specifying package_path in yarn_install or npm_install for nested package.json files. Setting package_path informs the linker to link the external npm repository to a node_modules folder in the package_path sub-directory specified. For example, given two yarn_install rules: ``` yarn_install( name = "npm", package_json = "//:package.json", yarn_lock = "//:yarn.lock", ) yarn_install( name = "npm_subdir", package_json = "//subdir:package.json", package_path = "subdir", yarn_lock = "//subdir:yarn.lock", ) ``` a target may depend on npm packages from both, ``` jasmine_node_test( name = "test", srcs = ["test.js"], deps = [ "@npm//foo", "@npm_subdir//bar", ], ) ``` and the linker will link multiple 3rd party node_modules folders, ``` /node_modules => @npm//:node_modules /subdir/node_modules => @npm_subdir/:node_modules ``` making the 3rd party npm deps foo & bar available at ``` /node_modules/foo /subdir/node_modules/bar ``` This feature is opt-in as package_path currently defaults to "". In a future major release (4.0.0), package_path will default to the directory the package.json file is in, which will turn it on by default. This feature is an alternative to yarn workspaces & npm workspaces, which also lay out node_modules in multiple directories from multiple package.json files. To date, targets have been limited to depending on and resolving form a single yarn_install or npm_install workspace which was always linked to a node_modules folder at the root of the workspace. With this change, the linker is able to link to both the root of the workspace as well as sub-directories in the workspace so targets may now resolve 3rd party npm deps from multiple package.json files which correspond to multiple node_modules folders in the tree. 1st party deps are still always linked to the root node_modules folder as they were before this change. NB: depending on multiple npm workspaces will not be supported by ts_library as its resolution logic is limited to a single node_modules root NB: pre-linker node require patches (deprecated as of 3.0.0) will only include root node_modules; no nested node_modules shall included in require patch resolution
1 parent f557f9d commit 2c2cc6e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+974
-701
lines changed

WORKSPACE

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ workspace(
1919
# cypress_deps must be a managed directory to ensure it is downloaded before cypress_repository is run.
2020
"@cypress_deps": ["packages/cypress/test/node_modules"],
2121
"@npm": ["node_modules"],
22+
"@npm_internal_linker_test_multi_linker": ["internal/linker/test/multi_linker/node_modules"],
2223
"@npm_node_patches": ["packages/node-patches/node_modules"],
2324
},
2425
)
@@ -57,6 +58,21 @@ yarn_install(
5758
yarn_lock = "//:yarn.lock",
5859
)
5960

61+
yarn_install(
62+
name = "npm_internal_linker_test_multi_linker",
63+
package_json = "//internal/linker/test/multi_linker:package.json",
64+
package_path = "internal/linker/test/multi_linker",
65+
yarn_lock = "//internal/linker/test/multi_linker:yarn.lock",
66+
)
67+
68+
yarn_install(
69+
name = "onepa_npm_deps",
70+
package_json = "//internal/linker/test/multi_linker/onepa:package.json",
71+
package_path = "internal/linker/test/multi_linker/onepa",
72+
symlink_node_modules = False,
73+
yarn_lock = "//internal/linker/test/multi_linker/onepa:yarn.lock",
74+
)
75+
6076
npm_install(
6177
name = "npm_node_patches",
6278
package_json = "//packages/node-patches:package.json",

internal/bazel_integration_test/test_runner.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -358,11 +358,10 @@ for (const bazelCommand of config.bazelCommands) {
358358
'BASH_FUNC_is_absolute%%',
359359
'BASH_FUNC_rlocation%%',
360360
'BASH_FUNC_runfiles_export_envvars%%',
361-
'BAZEL_NODE_MODULES_ROOT',
361+
'BAZEL_NODE_MODULES_ROOTS',
362362
'BAZEL_NODE_PATCH_REQUIRE',
363363
'BAZEL_NODE_RUNFILES_HELPER',
364-
'BAZEL_PATCH_GUARDS',
365-
'BAZEL_PATCH_ROOT',
364+
'BAZEL_PATCH_ROOTS',
366365
'BAZEL_TARGET',
367366
'BAZEL_WORKSPACE',
368367
'BAZELISK_SKIP_WRAPPER',

internal/js_library/js_library.bzl

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ _ATTRS = {
3838
),
3939
"deps": attr.label_list(),
4040
"external_npm_package": attr.bool(
41-
doc = """Indictates that this js_library target is one or more external npm packages in node_modules.
41+
doc = """Internal use only. Indictates that this js_library target is one or more external npm packages in node_modules.
4242
This is used by the yarn_install & npm_install repository rules for npm dependencies installed by
4343
yarn & npm. When true, js_library will provide ExternalNpmPackageInfo.
4444
@@ -73,6 +73,12 @@ _ATTRS = {
7373
See `examples/user_managed_deps` for a working example of user-managed npm dependencies.""",
7474
default = False,
7575
),
76+
"external_npm_package_path": attr.string(
77+
doc = """Internal use only. The local workspace path that the linker should link these node_modules to.
78+
79+
Used only when external_npm_package is True. If empty, the linker will link these node_modules at the root.""",
80+
default = "",
81+
),
7682
"is_windows": attr.bool(
7783
doc = "Internal use only. Automatically set by macro",
7884
mandatory = True,
@@ -230,6 +236,7 @@ def _impl(ctx):
230236
direct_sources = depset(transitive = direct_sources_depsets),
231237
sources = depset(transitive = npm_sources_depsets),
232238
workspace = workspace_name,
239+
path = ctx.attr.external_npm_package_path,
233240
))
234241

235242
# Don't provide DeclarationInfo if there are no typings to provide.

internal/linker/index.js

Lines changed: 138 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ function mkdirp(p) {
3434
return __awaiter(this, void 0, void 0, function* () {
3535
if (p && !(yield exists(p))) {
3636
yield mkdirp(path.dirname(p));
37-
log_verbose(`mkdir( ${p} )`);
37+
log_verbose(`creating directory ${p} in ${process.cwd()}`);
3838
try {
3939
yield fs.promises.mkdir(p);
4040
}
@@ -98,7 +98,10 @@ function deleteDirectory(p) {
9898
}
9999
function symlink(target, p) {
100100
return __awaiter(this, void 0, void 0, function* () {
101-
log_verbose(`symlink( ${p} -> ${target} )`);
101+
if (!path.isAbsolute(target)) {
102+
target = path.resolve(process.cwd(), target);
103+
}
104+
log_verbose(`creating symlink ${p} -> ${target}`);
102105
try {
103106
yield fs.promises.symlink(target, p, 'junction');
104107
return true;
@@ -117,33 +120,24 @@ function symlink(target, p) {
117120
}
118121
});
119122
}
120-
function resolveRoot(root, startCwd, isExecroot, runfiles) {
123+
function resolveExternalWorkspacePath(workspace, startCwd, isExecroot, execroot, runfiles) {
121124
return __awaiter(this, void 0, void 0, function* () {
122125
if (isExecroot) {
123-
return root ? `${startCwd}/external/${root}` : `${startCwd}/node_modules`;
124-
}
125-
const match = startCwd.match(BAZEL_OUT_REGEX);
126-
if (!match) {
127-
if (!root) {
128-
return `${startCwd}/node_modules`;
129-
}
130-
return path.resolve(`${startCwd}/../${root}`);
126+
return `${execroot}/external/${workspace}`;
131127
}
132-
const symlinkRoot = startCwd.slice(0, match.index);
133-
process.chdir(symlinkRoot);
134-
if (!root) {
135-
return `${symlinkRoot}/node_modules`;
128+
if (!execroot) {
129+
return path.resolve(`${startCwd}/../${workspace}`);
136130
}
137-
const fromManifest = runfiles.lookupDirectory(root);
131+
const fromManifest = runfiles.lookupDirectory(workspace);
138132
if (fromManifest) {
139133
return fromManifest;
140134
}
141135
else {
142-
const maybe = path.resolve(`${symlinkRoot}/external/${root}`);
143-
if (fs.existsSync(maybe)) {
136+
const maybe = path.resolve(`${execroot}/external/${workspace}`);
137+
if (yield exists(maybe)) {
144138
return maybe;
145139
}
146-
return path.resolve(`${startCwd}/../${root}`);
140+
return path.resolve(`${startCwd}/../${workspace}`);
147141
}
148142
});
149143
}
@@ -211,7 +205,7 @@ class Runfiles {
211205
if (result) {
212206
return result;
213207
}
214-
const e = new Error(`could not resolve modulePath ${modulePath}`);
208+
const e = new Error(`could not resolve module ${modulePath}`);
215209
e.code = 'MODULE_NOT_FOUND';
216210
throw e;
217211
}
@@ -272,6 +266,9 @@ function exists(p) {
272266
});
273267
}
274268
function existsSync(p) {
269+
if (!p) {
270+
return false;
271+
}
275272
try {
276273
fs.lstatSync(p);
277274
return true;
@@ -328,19 +325,6 @@ function liftElement(element) {
328325
}
329326
return element;
330327
}
331-
function toParentLink(link) {
332-
return [link[0], path.dirname(link[1])];
333-
}
334-
function allElementsAlign(name, elements) {
335-
if (!elements[0].link) {
336-
return false;
337-
}
338-
const parentLink = toParentLink(elements[0].link);
339-
if (!elements.every(e => !!e.link && isDirectChildLink(parentLink, e.link))) {
340-
return false;
341-
}
342-
return !!elements[0].link && allElementsAlignUnder(name, parentLink, elements);
343-
}
344328
function allElementsAlignUnder(parentName, parentLink, elements) {
345329
for (const { name, link, children } of elements) {
346330
if (!link || children) {
@@ -361,16 +345,10 @@ function allElementsAlignUnder(parentName, parentLink, elements) {
361345
function isDirectChildPath(parent, child) {
362346
return parent === path.dirname(child);
363347
}
364-
function isDirectChildLink([parentRel, parentPath], [childRel, childPath]) {
365-
if (parentRel !== childRel) {
366-
return false;
367-
}
368-
if (!isDirectChildPath(parentPath, childPath)) {
369-
return false;
370-
}
371-
return true;
348+
function isDirectChildLink(parentLink, childLink) {
349+
return parentLink === path.dirname(childLink);
372350
}
373-
function isNameLinkPathTopAligned(namePath, [, linkPath]) {
351+
function isNameLinkPathTopAligned(namePath, linkPath) {
374352
return path.basename(namePath) === path.basename(linkPath);
375353
}
376354
function visitDirectoryPreserveLinks(dirPath, visit) {
@@ -390,28 +368,96 @@ function visitDirectoryPreserveLinks(dirPath, visit) {
390368
}
391369
});
392370
}
371+
function findExecroot(startCwd) {
372+
if (existsSync(`${startCwd}/bazel-out`)) {
373+
return startCwd;
374+
}
375+
const bazelOutMatch = startCwd.match(BAZEL_OUT_REGEX);
376+
return bazelOutMatch ? startCwd.slice(0, bazelOutMatch.index) : undefined;
377+
}
393378
function main(args, runfiles) {
394379
return __awaiter(this, void 0, void 0, function* () {
395380
if (!args || args.length < 1)
396381
throw new Error('requires one argument: modulesManifest path');
397382
const [modulesManifest] = args;
398-
let { bin, root, modules, workspace } = JSON.parse(fs.readFileSync(modulesManifest));
383+
log_verbose('manifest file:', modulesManifest);
384+
let { workspace, bin, roots, modules } = JSON.parse(fs.readFileSync(modulesManifest));
399385
modules = modules || {};
400-
log_verbose('manifest file', modulesManifest);
401-
log_verbose('manifest contents', JSON.stringify({ workspace, bin, root, modules }, null, 2));
386+
log_verbose('manifest contents:', JSON.stringify({ workspace, bin, roots, modules }, null, 2));
402387
const startCwd = process.cwd().replace(/\\/g, '/');
403-
log_verbose('startCwd', startCwd);
404-
const isExecroot = existsSync(`${startCwd}/bazel-out`);
405-
log_verbose('isExecroot', isExecroot.toString());
406-
const rootDir = yield resolveRoot(root, startCwd, isExecroot, runfiles);
407-
log_verbose('resolved node_modules root', root, 'to', rootDir);
408-
log_verbose('cwd', process.cwd());
409-
if (!(yield exists(rootDir))) {
410-
log_verbose('no third-party packages; mkdir node_modules at', root);
411-
yield mkdirp(rootDir);
412-
}
413-
yield symlink(rootDir, 'node_modules');
414-
process.chdir(rootDir);
388+
log_verbose('startCwd:', startCwd);
389+
const execroot = findExecroot(startCwd);
390+
log_verbose('execroot:', execroot ? execroot : 'not found');
391+
const isExecroot = startCwd == execroot;
392+
log_verbose('isExecroot:', isExecroot.toString());
393+
if (!isExecroot && execroot) {
394+
process.chdir(execroot);
395+
log_verbose('changed directory to execroot', execroot);
396+
}
397+
function symlinkWithUnlink(target, p, stats = null) {
398+
return __awaiter(this, void 0, void 0, function* () {
399+
if (!path.isAbsolute(target)) {
400+
target = path.resolve(process.cwd(), target);
401+
}
402+
if (stats === null) {
403+
stats = yield gracefulLstat(p);
404+
}
405+
if (runfiles.manifest && execroot && stats !== null && stats.isSymbolicLink()) {
406+
const symlinkPath = fs.readlinkSync(p).replace(/\\/g, '/');
407+
if (path.relative(symlinkPath, target) != '' &&
408+
!path.relative(execroot, symlinkPath).startsWith('..')) {
409+
log_verbose(`Out-of-date symlink for ${p} to ${symlinkPath} detected. Target should be ${target}. Unlinking.`);
410+
yield unlink(p);
411+
}
412+
}
413+
return symlink(target, p);
414+
});
415+
}
416+
for (const packagePath of Object.keys(roots)) {
417+
const workspace = roots[packagePath];
418+
const workspacePath = yield resolveExternalWorkspacePath(workspace, startCwd, isExecroot, execroot, runfiles);
419+
log_verbose(`resolved ${workspace} workspace path to ${workspacePath}`);
420+
const workspaceNodeModules = `${workspacePath}/node_modules`;
421+
if (packagePath) {
422+
if (yield exists(workspaceNodeModules)) {
423+
let resolvedPackagePath;
424+
if (yield exists(packagePath)) {
425+
yield symlinkWithUnlink(workspaceNodeModules, `${packagePath}/node_modules`);
426+
resolvedPackagePath = packagePath;
427+
}
428+
if (!isExecroot) {
429+
const runfilesPackagePath = `${startCwd}/${packagePath}`;
430+
if (yield exists(runfilesPackagePath)) {
431+
if (resolvedPackagePath) {
432+
yield symlinkWithUnlink(`${resolvedPackagePath}/node_modules`, `${runfilesPackagePath}/node_modules`);
433+
}
434+
else {
435+
yield symlinkWithUnlink(workspaceNodeModules, `${runfilesPackagePath}/node_modules`);
436+
}
437+
resolvedPackagePath = runfilesPackagePath;
438+
}
439+
}
440+
const packagePathBin = `${bin}/${packagePath}`;
441+
if (resolvedPackagePath && (yield exists(packagePathBin))) {
442+
yield symlinkWithUnlink(`${resolvedPackagePath}/node_modules`, `${packagePathBin}/node_modules`);
443+
}
444+
}
445+
}
446+
else {
447+
if (yield exists(workspaceNodeModules)) {
448+
yield symlinkWithUnlink(workspaceNodeModules, `node_modules`);
449+
}
450+
else {
451+
log_verbose('no root npm workspace node_modules folder to link to; creating node_modules directory in', process.cwd());
452+
yield mkdirp('node_modules');
453+
}
454+
}
455+
}
456+
if (!roots || !roots['']) {
457+
log_verbose('no root npm workspace; creating node_modules directory in ', process.cwd());
458+
yield mkdirp('node_modules');
459+
}
460+
process.chdir('node_modules');
415461
function isLeftoverDirectoryFromLinker(stats, modulePath) {
416462
return __awaiter(this, void 0, void 0, function* () {
417463
if (runfiles.manifest === undefined) {
@@ -451,44 +497,43 @@ function main(args, runfiles) {
451497
return __awaiter(this, void 0, void 0, function* () {
452498
yield mkdirp(path.dirname(m.name));
453499
if (m.link) {
454-
const [root, modulePath] = m.link;
500+
const modulePath = m.link;
455501
let target;
456-
switch (root) {
457-
case 'execroot':
458-
if (isExecroot) {
459-
target = `${startCwd}/${modulePath}`;
460-
break;
461-
}
462-
case 'runfiles':
463-
let runfilesPath = modulePath;
464-
if (runfilesPath.startsWith(`${bin}/`)) {
465-
runfilesPath = runfilesPath.slice(bin.length + 1);
466-
}
467-
else if (runfilesPath === bin) {
468-
runfilesPath = '';
469-
}
470-
const externalPrefix = 'external/';
471-
if (runfilesPath.startsWith(externalPrefix)) {
472-
runfilesPath = runfilesPath.slice(externalPrefix.length);
473-
}
474-
else {
475-
runfilesPath = `${workspace}/${runfilesPath}`;
476-
}
477-
try {
478-
target = runfiles.resolve(runfilesPath);
479-
if (runfiles.manifest && root == 'execroot' && modulePath.startsWith(`${bin}/`)) {
480-
if (!target.includes(`/${bin}/`)) {
481-
const e = new Error(`could not resolve modulePath ${modulePath}`);
482-
e.code = 'MODULE_NOT_FOUND';
483-
throw e;
484-
}
502+
if (isExecroot) {
503+
target = `${startCwd}/${modulePath}`;
504+
}
505+
if (!isExecroot || !existsSync(target)) {
506+
let runfilesPath = modulePath;
507+
if (runfilesPath.startsWith(`${bin}/`)) {
508+
runfilesPath = runfilesPath.slice(bin.length + 1);
509+
}
510+
else if (runfilesPath === bin) {
511+
runfilesPath = '';
512+
}
513+
const externalPrefix = 'external/';
514+
if (runfilesPath.startsWith(externalPrefix)) {
515+
runfilesPath = runfilesPath.slice(externalPrefix.length);
516+
}
517+
else {
518+
runfilesPath = `${workspace}/${runfilesPath}`;
519+
}
520+
try {
521+
target = runfiles.resolve(runfilesPath);
522+
if (runfiles.manifest && modulePath.startsWith(`${bin}/`)) {
523+
if (!target.match(BAZEL_OUT_REGEX)) {
524+
const e = new Error(`could not resolve module ${runfilesPath} in output tree`);
525+
e.code = 'MODULE_NOT_FOUND';
526+
throw e;
485527
}
486528
}
487-
catch (err) {
488-
target = undefined;
489-
log_verbose(`runfiles resolve failed for module '${m.name}': ${err.message}`);
490-
}
491-
break;
529+
}
530+
catch (err) {
531+
target = undefined;
532+
log_verbose(`runfiles resolve failed for module '${m.name}': ${err.message}`);
533+
}
534+
}
535+
if (target && !path.isAbsolute(target)) {
536+
target = path.resolve(process.cwd(), target);
492537
}
493538
const stats = yield gracefulLstat(m.name);
494539
const isLeftOver = (stats !== null && (yield isLeftoverDirectoryFromLinker(stats, m.name)));
@@ -497,7 +542,7 @@ function main(args, runfiles) {
497542
yield createSymlinkAndPreserveContents(stats, m.name, target);
498543
}
499544
else {
500-
yield symlink(target, m.name);
545+
yield symlinkWithUnlink(target, m.name, stats);
501546
}
502547
}
503548
else {

0 commit comments

Comments
 (0)