Support TypeScript workers with isolated declarations and custom TypeScript

ts/defs.bzl

@@ -373,18 +373,20 @@ def ts_project(
     if transpile_target_name or declarations_target_name:
         tsc_target_name = "%s_tsc" % name
 
-        lib_srcs = []
+        # Always include tsc target since even if it doesn't output js+dts, it may
+        # still be needed for JSON files to be included in the sources. Downstream
+        # dependencies of the js_library won't inadvertently cause this target to be
+        # typechecked when both transpiler targets for js+dts are present because
+        # the js_library doesn't include the ".typecheck" file which is only in the
+        # "typecheck" output group.
+        lib_srcs = [tsc_target_name]
 
         # Include the transpiler targets for both js+dts if they exist.
         if transpile_target_name:
             lib_srcs.append(transpile_target_name)
         if declarations_target_name:
             lib_srcs.append(declarations_target_name)
 
-        # If tsc outputs anything (js or dts) then it should be included in the srcs.
-        if not (transpile_target_name and declarations_target_name):
-            lib_srcs.append(tsc_target_name)
-
         # Include direct & transitive deps in addition to transpiled sources so
         # that this js_library can be a valid dep for downstream ts_project or other rules_js derivative rules.
         js_library(
@@ -442,7 +444,7 @@ def ts_project(
     # Disable typescript version detection if tsc is not the default:
     # - it would be wrong anyways
     # - it is only used to warn if worker support is configured incorrectly.
-    if tsc != _tsc:
+    if tsc != _tsc or tsc_worker != _tsc_worker:
         is_typescript_5_or_greater = None
     else:
         is_typescript_5_or_greater = select({

ts/private/ts_project.bzl

@@ -84,7 +84,6 @@ def _ts_project_impl(ctx):
 
     typecheck_outs = []
 
-    arguments = ctx.actions.args()
     execution_requirements = {}
     executable = ctx.executable.tsc
 
@@ -114,35 +113,35 @@ See https://github.com/aspect-build/rules_ts/issues/361 for more details.
 """)
 
     if supports_workers:
-        # Set to use a multiline param-file for worker mode
-        arguments.use_param_file("@%s", use_always = True)
-        arguments.set_param_file_format("multiline")
         execution_requirements["supports-workers"] = "1"
         execution_requirements["worker-key-mnemonic"] = "TsProject"
         executable = ctx.executable.tsc_worker
 
+    common_args = []
+
     # Add all arguments from options first to allow users override them via args.
-    arguments.add_all(options.args)
+    common_args.extend(options.args)
 
     # Add user specified arguments *before* rule supplied arguments
-    arguments.add_all(ctx.attr.args)
+    common_args.extend(ctx.attr.args)
 
     outdir = _lib.join(
         ctx.label.workspace_root,
         _lib.join(ctx.label.package, ctx.attr.out_dir) if ctx.attr.out_dir else ctx.label.package,
     )
     tsconfig_path = to_output_relative_path(tsconfig)
-    arguments.add_all([
+    common_args.extend([
         "--project",
         tsconfig_path,
         "--outDir",
         outdir,
         "--rootDir",
         _lib.calculate_root_dir(ctx),
     ])
+
     if len(typings_outs) > 0:
         declaration_dir = _lib.join(ctx.label.workspace_root, ctx.label.package, typings_out_dir)
-        arguments.add("--declarationDir", declaration_dir)
+        common_args.extend(["--declarationDir", declaration_dir])
 
     inputs = srcs_inputs + tsconfig_inputs
 
@@ -166,7 +165,7 @@ See https://github.com/aspect-build/rules_ts/issues/361 for more details.
 
     outputs = js_outs + map_outs + typings_outs + typing_maps_outs
     if ctx.outputs.buildinfo_out:
-        arguments.add("--tsBuildInfoFile", to_output_relative_path(ctx.outputs.buildinfo_out))
+        common_args.extend(["--tsBuildInfoFile", to_output_relative_path(ctx.outputs.buildinfo_out)])
         outputs.append(ctx.outputs.buildinfo_out)
 
     output_sources = js_outs + map_outs + assets_outs
@@ -233,15 +232,30 @@ See https://github.com/aspect-build/rules_ts/issues/361 for more details.
         typecheck_outs.append(typecheck_output)
 
         typecheck_arguments = ctx.actions.args()
-        if supports_workers:
-            typecheck_arguments.add("--bazelValidationFile", typecheck_output.short_path)
+        typecheck_arguments.add_all(common_args)
 
         typecheck_arguments.add("--noEmit")
 
+        env = {
+            "BAZEL_BINDIR": ctx.bin_dir.path,
+        }
+
+        # In non-worker mode, we create the marker file by capturing stdout of the action
+        # via the JS_BINARY__STDOUT_OUTPUT_FILE environment variable, but in worker mode, the
+        # persistent worker protocol communicates with the worker process via stdin/stdout, so
+        # the output cannot just be captured. Instead, we tell the worker where to write the
+        # marker file by passing the path via the --bazelValidationFile flag.
+        if supports_workers:
+            typecheck_arguments.use_param_file("@%s", use_always = True)
+            typecheck_arguments.set_param_file_format("multiline")
+            typecheck_arguments.add("--bazelValidationFile", typecheck_output.short_path)
+        else:
+            env["JS_BINARY__STDOUT_OUTPUT_FILE"] = typecheck_output.path
+
         ctx.actions.run(
             executable = executable,
             inputs = transitive_inputs_depset,
-            arguments = [arguments, typecheck_arguments],
+            arguments = [typecheck_arguments],
             outputs = [typecheck_output],
             mnemonic = "TsProjectCheck",
             execution_requirements = execution_requirements,
@@ -250,16 +264,14 @@ See https://github.com/aspect-build/rules_ts/issues/361 for more details.
                 ctx.label,
                 tsconfig_path,
             ),
-            env = {
-                "BAZEL_BINDIR": ctx.bin_dir.path,
-                "JS_BINARY__STDOUT_OUTPUT_FILE": typecheck_output.path,
-            },
+            env = env,
         )
     else:
         typecheck_outs.extend(output_types)
 
     if use_tsc_for_js or use_tsc_for_dts:
         tsc_emit_arguments = ctx.actions.args()
+        tsc_emit_arguments.add_all(common_args)
 
         # Type-checking is done async as a separate action and can be skipped.
         if ctx.attr.isolated_typecheck:
@@ -273,18 +285,20 @@ See https://github.com/aspect-build/rules_ts/issues/361 for more details.
 
         elif not use_tsc_for_dts:
             # Not emitting declarations
-            # TODO: why doesn't this work with workers?
-            if not supports_workers:
-                tsc_emit_arguments.add("--declaration", "false")
+            tsc_emit_arguments.add("--declaration", "false")
 
         verb = "Transpiling" if ctx.attr.isolated_typecheck else "Transpiling & type-checking"
 
         inputs_depset = inputs if ctx.attr.isolated_typecheck else transitive_inputs_depset
 
+        if supports_workers:
+            tsc_emit_arguments.use_param_file("@%s", use_always = True)
+            tsc_emit_arguments.set_param_file_format("multiline")
+
         ctx.actions.run(
             executable = executable,
             inputs = inputs_depset,
-            arguments = [arguments, tsc_emit_arguments],
+            arguments = [tsc_emit_arguments],
             outputs = outputs,
             mnemonic = "TsProjectEmit" if ctx.attr.isolated_typecheck else "TsProject",
             execution_requirements = execution_requirements,

ts/private/ts_project_worker.js

@@ -310,7 +310,7 @@ function createFilesystemTree(root, inputs) {
         if (typeof node == "object" && node[Type] == TYPE.SYMLINK) {
            return parents.join(path.sep);
         }
-        
+
         return undefined;
     }
 
@@ -467,11 +467,6 @@ function createFilesystemTree(root, inputs) {
 
 
 /** Program and Caching */
-function isExternalLib(path) {
-    return path.includes('external') &&
-        path.includes('typescript@') &&
-        path.includes('node_modules/typescript/lib')
-}
 
 const libCache = new Map();
 
@@ -537,7 +532,7 @@ function createEmitAndLibCacheAndDiagnosticsProgram(
             return libCache.get(fileName);
         }
         const sf = getSourceFile(fileName);
-        if (sf && isExternalLib(fileName)) {
+        if (sf && path.relative(host.getDefaultLibLocation(), fileName).startsWith('..')) {
             debug(`createEmitAndLibCacheAndDiagnosticsProgram: putting default lib ${fileName} into cache.`)
             libCache.set(fileName, sf);
         }
@@ -553,7 +548,8 @@ function createProgram(args, inputs, output, exit) {
     const execroot = path.resolve(bin, '..', '..', '..'); // execroot
     const tsconfig = path.relative(execroot, path.resolve(bin, cmd.options.project)); // bazel-bin/<cfg>/bin/<pkg>/<options.project>
     const cfg = path.relative(execroot, bin) // /bazel-bin/<cfg>/bin
-    const executingfilepath = path.relative(execroot, require.resolve("typescript")); // /bazel-bin/<opt-cfg>/bin/node_modules/tsc/...
+    const executingFilePath = path.relative(execroot, require.resolve("typescript")); // /bazel-bin/<opt-cfg>/bin/node_modules/tsc/...
+    const executingDirectoryPath = path.dirname(executingFilePath);
 
     const filesystem = createFilesystemTree(execroot, inputs);
     const outputs = new Set();
@@ -565,7 +561,7 @@ function createProgram(args, inputs, output, exit) {
         write: write,
         writeOutputIsTTY: () => false,
         getCurrentDirectory: () => "/" + cfg,
-        getExecutingFilePath: () => "/" + executingfilepath,
+        getExecutingFilePath: () => "/" + executingFilePath,
         exit: exit,
         resolvePath: notImplemented("sys.resolvePath", true, 0),
         // handled by fstree.
@@ -605,7 +601,7 @@ function createProgram(args, inputs, output, exit) {
     debug(`execroot: ${execroot}`);
     debug(`bin: ${bin}`);
     debug(`cfg: ${cfg}`);
-    debug(`executingfilepath: ${executingfilepath}`);
+    debug(`executingFilePath: ${executingFilePath}`);
 
     let compilerOptions = readCompilerOptions();
 
@@ -757,8 +753,12 @@ function createProgram(args, inputs, output, exit) {
     function readFile(filePath, encoding) {
         filePath = path.resolve(sys.getCurrentDirectory(), filePath)
 
-        //external lib are transitive sources thus not listed in the inputs map reported by bazel.
-        if (!filesystem.fileExists(filePath) && !isExternalLib(filePath) && !outputs.has(filePath)) {
+        // external lib are transitive sources thus not listed in the inputs map reported by bazel.
+        if (
+            !filesystem.fileExists(filePath) &&
+            path.relative("/" + executingDirectoryPath, filePath).startsWith('..') &&
+            !outputs.has(filePath)
+        ) {
             output.write(`tsc tried to read file (${filePath}) that wasn't an input to it.` + "\n");
             throw new Error(`tsc tried to read file (${filePath}) that wasn't an input to it.`);
         }
@@ -880,9 +880,11 @@ async function emit(request) {
     debug(`# Beginning new work`);
     debug(`arguments: ${request.arguments.join(' ')}`)
 
-    const validationPath = request.arguments[request.arguments.indexOf('--bazelValidationFile') + 1]
-    fs.writeFileSync(path.resolve(process.cwd(), validationPath), '');
-
+    if (request.arguments.includes('--bazelValidationFile')) {
+        const [, validationPath] = request.arguments.splice(request.arguments.indexOf('--bazelValidationFile'), 2);
+        fs.writeFileSync(path.resolve(process.cwd(), validationPath), '');
+    }
+
     const inputs = Object.fromEntries(
         request.inputs.map(input => [
             input.path,
@@ -945,22 +947,22 @@ async function emit(request) {
     const diagnostics = ts.getPreEmitDiagnostics(program, undefined, cancellationToken).concat(result?.diagnostics);
     timingEnd('diagnostics');
 
-    const succeded = diagnostics.length === 0;
+    const succeeded = diagnostics.length === 0;
 
-    if (!succeded) {
+    if (!succeeded) {
         request.output.write(worker.formatDiagnostics(diagnostics));
         VERBOSE && worker.printFSTree()
     }
 
     if (ts.performance && ts.performance.isEnabled()) {
         ts.performance.forEachMeasure((name, duration) => request.output.write(`${name} time: ${duration}\n`));
     }
- 
+
     worker.previousInputs = inputs;
     worker.postRun();
 
     debug(`# Finished the work`);
-    return succeded ? 0 : 1;
+    return succeeded ? 0 : 1;
 }
 
 
@@ -997,7 +999,7 @@ if (require.main === module && worker_protocol.isPersistentWorker(process.argv))
     const args = fs.readFileSync(p).toString().trim().split('\n');
 
     if (args.includes('--bazelValidationFile')) {
-        const [, validationPath] = args.splice(args.indexOf('--bazelValidationFile'), 2);    
+        const [, validationPath] = args.splice(args.indexOf('--bazelValidationFile'), 2);
         fs.writeFileSync(path.resolve(process.cwd(), validationPath), '');
     }