typescript: devmode manifest and externs not declared as action outputs

[devversion]

🐞 bug report

Affected Rule

ts_library

Is this a regression?

No

Description

Currently the ts_library rule writes a manifest file in devmode. In prodmode, it writes
closure extern files.

All of these non-compilation files are not declared as outputs of the actual
tsc-wrapped action that intends to write these files. This means that those files
are written from action that does not declare these outputs.

This breaks hermeticity and causes exceptions as Bazel disallows writing to output files
of other actions. This happens because we currently have workaround logic to always write
empty manifest and extern files as standalone actions using ctx.actions.write.

https://github.com/bazelbuild/rules_nodejs/blob/388f53f9e6640ab8ff2ef013a447a4e0dce63ee0/packages/typescript/src/internal/build_defs.bzl#L102-L105

Note: we always need to populate these files because those are declared as action outputs. And sometimes they are not written by tsc-wrapped if the bazelOpts.tsickle flag is set to false.

Once these files are written through ctx.actions.write, the actual compilation action below tries to overwrite these files sometimes (if bazelOpts.tsickle is actually enabled). This causes an error as the previous action was already performed and Bazel marked the output files as readonly to ensure hermeticity.

Resulting in an error like:

ERROR: /material2/src/cdk/BUILD.bazel:6:1: Compiling TypeScript (devmode) //src/cdk:cdk failed (Exit 1)
Compilation failed Error: EPERM: operation not permitted, open 'bazel-out/x64_windows-fastbuild/bin/src/cdk/cdk.es5.MF'
    at Object.openSync (fs.js:443:3)
    at Object.writeFileSync (fs.js:1194:35)
    at perfTrace.wrap (C:\bazel_output_root\cqvt7iss\external\npm\node_modules\@bazel\typescript\internal\tsc_wrapped\tsc_wrapped.js:406:20)

Solutions

There are basically two options:

1. Introduce logic similar to writeEmptyCodegenFiles where we just move the ctx.actions.write workaround into tsc-wrapped. That way, we only have one action that writes those files.

2. We actually remove the bazelOpts.tsickle flag and move it to a static Bazel rule attribute. That way we can dynamically declare the externs/manifest outputs if tsickle is actually enabled. Meaning that we do not need the ctx.actions.write workaround and the issue is solved.

The second option is actually the most clean and reasonable solution but I doubt that we can do that without breaking too much things in Google3. I'd imagine a lot of apps depend on the tsickle option in the bazelOpts.

[alexeagle]

the externs problem was reported in #757

[github-actions]

This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

[github-actions]

This issue was automatically closed because it went two weeks without a reply since it was labeled "Can Close?"