fix(typescript): fix issue with types[] in non-sandboxed tsc

To make worker mode work with TypeScript ambient type discovery, always
zero out the compilerOptions#types in the generated tsconfig - users
should provide these as deps[] instead.

Fixes bazelbuild/rules_typescript#449

common.bazelrc

@@ -15,11 +15,6 @@ build --symlink_prefix=dist/
 # build --symlink_prefix=/
 # however this makes it harder to find outputs.
 
-# This option is changed to true in Bazel 0.27 but exposes a known worker mode bug:
-# https://github.com/bazelbuild/rules_typescript/issues/449
-# While we are fixing that issue, we need to keep this breaking change disabled
-build --incompatible_list_based_execution_strategy_selection=false
-
 # Specifies desired output mode for running tests.
 # Valid values are
 #   'summary' to output only test status summary

e2e/ts_library/WORKSPACE

@@ -26,6 +26,15 @@ load("@build_bazel_rules_nodejs//:defs.bzl", "yarn_install")
 
 yarn_install(
     name = "npm",
+    manual_build_file_contents = """
+filegroup(
+  name = "node_modules_filegroup",
+  srcs = [
+    "//node_modules/@types/hammerjs:hammerjs__files",
+    "//node_modules/@types/jasmine:jasmine__files",
+    "//node_modules/typescript:typescript__files",
+  ],
+)""",
     package_json = "//:package.json",
     yarn_lock = "//:yarn.lock",
 )

e2e/ts_library/reference_types_directive/BUILD.bazel

@@ -7,7 +7,10 @@ ts_library(
     expected_diagnostics = [
         "TS2304: Cannot find name 'Hammer'",
     ],
-    node_modules = "@npm//:node_modules",
+    # Use a filegroup to simulate user-managed node modules
+    # This exercises logic in build_defs.bzl that must preserve
+    # the types[] in tsconfig.json so that jasmine is found.
+    node_modules = "@npm//:node_modules_filegroup",
     tsconfig = ":tsconfig.json",
 )
 

e2e/ts_library/reference_types_directive/tsconfig_types.ts

@@ -1,3 +1,6 @@
+// Cecking that `describe` is a valid symbol here to ensure
+// that @types/jasmine is getting picked up
+describe('', () => {});
 // Since "hammerjs" is not included in the types=[] array in
 // tsconfig, this should result in a compile error: TS2304: Cannot find name 'Hammer'
 console.log(typeof Hammer);

internal/npm_install/test/package/BUILD.bazel

@@ -0,0 +1,15 @@
+# Generated file from yarn_install/npm_install rule.
+# See $(bazel info output_base)/external/build_bazel_rules_nodejs/internal/npm_install/generate_build_file.js
+
+# All rules in other repositories can use these targets
+package(default_visibility = ["//visibility:public"])
+
+load("@build_bazel_rules_nodejs//internal/npm_install:node_module_library.bzl", "node_module_library")
+
+# The node_modules directory in one catch-all node_module_library.
+# NB: Using this target may have bad performance implications if
+# there are many files in target.
+# See https://github.com/bazelbuild/bazel/issues/5153.
+node_module_library(
+    name = "node_modules",
+)

packages/typescript/WORKSPACE

@@ -40,7 +40,7 @@ rules_nodejs_dev_dependencies()
 # With http_archive it only sees releases/download/*.tar.gz urls
 git_repository(
     name = "build_bazel_rules_typescript",
-    commit = "b1fa8ad9d15a4b46da800b33333512e538c69bd4",
+    commit = "78af714217bd2b844d2f75961263be4a9e39773e",
     remote = "http://github.com/bazelbuild/rules_typescript.git",
 )
 

packages/typescript/internal/build_defs.bzl

@@ -23,6 +23,7 @@ load("@build_bazel_rules_typescript//internal:common/tsconfig.bzl", "create_tsco
 load("//internal:ts_config.bzl", "TsConfigInfo")
 
 _DEFAULT_COMPILER = "@npm//@bazel/typescript/bin:tsc_wrapped"
+_DEFAULT_NODE_MODULES = Label("@npm//typescript:typescript__typings")
 
 def _trim_package_node_modules(package_name):
     # trim a package name down to its path prior to a node_modules
@@ -35,6 +36,17 @@ def _trim_package_node_modules(package_name):
         segments += [n]
     return "/".join(segments)
 
+# Detect use of bazel-managed node modules.
+# Return True if the node_modules is the default or a course-grained deps
+#   `node_modules = "@npm//:node_modules"`
+# Return False if the user used a hand-rolled filegroup
+#   `node_modules = "//:my_node_modules"`
+# Note: this works for ts_library since we have a default for node_modules
+#       but wouldn't work for other rules like nodejs_binary
+def _uses_bazel_managed_node_modules(ctx):
+    # If the user put a filegroup as the node_modules it will have no provider
+    return NodeModuleSources in ctx.attr.node_modules
+
 # This function is similar but slightly different than _compute_node_modules_root
 # in /internal/node/node.bzl. TODO(gregmagolan): consolidate these functions
 def _compute_node_modules_root(ctx):
@@ -206,6 +218,19 @@ def tsc_wrapped_tsconfig(
     # Since g3 isn't ready to do this yet
     config["compilerOptions"]["target"] = "es2015"
 
+    # It's fine for users to have types[] in their tsconfig.json to help the editor
+    # know which of the node_modules/@types/* entries to include in the program.
+    # But we don't want TypeScript to do any automatic resolution under tsc_wrapped
+    # because when not run in a sandbox, it will scan the @types directory and find
+    # entries that aren't in the action inputs.
+    # See https://github.com/bazelbuild/rules_typescript/issues/449
+    # This setting isn't shared with g3 because there is no node_modules directory there.
+    # NB: Under user-managed dependencies we don't have a Provider in the node_modules
+    #     so our mechanism to collect typings and put them into the program doesn't work.
+    #     In that case we leave the types[] alone.
+    if _uses_bazel_managed_node_modules(ctx):
+        config["compilerOptions"]["types"] = []
+
     # If the user gives a tsconfig attribute, the generated file should extend
     # from the user's tsconfig.
     # See https://github.com/Microsoft/TypeScript/issues/9876
@@ -349,7 +374,7 @@ ts_library = rule(
                 yarn_lock = "//:yarn.lock",
             )
             """,
-            default = Label("@npm//typescript:typescript__typings"),
+            default = _DEFAULT_NODE_MODULES,
         ),
         "supports_workers": attr.bool(
             doc = """Intended for internal use only.