fix: improve the entry point refactoring

Co-authored-by: @gregmagolan
Closes: bazel-contrib#32

README.md

@@ -381,19 +381,8 @@ For example, the `protractor` package has two bin entries in its `package.json`:
 These will result in two generated `nodejs_binary` targets in the `@npm//protractor/bin`
 package (if your npm deps workspace is `@npm`):
 
-```python
-nodejs_binary(
-    name = "protractor",
-    entry_point = "protractor/bin/protractor",
-    data = ["//protractor"],
-)
-
-nodejs_binary(
-    name = "webdriver-manager",
-    entry_point = "protractor/bin/webdriver-manager",
-    data = ["//protractor"],
-)
-```
+* `@npm//protractor/bin:protractor`
+* `@npm//protractor/bin:webdriver-manager`
 
 These targets can be used as executables for actions in custom rules or can
 be run by Bazel directly. For example, you can run protractor with the
@@ -513,7 +502,7 @@ load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")
 
 nodejs_binary(
     name = "rollup",
-    entry_point = "rollup/bin/rollup",
+    entry_point = "//:node_modules/rollup/bin/rollup",
 )
 ```
 
@@ -541,7 +530,7 @@ nodejs_binary(
         "@//:node_modules",
         "main.js",
     ],
-    entry_point = "workspace_name/main.js",
+    entry_point = ":main.js",
     args = ["--node_options=--expose-gc"],
 )
 ```

e2e/ts_library/googmodule/BUILD.bazel

@@ -13,7 +13,7 @@ nodejs_binary(
         "@npm//@bazel/typescript",
         "@npm//tsickle",
     ],
-    entry_point = "@bazel/typescript/internal/tsc_wrapped/tsc_wrapped.js",
+    entry_point = "@npm//node_modules/@bazel/typescript:internal/tsc_wrapped/tsc_wrapped.js",
     install_source_map_support = False,
 )
 

e2e/ts_library/some_module/BUILD.bazel

@@ -25,7 +25,7 @@ nodejs_binary(
         ":main",
         ":some_module",
     ],
-    entry_point = "e2e_ts_library/some_module/main.js",
+    entry_point = ":main.js",
 )
 
 sh_test(

examples/program/BUILD.bazel

@@ -38,7 +38,7 @@ nodejs_binary(
         "index.js",
         ":node_modules",
     ],
-    entry_point = "examples_program/index.js",
+    entry_point = ":index.js",
 )
 
 jasmine_node_test(

internal/e2e/fine_grained_no_bin/BUILD.bazel

@@ -11,5 +11,5 @@ nodejs_binary(
         "index.js",
         "@fine_grained_no_bin//fs.realpath",
     ],
-    entry_point = "build_bazel_rules_nodejs/internal/e2e/fine_grained_no_bin/index.js",
+    entry_point = ":index.js",
 )

internal/e2e/rollup/BUILD.bazel

@@ -7,7 +7,7 @@ rollup_bundle(
         "bar.js",
         "foo.js",
     ],
-    entry_point = "foo.js",
+    entry_point = "internal/e2e/rollup/foo.js",
     globals = {"some_global_var": "runtime_name_of_global_var"},
     license_banner = ":license.txt",
     deps = [

internal/e2e/rollup_code_splitting/BUILD.bazel

@@ -7,7 +7,7 @@ rollup_bundle(
         exclude = ["*.spec.js"],
     ),
     additional_entry_points = ["internal/e2e/rollup_code_splitting/additional_entry.js"],
-    entry_point = "//internal/e2e/rollup_code_splitting/main1.js",
+    entry_point = "internal/e2e/rollup_code_splitting/main1.js",
     license_banner = ":license.txt",
 )
 

internal/e2e/rollup_fine_grained_deps/BUILD.bazel

@@ -5,15 +5,15 @@ load("//:defs.bzl", "jasmine_node_test", "nodejs_binary", "rollup_bundle")
 rollup_bundle(
     name = "bundle_no_deps",
     srcs = ["no-deps.js"],
-    entry_point = "@build_bazel_rules_nodejs//internal/e2e/rollup_fine_grained_deps:no-deps.js",
+    entry_point = "internal/e2e/rollup_fine_grained_deps/no-deps.js",
 )
 
 # You can have a rollup_bundle with no node_modules attribute
 # and fine grained deps
 rollup_bundle(
     name = "bundle",
     srcs = ["has-deps.js"],
-    entry_point = "@build_bazel_rules_nodejs//internal/e2e/rollup_fine_grained_deps:has-deps.js",
+    entry_point = "internal/e2e/rollup_fine_grained_deps/has-deps.js",
     deps = [
         "@fine_grained_deps_yarn//@gregmagolan/test-a",
         "@fine_grained_deps_yarn//@gregmagolan/test-b",
@@ -25,7 +25,7 @@ rollup_bundle(
 rollup_bundle(
     name = "bundle_legacy",
     srcs = ["has-deps.js"],
-    entry_point = "@build_bazel_rules_nodejs//internal/e2e/rollup_fine_grained_deps:has-deps.js",
+    entry_point = "internal/e2e/rollup_fine_grained_deps/has-deps.js",
     node_modules = "@fine_grained_deps_yarn//:node_modules",
 )
 
@@ -34,7 +34,7 @@ rollup_bundle(
 rollup_bundle(
     name = "bundle_hybrid",
     srcs = ["has-deps.js"],
-    entry_point = "@build_bazel_rules_nodejs//internal/e2e/rollup_fine_grained_deps:has-deps.js",
+    entry_point = "internal/e2e/rollup_fine_grained_deps/has-deps.js",
     node_modules = "@fine_grained_deps_yarn//:node_modules",
     deps = [
         "@fine_grained_deps_yarn//@gregmagolan/test-a",
@@ -93,6 +93,6 @@ nodejs_binary(
         "@npm//jasmine",
         "@npm//unidiff",
     ],
-    entry_point = "build_bazel_rules_nodejs/internal/e2e/rollup_fine_grained_deps/update_golden.js",
+    entry_point = ":update_golden.js",
     install_source_map_support = False,
 )

internal/history-server/history_server.bzl

@@ -26,7 +26,7 @@ def history_server(templated_args = [], **kwargs):
 
     nodejs_binary_macro(
         node_modules = "@history-server_runtime_deps//:node_modules",
-        entry_point = "history-server/modules/cli.js",
+        entry_point = "@history-server_runtime_deps//node_modules/history-server:modules/cli.js",
         install_source_map_support = False,
         templated_args = templated_args,
         **kwargs

internal/http-server/http_server.bzl

@@ -29,7 +29,7 @@ def http_server(templated_args = [], **kwargs):
 
     nodejs_binary_macro(
         node_modules = "@http-server_runtime_deps//:node_modules",
-        entry_point = "http-server/bin/http-server",
+        entry_point = "@http-server_runtime_deps//node_modules/http-server:bin/http-server",
         install_source_map_support = False,
         templated_args = templated_args,
         **kwargs

internal/jasmine_node_test/test/BUILD.bazel

@@ -30,5 +30,5 @@ nodejs_test(
         "no_jasmine_test.js",
         "//internal/jasmine_node_test:jasmine_runner.js",
     ],
-    entry_point = "build_bazel_rules_nodejs/internal/jasmine_node_test/test/no_jasmine_test.js",
+    entry_point = ":no_jasmine_test.js",
 )

internal/node/node.bzl

@@ -20,7 +20,7 @@ They support module mapping: any targets in the transitive dependencies with
 a `module_name` attribute can be `require`d by that name.
 """
 
-load("//internal/common:expand_into_runfiles.bzl", "expand_location_into_runfiles")
+load("//internal/common:expand_into_runfiles.bzl", "expand_location_into_runfiles", "expand_path_into_runfiles")
 load("//internal/common:module_mappings.bzl", "module_mappings_runtime_aspect")
 load("//internal/common:node_module_info.bzl", "NodeModuleInfo", "collect_node_modules_aspect")
 load("//internal/common:sources_aspect.bzl", "sources_aspect")
@@ -77,22 +77,6 @@ def _write_loader_script(ctx):
     if len(ctx.attr.entry_point.files) != 1:
         fail("labels in entry_point must contain exactly one file")
 
-    entry_point = ctx.file.entry_point
-
-    print(">>>>>")
-    print("basename=(%s)" % entry_point.basename)
-    print("dirname=(%s)" % entry_point.dirname)
-    print("extension=(%s)" % entry_point.extension)
-    print("is_source=(%s)" % entry_point.is_source)
-    print("owner=(%s)" % entry_point.owner)
-    print("path=(%s)" % entry_point.path)
-    print("root=(%s)" % entry_point.root)
-    print("root.path=(%s)" % entry_point.root.path)
-    print("short_path=(%s)" % entry_point.short_path)
-    print("attr.entry_point=(%s)" % ctx.attr.entry_point.label)
-    print("ctx.label=(%s)" % str(ctx.label))
-    print("<<<<<")
-
     ctx.actions.expand_template(
         template = ctx.file._loader_template,
         output = ctx.outputs.loader,
@@ -101,7 +85,7 @@ def _write_loader_script(ctx):
             "TEMPLATED_bootstrap": "\n  " + ",\n  ".join(
                 ["\"" + d + "\"" for d in ctx.attr.bootstrap],
             ),
-            "TEMPLATED_entry_point": entry_point.short_path,
+            "TEMPLATED_entry_point": expand_path_into_runfiles(ctx, ctx.file.entry_point.short_path),
             "TEMPLATED_gen_dir": ctx.genfiles_dir.path,
             "TEMPLATED_install_source_map_support": str(ctx.attr.install_source_map_support).lower(),
             "TEMPLATED_module_roots": "\n  " + ",\n  ".join(module_mappings),
@@ -170,7 +154,7 @@ def _nodejs_binary_impl(ctx):
         is_executable = True,
     )
 
-    runfiles = depset([node, ctx.outputs.loader, ctx.file._repository_args] + node_modules + ctx.files._node_runfiles, transitive = [sources])
+    runfiles = depset([node, ctx.outputs.loader, ctx.file._repository_args, ctx.file.entry_point] + node_modules + ctx.files._node_runfiles, transitive = [sources])
 
     return [DefaultInfo(
         executable = ctx.outputs.script,
@@ -240,6 +224,7 @@ _NODEJS_EXECUTABLE_ATTRS = {
         ```
 
         """,
+        mandatory = True,
         allow_single_file = True,
     ),
     "install_source_map_support": attr.bool(

internal/node/node_loader.js

@@ -25,7 +25,7 @@
 var path = require('path');
 var fs = require('fs');
 
-const DEBUG = true;
+const DEBUG = false;
 
 /**
  * The module roots as pairs of a RegExp to match the require path, and a
@@ -45,16 +45,26 @@ var BOOTSTRAP = [TEMPLATED_bootstrap];
 const USER_WORKSPACE_NAME = 'TEMPLATED_user_workspace_name';
 const NODE_MODULES_ROOT = 'TEMPLATED_node_modules_root';
 const BIN_DIR = 'TEMPLATED_bin_dir';
+const ENTRY_POINT = 'TEMPLATED_entry_point';
 const GEN_DIR = 'TEMPLATED_gen_dir';
+const INSTALL_SOURCE_MAP_SUPPORT = TEMPLATED_install_source_map_support;
+const TARGET = 'TEMPLATED_target';
 
 if (DEBUG)
   console.error(`
-node_loader: running TEMPLATED_target with
-  MODULE_ROOTS: ${JSON.stringify(MODULE_ROOTS, undefined, 2)}
-  BOOTSTRAP: ${JSON.stringify(BOOTSTRAP, undefined, 2)}
-  NODE_MODULES_ROOT: ${NODE_MODULES_ROOT}
+node_loader: running ${TARGET} with
+  cwd: ${process.cwd()}
+  runfiles: ${process.env.RUNFILES}
+
   BIN_DIR: ${BIN_DIR}
+  BOOTSTRAP: ${JSON.stringify(BOOTSTRAP, undefined, 2)}
+  ENTRY_POINT: ${ENTRY_POINT}
   GEN_DIR: ${GEN_DIR}
+  INSTALL_SOURCE_MAP_SUPPORT: ${INSTALL_SOURCE_MAP_SUPPORT}
+  MODULE_ROOTS: ${JSON.stringify(MODULE_ROOTS, undefined, 2)}
+  NODE_MODULES_ROOT: ${NODE_MODULES_ROOT}
+  TARGET: ${TARGET}
+  USER_WORKSPACE_NAME: ${USER_WORKSPACE_NAME}
 `);
 
 function resolveToModuleRoot(path) {
@@ -326,7 +336,8 @@ function resolveRunfiles(parent, ...pathSegments) {
 }
 
 var originalResolveFilename = module.constructor._resolveFilename;
-module.constructor._resolveFilename = function(request, parent) {
+module.constructor._resolveFilename =
+    function(request, parent) {
   const parentFilename = (parent && parent.filename) ? parent.filename : undefined;
   if (DEBUG)
     console.error(`node_loader: resolve ${request} from ${parentFilename}`);
@@ -446,15 +457,15 @@ module.constructor._resolveFilename = function(request, parent) {
   }
 
   const error = new Error(
-      `TEMPLATED_target cannot find module '${request}' required by '${parentFilename}'\n  looked in:\n` +
+      `${TARGET} cannot find module '${request}' required by '${parentFilename}'\n  looked in:\n` +
       failedResolutions.map(r => `    ${r}`).join('\n') + '\n');
   error.code = 'MODULE_NOT_FOUND';
   throw error;
 }
 
 // Before loading anything that might print a stack, install the
 // source-map-support.
-if (TEMPLATED_install_source_map_support) {
+if (INSTALL_SOURCE_MAP_SUPPORT) {
   try {
     const sourcemap_support_package = path.resolve(process.cwd(),
           '../build_bazel_rules_nodejs/third_party/github.com/source-map-support');
@@ -463,7 +474,7 @@ if (TEMPLATED_install_source_map_support) {
     if (DEBUG) {
       console.error(`WARNING: source-map-support module not installed.
       Stack traces from languages like TypeScript will point to generated .js files.
-      Set install_source_map_support = False in TEMPLATED_target to turn off this warning.
+      Set install_source_map_support = False in ${TARGET} to turn off this warning.
       `);
     }
   }
@@ -482,7 +493,7 @@ if (require.main === module) {
   // Set the actual entry point in the arguments list.
   // argv[0] == node, argv[1] == entry point.
   // NB: entry_point below is replaced during the build process.
-  var mainScript = process.argv[1] = 'TEMPLATED_entry_point';
+  var mainScript = process.argv[1] = ENTRY_POINT;
   try {
     module.constructor._load(mainScript, this, /*isMain=*/true);
   } catch (e) {
@@ -494,7 +505,7 @@ if (require.main === module) {
       // (which is an empty filegroup).
       // See https://github.com/bazelbuild/rules_nodejs/wiki#migrating-to-rules_nodejs-013
       console.error(
-          `\nWARNING: Due to a breaking change in rules_nodejs 0.13.0, target TEMPLATED_target\n` +
+          `\nWARNING: Due to a breaking change in rules_nodejs 0.13.0, target ${TARGET}\n` +
           `must now declare either an explicit node_modules attribute, or\n` +
           `list explicit deps[] or data[] fine grained dependencies on npm labels\n` +
           `if it has any node_modules dependencies.\n` +

internal/node/test/BUILD.bazel

@@ -13,7 +13,7 @@ nodejs_binary(
 nodejs_binary(
     name = "has_deps_legacy",
     data = ["has-deps.js"],
-    entry_point = ":has-deps",
+    entry_point = ":has-deps.js",
     node_modules = "@fine_grained_deps_yarn//:node_modules",
 )
 
@@ -25,7 +25,7 @@ nodejs_binary(
         "has-deps.js",
         "@fine_grained_deps_yarn//typescript",
     ],
-    entry_point = ":has-deps",
+    entry_point = ":has-deps.js",
 )
 
 # You can have a nodejs_binary with both a node_modules attribute
@@ -36,6 +36,6 @@ nodejs_binary(
         "has-deps.js",
         "@fine_grained_deps_yarn//typescript",
     ],
-    entry_point = ":has-deps",
+    entry_point = ":has-deps.js",
     node_modules = "@fine_grained_deps_yarn//:node_modules",
 )

internal/npm_install/generate_build_file.js

@@ -714,7 +714,7 @@ filegroup(
       result += `# Wire up the \`bin\` entry \`${name}\`
 nodejs_binary(
     name = "${name}__bin",
-    entry_point = "//node_modules/${pkg._dir}:${name}",
+    entry_point = ":${path}",
     install_source_map_support = False,
     data = [":${pkg._name}__pkg"],
 )

internal/npm_install/test/BUILD.bazel

@@ -34,6 +34,6 @@ nodejs_binary(
         "@npm//jasmine",
         "@npm//unidiff",
     ],
-    entry_point = "//internal/npm_install/test:update_golden.js",
+    entry_point = ":update_golden.js",
     install_source_map_support = False,
 )

internal/npm_package/BUILD.bazel

@@ -19,7 +19,7 @@ nodejs_binary(
         "//third_party/github.com/gjtorikian/isBinaryFile",
         "@nodejs//:run_npm.sh.template",
     ],
-    entry_point = "@build_bazel_rules_nodejs//internal/npm_package:packager.js",
+    entry_point = ":packager.js",
     install_source_map_support = False,
     node_modules = "@build_bazel_rules_nodejs_npm_install_deps//:node_modules",
     visibility = ["//visibility:public"],