feat(typescript): warn the user when rootDirs looks wrong in ts_proje… (bazel-contrib#3126)

alexeagle · web-flow · commit 8df86ccb799e · 2022-02-17T14:37:49.000-08:00
* feat(typescript): warn the user when rootDirs looks wrong in ts_project tsconfig

This should help prevent users from falling into confusing typescript failures as often

* code review comments
diff --git a/docs/TypeScript.md b/docs/TypeScript.md
@@ -248,12 +248,16 @@ Any code that works with `tsc` should work with `ts_project` with a few caveats:
   Worse, if you build them separately then the output directory will contain whichever
   one you happened to build most recently. This is highly discouraged.
 
+As a thin wrapper, this rule doesn't try to compensate for behavior of the TypeScript compiler.
+See https://github.com/bazelbuild/rules_nodejs/wiki/Debugging-problems-with-ts_project for notes
+that may help you debug issues.
+
 > Note: in order for TypeScript to resolve relative references to the bazel-out folder,
 > we recommend that the base tsconfig contain a rootDirs section that includes all
 > possible locations they may appear.
 >
 > We hope this will not be needed in some future release of TypeScript.
-> Follow https://github.com/microsoft/TypeScript/issues/37257 for more info.
+> Follow https://github.com/microsoft/TypeScript/issues/37378 for more info.
 >
 > For example, if the base tsconfig file relative to the workspace root is
 > `path/to/tsconfig.json` then you should configure like:
diff --git a/packages/typescript/index.bzl b/packages/typescript/index.bzl
@@ -30,6 +30,50 @@ load("@bazel_skylib//rules:build_test.bzl", "build_test")
 
 ts_config = _ts_config
 
+# Copied from aspect_bazel_lib
+# https://github.com/aspect-build/bazel-lib/blob/main/lib/private/utils.bzl#L73-L82
+# TODO(6.0): depend on that library and remove this copy
+def _to_label(param):
+    """Converts a string to a Label. If Label is supplied, the same label is returned.
+
+    Args:
+        param: a string representing a label or a Label
+    Returns:
+        a Label
+    """
+    param_type = type(param)
+    if param_type == "string":
+        if param.startswith("@"):
+            return Label(param)
+        if param.startswith("//"):
+            return Label("@" + param)
+
+        # resolve the relative label from the current package
+        # if 'param' is in another workspace, then this would return the label relative to that workspace, eg:
+        # `Label("@my//foo:bar").relative("@other//baz:bill") == Label("@other//baz:bill")`
+        if param.startswith(":"):
+            param = param[1:]
+        if native.package_name():
+            return Label("@//" + native.package_name()).relative(param)
+        else:
+            return Label("@//:" + param)
+
+    elif param_type == "Label":
+        return param
+    else:
+        fail("Expected 'string' or 'Label' but got '%s'" % param_type)
+
+# copied from aspect_bazel_lib, see comment above
+def _is_external_label(param):
+    """Returns True if the given Label (or stringy version of a label) represents a target outside of the workspace
+
+    Args:
+        param: a string or label
+    Returns:
+        a bool
+    """
+    return len(_to_label(param).workspace_root) > 0
+
 _DEFAULT_TYPESCRIPT_PACKAGE = (
     # BEGIN-INTERNAL
     "@npm" +
@@ -96,12 +140,16 @@ def ts_project(
       Worse, if you build them separately then the output directory will contain whichever
       one you happened to build most recently. This is highly discouraged.
 
+    As a thin wrapper, this rule doesn't try to compensate for behavior of the TypeScript compiler.
+    See https://github.com/bazelbuild/rules_nodejs/wiki/Debugging-problems-with-ts_project for notes
+    that may help you debug issues.
+
     > Note: in order for TypeScript to resolve relative references to the bazel-out folder,
     > we recommend that the base tsconfig contain a rootDirs section that includes all
     > possible locations they may appear.
     >
     > We hope this will not be needed in some future release of TypeScript.
-    > Follow https://github.com/microsoft/TypeScript/issues/37257 for more info.
+    > Follow https://github.com/microsoft/TypeScript/issues/37378 for more info.
     >
     > For example, if the base tsconfig file relative to the workspace root is
     > `path/to/tsconfig.json` then you should configure like:
@@ -434,6 +482,7 @@ def ts_project(
                 allow_js = allow_js,
                 tsconfig = tsconfig,
                 extends = extends,
+                has_local_deps = len([d for d in deps if not _is_external_label(d)]) > 0,
                 **common_kwargs
             )
             tsc_deps = tsc_deps + ["_validate_%s_options" % name]
diff --git a/packages/typescript/internal/ts_project_options_validator.ts b/packages/typescript/internal/ts_project_options_validator.ts
@@ -1,4 +1,4 @@
-import {relative} from 'path';
+import {dirname, relative} from 'path';
 import * as ts from 'typescript';
 
 const diagnosticsHost: ts.FormatDiagnosticsHost = {
@@ -19,12 +19,12 @@ function main([tsconfigPath, output, target, attrsStr]: string[]): 0|1 {
   const {config, error} = ts.readConfigFile(tsconfigPath, ts.sys.readFile);
   if (error) throw new Error(tsconfigPath + ':' + ts.formatDiagnostic(error, diagnosticsHost));
   const {errors, options} =
-      ts.parseJsonConfigFileContent(config, ts.sys, require('path').dirname(tsconfigPath));
+      ts.parseJsonConfigFileContent(config, ts.sys, dirname(tsconfigPath));
   // We don't pass the srcs to this action, so it can't know if the program has the right sources.
   // Diagnostics look like
   // error TS18002: The 'files' list in config file 'tsconfig.json' is empty.
   // error TS18003: No inputs were found in config file 'tsconfig.json'. Specified 'include'...
-  const fatalErrors = errors.filter(e => e.code !== 18002 && e.code != 18003);
+  const fatalErrors = errors.filter(e => e.code !== 18002 && e.code !== 18003);
   if (fatalErrors.length > 0)
     throw new Error(tsconfigPath + ':' + ts.formatDiagnostics(fatalErrors, diagnosticsHost));
 
@@ -93,6 +93,28 @@ function main([tsconfigPath, output, target, attrsStr]: string[]): 0|1 {
     console.error('- Otherwise, remove the noEmit option from tsconfig and try again.');
     return 1;
   } 
+  
+  // When there are dependencies on other ts_project targets, the tsconfig must be configured
+  // to help TypeScript resolve them.
+  if (attrs.has_local_deps) {
+    let rootDirsValid = true
+    if (!options.rootDirs) {
+      console.error(`ERROR: ts_project rule ${target} is configured without rootDirs.`);
+      rootDirsValid = false;
+    }
+    else if (!options.rootDirs.some(d => d.startsWith(process.env['BINDIR']))) {
+      console.error(`ERROR: ts_project rule ${target} is missing a needed rootDir under ${process.env['BINDIR']}.`);
+      console.error('Found only: ', options.rootDirs);
+      rootDirsValid = false;
+    }
+    if (!rootDirsValid) {
+      console.error('This makes it likely that TypeScript will be unable to resolve dependencies using relative import paths');
+      console.error(`For example, if you 'import {} from ./foo', this expects to resolve foo.d.ts from Bazel's output tree`);
+      console.error('and TypeScript only knows how to locate this when the rootDirs attribute includes that path.');
+      console.error('See the ts_project documentation: https://bazelbuild.github.io/rules_nodejs/TypeScript.html#ts_project');
+      return 1;
+    }
+  }
 
   check('allowJs', 'allow_js');
   check('declarationMap', 'declaration_map');
diff --git a/packages/typescript/internal/validate_options.bzl b/packages/typescript/internal/validate_options.bzl
@@ -29,6 +29,7 @@ def _validate_options_impl(ctx):
         allow_js = ctx.attr.allow_js,
         declaration = ctx.attr.declaration,
         declaration_map = ctx.attr.declaration_map,
+        has_local_deps = ctx.attr.has_local_deps,
         preserve_jsx = ctx.attr.preserve_jsx,
         composite = ctx.attr.composite,
         emit_declaration_only = ctx.attr.emit_declaration_only,
@@ -47,6 +48,9 @@ def _validate_options_impl(ctx):
         outputs = [marker],
         arguments = [arguments],
         executable = "validator",
+        env = {
+            "BINDIR": ctx.var["BINDIR"],
+        },
     )
     return [
         ValidOptionsInfo(marker = marker),
@@ -69,6 +73,7 @@ SHARED_ATTRS = {
 validate_options = rule(
     implementation = _validate_options_impl,
     attrs = dict(SHARED_ATTRS, **{
+        "has_local_deps": attr.bool(doc = "Whether any of the deps are in the local workspace (not starting with '@')"),
         "target": attr.string(),
         "ts_build_info_file": attr.string(),
         "tsconfig": attr.label(mandatory = True, allow_single_file = [".json"]),
