fix: use tree artifacts via copy_directory with exports_directories_only

docs/Providers.md

@@ -15,7 +15,7 @@ Users should not load files under "/internal"
 **USAGE**
 
 <pre>
-ExternalNpmPackageInfo(<a href="#ExternalNpmPackageInfo-direct_sources">direct_sources</a>, <a href="#ExternalNpmPackageInfo-has_directories">has_directories</a>, <a href="#ExternalNpmPackageInfo-path">path</a>, <a href="#ExternalNpmPackageInfo-sources">sources</a>, <a href="#ExternalNpmPackageInfo-workspace">workspace</a>)
+ExternalNpmPackageInfo(<a href="#ExternalNpmPackageInfo-direct_sources">direct_sources</a>, <a href="#ExternalNpmPackageInfo-path">path</a>, <a href="#ExternalNpmPackageInfo-sources">sources</a>, <a href="#ExternalNpmPackageInfo-workspace">workspace</a>)
 </pre>
 
 Provides information about one or more external npm packages
@@ -25,9 +25,6 @@ Provides information about one or more external npm packages
 <h4 id="ExternalNpmPackageInfo-direct_sources">direct_sources</h4>
 
  Depset of direct source files in these external npm package(s) 
-<h4 id="ExternalNpmPackageInfo-has_directories">has_directories</h4>
-
- True if any sources are directories 
 <h4 id="ExternalNpmPackageInfo-path">path</h4>
 
  The local workspace path that these external npm deps should be linked at. If empty, they will be linked at the root. 
@@ -131,7 +128,7 @@ do the same.
 **USAGE**
 
 <pre>
-NpmPackageInfo(<a href="#NpmPackageInfo-direct_sources">direct_sources</a>, <a href="#NpmPackageInfo-has_directories">has_directories</a>, <a href="#NpmPackageInfo-path">path</a>, <a href="#NpmPackageInfo-sources">sources</a>, <a href="#NpmPackageInfo-workspace">workspace</a>)
+NpmPackageInfo(<a href="#NpmPackageInfo-direct_sources">direct_sources</a>, <a href="#NpmPackageInfo-path">path</a>, <a href="#NpmPackageInfo-sources">sources</a>, <a href="#NpmPackageInfo-workspace">workspace</a>)
 </pre>
 
 Provides information about one or more external npm packages
@@ -141,9 +138,6 @@ Provides information about one or more external npm packages
 <h4 id="NpmPackageInfo-direct_sources">direct_sources</h4>
 
  Depset of direct source files in these external npm package(s) 
-<h4 id="NpmPackageInfo-has_directories">has_directories</h4>
-
- True if any sources are directories 
 <h4 id="NpmPackageInfo-path">path</h4>
 
  The local workspace path that these external npm deps should be linked at. If empty, they will be linked at the root. 

examples/from_source/tsconfig.json

@@ -1,5 +1,6 @@
 {
   "compilerOptions": {
+    "skipLibCheck": true,
     "lib": [
       "ES2017",
       "dom"

examples/webapp/BUILD.bazel

@@ -19,7 +19,6 @@ protractor_web_test_suite(
     name = "server_test",
     srcs = ["app.spec.js"],
     on_prepare = ":protractor.on-prepare.js",
-    protractor_entry_point = {"@npm_deps//:node_modules/protractor": "bin/protractor"},
     server = ":server",
 )
 

examples/webapp/WORKSPACE

@@ -30,7 +30,7 @@ load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")
 
 yarn_install(
     name = "npm_deps",
-    exports_directories_only = True,
+    exports_directories_only = False,
     package_json = "//:package.json",
     yarn_lock = "//:yarn.lock",
 )

internal/js_library/js_library.bzl

@@ -149,10 +149,14 @@ def _impl(ctx):
     typings = []
     js_files = []
     named_module_files = []
+    has_directories = False
 
     for idx, f in enumerate(input_files):
         file = f
 
+        if file.is_directory:
+            has_directories = True
+
         # copy files into bin if needed
         if file.is_source and not file.path.startswith("external/"):
             dst = ctx.actions.declare_file(file.basename, sibling = file)
@@ -263,15 +267,14 @@ def _impl(ctx):
         ),
     ]
 
-    if ctx.attr.package_name == "$node_modules$" or ctx.attr.package_name == "$node_modules_dir$":
+    if ctx.attr.package_name == "$node_modules$":
         # special case for external npm deps
         workspace_name = ctx.label.workspace_name if ctx.label.workspace_name else ctx.workspace_name
         providers.append(ExternalNpmPackageInfo(
             direct_sources = depset(transitive = direct_sources_depsets),
             sources = depset(transitive = npm_sources_depsets),
             workspace = workspace_name,
             path = ctx.attr.package_path,
-            has_directories = (ctx.attr.package_name == "$node_modules_dir$"),
         ))
     else:
         providers.append(LinkablePackageInfo(
@@ -290,7 +293,7 @@ def _impl(ctx):
             deps = ctx.attr.deps,
         ))
         providers.append(OutputGroupInfo(types = decls))
-    elif ctx.attr.package_name == "$node_modules_dir$":
+    elif has_directories:
         # If this is directory artifacts npm package then always provide declaration_info
         # since we can't scan through files
         decls = depset(transitive = files_depsets)
@@ -458,7 +461,7 @@ def js_library(
         # module_name for legacy ts_library module_mapping support
         # which is still being used in a couple of tests
         # TODO: remove once legacy module_mapping is removed
-        module_name = package_name if package_name != "$node_modules$" and package_name != "$node_modules_dir$" else None,
+        module_name = package_name if package_name != "$node_modules$" else None,
         is_windows = select({
             "@bazel_tools//src/conditions:host_windows": True,
             "//conditions:default": False,

internal/linker/link_node_modules.bzl

@@ -50,8 +50,22 @@ def _link_mapping(label, mappings, k, v):
         # Map values are of format [deprecated, link_path]
         iter_package_name = iter_key.split(":")[0]
         iter_source_path = iter_values
-        if package_name == iter_package_name and link_path != iter_source_path:
-            fail("conflicting mapping at '%s': '%s' and '%s' map to conflicting %s and %s" % (label, k, iter_key, link_path, iter_source_path))
+        if package_name == iter_package_name:
+            # If we're linking to the output tree be tolerant of linking to different
+            # output trees since we can have "static" links that come from cfg="exec" binaries.
+            # In the future when we static link directly into runfiles without the linker
+            # we can remove this logic.
+            link_path_segments = link_path.split("/")
+            iter_source_path_segments = iter_source_path.split("/")
+            bin_links = link_path_segments[0] == "bazel-out" and iter_source_path_segments[0] == "bazel-out" and link_path_segments[2] == "bin" and iter_source_path_segments[2] == "bin"
+            if bin_links:
+                compare_link_path = "/".join(link_path_segments[3:])
+                compare_iter_source_path = "/".join(iter_source_path_segments[3:])
+            else:
+                compare_link_path = link_path
+                compare_iter_source_path = iter_source_path
+            if compare_link_path != compare_iter_source_path:
+                fail("conflicting mapping at '%s': '%s' and '%s' map to conflicting %s and %s" % (label, k, iter_key, compare_link_path, compare_iter_source_path))
 
     return True
 

internal/linker/test/default_path/custom_external_npm_package_info.bzl

@@ -9,7 +9,6 @@ def _custom_external_npm_package_info_impl(ctx):
     return ExternalNpmPackageInfo(
         direct_sources = depset([], transitive = ctx.files.deps),
         sources = depset(ctx.files.srcs, transitive = ctx.files.deps),
-        has_directories = False,
         workspace = "npm",
         # `path` is intentionally **not** provided.
         # Historical note: `ExternalNpmPackageInfo` was publicly exported prior

internal/node/test/BUILD.bazel

@@ -502,32 +502,6 @@ nodejs_test(
     entry_point = ":main.js",
 )
 
-# Test that we can run a nodejs_binary with --bazel_patch_module_resolver with exports_directories_only = True
-nodejs_binary(
-    name = "protractor_directory_artifacts_version",
-    data = ["@npm_directory_artifacts//protractor"],
-    entry_point = {"@npm_directory_artifacts//:node_modules/protractor": "bin/protractor"},
-    tags = ["requires-runfiles"],
-    templated_args = [
-        "--bazel_patch_module_resolver",
-        "--version",
-    ],
-)
-
-npm_package_bin(
-    name = "protractor_directory_artifacts_version_output",
-    stdout = "protractor_directory_artifacts_version.out",
-    tags = ["requires-runfiles"],
-    tool = ":protractor_directory_artifacts_version",
-)
-
-generated_file_test(
-    name = "protractor_directory_artifacts_version_test",
-    src = "protractor_directory_artifacts_version.golden",
-    generated = ":protractor_directory_artifacts_version.out",
-    tags = ["requires-runfiles"],
-)
-
 # Targets that can help with debugging and ensuring that node versions can be specified
 # for nodejs_binary and nodejs_test
 # Used in nodejs_bianry later to help see which version of node is run