From 483e9cb546f4da3b95ed31d79b322fa1cb10ffbf Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Tue, 11 Jun 2019 15:31:48 -0700 Subject: [PATCH] fix(builtin): fix for issue 834 Skip module root resolution in node resolver if main entry point. Also don't throw on resolve failure so that resolve does cascade down incase the module_root conflicts with the workspace name for fully resolved requires --- .circleci/config.yml | 1 + internal/node/node_loader.js | 23 +++++++++++++++-------- internal/node/test/BUILD.bazel | 15 +++++++++++++++ internal/node/test/module-name.js | 1 + scripts/test_all.sh | 1 + 5 files changed, 33 insertions(+), 8 deletions(-) create mode 100644 internal/node/test/module-name.js diff --git a/.circleci/config.yml b/.circleci/config.yml index 2c2e24db1a..315344ad3d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -160,6 +160,7 @@ jobs: - run: bazel run //internal/node/test:has_deps_legacy - run: bazel run //internal/node/test:has_deps - run: bazel run //internal/node/test:has_deps_hybrid + - run: bazel run //internal/node/test:module_name_test - run: bazel run //internal/e2e/fine_grained_no_bin:index - run: bazel run @fine_grained_deps_yarn//typescript/bin:tsc - run: bazel run @bazel_workspace_a//:bin diff --git a/internal/node/node_loader.js b/internal/node/node_loader.js index 023350aeb3..9fc148e0ce 100644 --- a/internal/node/node_loader.js +++ b/internal/node/node_loader.js @@ -350,16 +350,23 @@ module.constructor._resolveFilename = function(request, parent, isMain, options) // Attempt to resolve to module root. // This should be the first attempted resolution because: // - it's fairly cheap to check (regex over a small array); - // - will throw if something is wrong and not cascade down; // - it is be very common when there are a lot of packages built from source; - const moduleRoot = resolveToModuleRoot(request); - if (moduleRoot) { - const moduleRootInRunfiles = resolveRunfiles(undefined, moduleRoot); - const filename = module.constructor._findPath(moduleRootInRunfiles, []); - if (!filename) { - throw new Error(`No file ${request} found in module root ${moduleRoot}`); + if (!isMain) { + // Don't resolve to module root if this is the main entry point + // as the main entry point will always be fully qualified with the + // workspace name and full path. + // See https://github.com/bazelbuild/rules_nodejs/issues/834 + const moduleRoot = resolveToModuleRoot(request); + if (moduleRoot) { + const moduleRootInRunfiles = resolveRunfiles(undefined, moduleRoot); + const filename = module.constructor._findPath(moduleRootInRunfiles, []); + if (filename) { + return filename; + } else { + failedResolutions.push( + `module root ${moduleRoot} - No file ${request} found in module root ${moduleRoot}`); + } } - return filename; } // Built-in modules, relative, absolute imports and npm dependencies diff --git a/internal/node/test/BUILD.bazel b/internal/node/test/BUILD.bazel index 4b524b8260..6174270b7f 100644 --- a/internal/node/test/BUILD.bazel +++ b/internal/node/test/BUILD.bazel @@ -1,4 +1,5 @@ load("//:defs.bzl", "nodejs_binary") +load("//internal/js_library:js_library.bzl", "js_library") # You can have a nodejs_binary with no node_modules attribute # and no fine grained deps @@ -49,3 +50,17 @@ nodejs_binary( name = "has_entry_file", entry_point = ":entry_file", ) + +# Coverage for issue https://github.com/bazelbuild/rules_nodejs/issues/834 +# where module_name is equal to the workspace naem +js_library( + name = "module_name_lib", + srcs = ["module-name.js"], + module_name = "build_bazel_rules_nodejs", +) + +nodejs_binary( + name = "module_name_test", + data = [":module_name_lib"], + entry_point = ":module-name.js", +) diff --git a/internal/node/test/module-name.js b/internal/node/test/module-name.js new file mode 100644 index 0000000000..7e4218acbc --- /dev/null +++ b/internal/node/test/module-name.js @@ -0,0 +1 @@ +console.log('Awesome APP is launched!'); diff --git a/scripts/test_all.sh b/scripts/test_all.sh index 8a412f964e..65c90ed366 100755 --- a/scripts/test_all.sh +++ b/scripts/test_all.sh @@ -44,6 +44,7 @@ echo_and_run bazel run //internal/node/test:no_deps echo_and_run bazel run //internal/node/test:has_deps_legacy echo_and_run bazel run //internal/node/test:has_deps echo_and_run bazel run //internal/node/test:has_deps_hybrid +echo_and_run bazel run //internal/node/test:module_name_test echo_and_run bazel run //internal/e2e/fine_grained_no_bin:index echo_and_run bazel run @fine_grained_deps_yarn//typescript/bin:tsc