Skip to content

Commit

Permalink
fix(builtin): fix for issue 834 (#847)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gregmagolan authored Jun 11, 2019
1 parent 9a95730 commit c0fe512
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 8 deletions.
1 change: 1 addition & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 15 additions & 8 deletions internal/node/node_loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions internal/node/test/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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",
)
1 change: 1 addition & 0 deletions internal/node/test/module-name.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('Awesome APP is launched!');
1 change: 1 addition & 0 deletions scripts/test_all.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit c0fe512

Please sign in to comment.