diff --git a/internal/node/node.bzl b/internal/node/node.bzl index a3bebbe31c..d9cf20f101 100644 --- a/internal/node/node.bzl +++ b/internal/node/node.bzl @@ -172,8 +172,9 @@ def _nodejs_binary_impl(ctx): node_modules_roots = _compute_node_modules_roots(ctx) - if "" in node_modules_roots: - node_modules_root = node_modules_roots[""] + "/node_modules" + node_modules_roots_key = node_modules_roots.keys() + if len(node_modules_roots_key) > 0: + node_modules_root = ":".join([root + "/node_modules" for root in node_modules_roots]) else: # there are no fine grained deps but we still need a node_modules_root even if it is a non-existant one node_modules_root = "build_bazel_rules_nodejs/node_modules" diff --git a/internal/node/require_patch.js b/internal/node/require_patch.js index e88833b737..8575623c53 100644 --- a/internal/node/require_patch.js +++ b/internal/node/require_patch.js @@ -39,6 +39,10 @@ function log_verbose(...m) { if (VERBOSE_LOGS) console.error(`[${path.basename(__filename)}]`, ...m); } +function pretty_json(m) { + return JSON.stringify(m, undefined, 2); +} + /** * The module roots as pairs of a RegExp to match the require path, and a * module_root to substitute for the require path. @@ -54,14 +58,23 @@ const BIN_DIR = 'TEMPLATED_bin_dir'; const GEN_DIR = 'TEMPLATED_gen_dir'; const TARGET = 'TEMPLATED_target'; +/** + * The node_modules roots as a list for importing modules from multiple roots. + * + * There is currently no support for duplicate modules across roots. Avoid + * including identical packages from multiple roots, such as + * ['@npm_a//jest', '@npm_b//jest'] + */ +const NODE_MODULES_ROOTS = NODE_MODULES_ROOT.split(":") + log_verbose(`patching require for ${TARGET} cwd: ${process.cwd()} RUNFILES: ${process.env.RUNFILES} TARGET: ${TARGET} BIN_DIR: ${BIN_DIR} GEN_DIR: ${GEN_DIR} - MODULE_ROOTS: ${JSON.stringify(MODULE_ROOTS, undefined, 2)} - NODE_MODULES_ROOT: ${NODE_MODULES_ROOT} + MODULE_ROOTS: ${pretty_json(MODULE_ROOTS)} + NODE_MODULES_ROOTS: ${pretty_json(NODE_MODULES_ROOT)} USER_WORKSPACE_NAME: ${USER_WORKSPACE_NAME} `); @@ -456,16 +469,18 @@ module.constructor._resolveFilename = } // If import was not resolved above then attempt to resolve - // within the node_modules filegroup in use - try { - const resolved = originalResolveFilename( - resolveRunfiles(undefined, NODE_MODULES_ROOT, request), parent, isMain, options); - log_verbose( - `resolved ${request} within node_modules (${NODE_MODULES_ROOT}) to ` + - `${resolved} from ${parentFilename}`); - return resolved; - } catch (e) { - failedResolutions.push(`node_modules attribute (${NODE_MODULES_ROOT}) - ${e.toString()}`); + // within the node_modules filegroups in use + for (const nodeModulesRoot of NODE_MODULES_ROOTS) { + try { + const resolved = originalResolveFilename( + resolveRunfiles(undefined, nodeModulesRoot, request), parent, isMain, options); + log_verbose( + `resolved ${request} within node_modules (${nodeModulesRoot}) to ` + + `${resolved} from ${parentFilename}`); + return resolved; + } catch (e) { + failedResolutions.push(`node_modules attribute (${nodeModulesRoot}) - ${e.toString()}`); + } } // Print the same error message that vanilla nodejs does. diff --git a/internal/node/test/BUILD.bazel b/internal/node/test/BUILD.bazel index 1bfe3a58d2..ed23b5005d 100644 --- a/internal/node/test/BUILD.bazel +++ b/internal/node/test/BUILD.bazel @@ -90,7 +90,7 @@ nodejs_test( ) nodejs_test( - name = "mode_resolution_test", + name = "module_resolution_test", data = [ "//internal/node/test/lib1", "//internal/node/test/lib2", @@ -209,7 +209,7 @@ write_file( ) nodejs_test( - name = "mode_resolution_built_test", + name = "module_resolution_built_test", data = [ "//internal/node/test/lib1", "//internal/node/test/lib2", @@ -226,6 +226,17 @@ nodejs_test( templated_args = ["--bazel_patch_module_resolver"], ) +nodejs_test( + name = "multi_module_resolution_built_test", + data = [ + "@fine_grained_deps_npm//rxjs", + "@fine_grained_deps_yarn//rxjs", + ], + entry_point = ":multi_module_resolution.spec.js", + # TODO: passes locally without this flag but fails on CircleCI + templated_args = ["--bazel_patch_module_resolver"], +) + nodejs_test( name = "data_resolution_built_test", data = [ diff --git a/internal/node/test/multi_module_resolution.spec.js b/internal/node/test/multi_module_resolution.spec.js new file mode 100644 index 0000000000..7413604b8d --- /dev/null +++ b/internal/node/test/multi_module_resolution.spec.js @@ -0,0 +1,7 @@ +// test node npm resolution with multiple roots +try { + require('rxjs'); +} catch (err) { + console.error('should resolve to one of the rxjs modules by default'); + process.exitCode = 1; +}