bazelbuild / rules_typescript

MOVED to https://github.com/bazelbuild/rules_nodejs/tree/3.x/third_party/github.com/bazelbuild/rules_typescript
https://github.com/bazelbuild/rules_nodejs
Apache License 2.0
275 stars 94 forks source link

fix(tsc_wrapped): do not leak generated module name from previous compilations #504

Closed devversion closed 4 years ago

devversion commented 4 years ago

Currently, the compiler host in @bazel/typescript assigns a moduleName to source files when the module format is AMD or UMD.

It does that so that devmode compilations (which by default use the module kinds as mentioned above), have unique module names for all compilation source files. This is necessary so that the JS output can run inside browsers and NodeJS in concatenated bundles.

The update of the moduleName for source files is critical though, because source files are re-used/cached across compilations. For example, an initial compilation might be for AMD devmode where the module name is computed and set to the source file. In a followed compilation in devmode then, the module name is still set by accident and the compilation output could unexpectedly use the module name in a compilation where relative imports are expected.

Another scenario would be that the moduleName has changed in the second compilation through an update to the module_name rule attribute. The host will now throw an error because it thinks that there are conflicting module names for the source file. This results in the commonly known error looking as followed:

Compiling Angular templates (Ivy - devmode) <..> failed (Exit 1)
Error: ERROR: <..>/index.ts contains a module name declaration {old} which would be
overwritten with {new_name} by Bazel's TypeScript compiler.

We can fix this by not modifying the moduleName of source files directly. This means that cached source files from a file loader are not accidentally invalidated if the compilation module kind changes.

Ideally we'd create recursive clones for source files when returning them from the file loader, so that cached files are never invalidated accidentally. Unfortunately though, it seems that this is not achievable in a reasonable way as it increases complexity and slows-down the cache significantly (due to required recursive clone/proxy-ing & increased memory consumption).

Although the above would be ideal, it seems like cloning the full source file is not necessarily needed to maintain "re-usable" cached source files. We can assume that TypeScript will not internally modify/invalidate essential information for cached source files through a standard emit.