ts_project_worker: default-initialize tsc outDir

[malt3]

Some time ago, the tsc_project macro changed behavior. Before, out_dir would always be set, whereas now it may be None. If out_dir is unset, this expression will result in an incorrect output directory being used:

host.optionsToExtend.outDir = `${host.optionsToExtend.outDir}/${SYNTHETIC_OUTDIR}`;

(Would result in 'undefined/__st_outdir__').

I understand that the persistent worker mode is mostly unused in upstream rules_ts (since it is also disabled by default on TS 5), so it is not surprising to see small breakages.
I still hope we can fix the worker here, and don't have to carry downstream patches.

---

Changes are visible to end-users: no

Test plan

• Manual testing; please provide instructions so we can reproduce:

Use a custom ts_worker and ensure you have a ts_project with out_dir unset (or set to ".").

[aspect-workflows]

Test

2 test targets passed

Targets

//ts/test/ts_project_worker:fstree_test [k8-fastbuild] 249ms
//ts/test/ts_project_worker:oom_test [k8-fastbuild]    99ms

Total test execution time was 348ms. 171 tests (98.8%) were fully cached saving 8s.

---

Buildifier      Format

[jbedard]

Why use rootDir? That seems wrong imo... 🤔

[jbedard]

Or is this rootDir different then compilerOptions.rootDir?

[malt3]

The cwd of the worker is the Bazel bindir.
rootDir is set to the package directory we are currently in by default, so this works correctly if I leave out_dir and root_dir unset in the macro.
I'm also open to using some other source of info to get the current package path.

[jbedard]

Can we just drop the use of rootDir here? Normally outDir || '.' would be create if we're talking about the tsconfig outDir...

[malt3]

No, since that would create outputs directly in Bazel's BINDIR. Actions are only allowed to write below their own package dir.
I tested the behavior of typescript closely, and . would only work for a BUILD file in the topmost directory.
I can also add a test for this or demonstrate the behavior with an example if needed.
As stated above, maybe we can use some other method to pass the current package as a default, without relying on rootDir?
Note that the worker doesn't cd into the rootDir.

[jbedard]

So this should probably be outDir || BUILD-dir? I still think rootDir is wrong here, correct me if this is different then the tsconfig rootDir though...

If you can add some tests that would be ideal 👍

[malt3]

Yes, outDir || BUILD-dir would be ideal. It's just that the long-lived worker process can only infer the BUILD directory from the work request. Right now this doesn't contain the BUILD directory. I just chose rootDir since it happens to match BUILD-dir by default.

I can add a test, but more importantly: should we add the current package (aka BUILD-dir) to the work request explicitly instead of misusing rootDir?

[jbedard]

Note that I don't know this worker code well and I'm going off assumptions here. So maybe this SYNTHETIC_OUTDIR logic is different then what I'm assuming.

Can we simply avoid this SYNTHETIC_OUTDIR thing when outDir is not configured?

[thesayyn]

rules_ts/ts/private/ts_project_worker.js

Lines 547 to 552 in 0d1dadb

	const bin = process.cwd(); // <execroot>/bazel-bin/<cfg>/bin
	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 executingDirectoryPath = path.dirname(executingFilePath);

one of these should give you the package relative path.