Import from parent directory causes: Cannot find module

[flolu]

🐞 bug report

Affected Rule

The issue is caused by the rule: ts_project

Is this a regression?

Yes, the previous version in which this bug was not present was: 4.6.1

Description

After upgrading to 5.0, imports from a parent directory (from a different ts_project), like below, broke.

import {num} from '../num'

🔬 Minimal Reproduction

https://github.com/flolu/cannot-find-module

🔥 Exception or Error

ERROR: /home/flo/Documents/cannot-find-module/app/subapp/BUILD.bazel:8:11: Compiling TypeScript project //app/subapp:lib

app/subapp/index.ts(1,19): error TS2307: Cannot find module '../num' or its corresponding type declarations.

[alexeagle]

https://github.com/bazelbuild/rules_nodejs/wiki/Debugging-problems-with-ts_project is my usual procedure for figuring these out.

Adding --traceResolution to the //app/subapp:lib and running bazel build //app/subapp:lib shows tsc says

======== Module name '../num' was successfully resolved to '/home/alexeagle/.cache/bazel/_bazel_alexeagle/524c8e2c39a7e4fe1e0ef9829421460a/sandbox/linux-sandbox/53/execroot/repro/bazel-out/k8-fastbuild/bin/app/num.d.ts'. ========

and the compilation succeeds.

However bazel build //... fails. This is confusing and subtle!

The reason is the nodejs_image target which transitively depends on the ts_project. The Breaking Changes section of the last rules_docker release https://github.com/bazelbuild/rules_docker/releases/tag/v0.23.0 indicates that the "Transitions" feature is now used so that transitive dependencies of a docker image get built for the target platform (linux x86). As a result, the dependencies are built under a different configuration, and this affects the paths.

The root cause is the workaround for an upstream TypeScript bug. The workaround is in your tsconfig.json file

In order for Typescript to resolve relative references to the bazel-out folder, it is recommended that the base tsconfig contains a rootDirs section that includes all possible locations

However the locations here are for configurations "fastbuild" and "dbg". With transitions, an additional configuration is created, so that list isn't exhaustive anymore.

As the error message from Bazel running tsc shows

ERROR: /home/alexeagle/repros/cannot-find-module/app/subapp/BUILD.bazel:8:11: Compiling TypeScript project //app/subapp:lib [tsc -p app/tsconfig.lib.json] failed: (Exit 2): tsc.sh failed: error executing command bazel-out/k8-opt-exec-2B5CBBC6-ST-625e526ca8a8/bin/external/npm/typescript/bin/tsc.sh --project app/tsconfig.lib.json --outDir bazel-out/k8-fastbuild-ST-4a519fd6d3e4/bin/app/subapp --rootDir app/subapp ... (remaining 3 arguments skipped)

the --outDir parameter passed to tsc is rooted at bazel-out/k8-fastbuild-ST-4a519fd6d3e4/bin so adding that path to the tsconfig rootDirs attribute fixes this compilation.

I think the main question here is whether the hash 4a519fd6d3e4 is sufficiently stable that this is the "rules_docker configuration" and can just be added to everyone's tsconfig. My understanding is that it's a simple hash of the target platform, and so it should be okay. We need to poke through Bazel source code to confirm.

---

What this really demonstrates is that our workaround for https://github.com/microsoft/TypeScript/issues/37378 is not scaling. We don't want to tell TypeScript an exhaustive list of all the places it needs to look for outputs of the deps (project references in TS lingo). We just told it where those outputs are in the --outDir option. So we really need TypeScript to give us a way to include the --outDir of the current compilation in the rootDirs attribute.

Maybe we can push harder to get some movement there, likely by finding someone in the Bazel community willing to make the PR to typescript, and someone at Microsoft to approve.

[flolu]

@alexeagle you are a legend! I would have never came to this conclusion. Although this fix is quite ugly, it works really well. Thanks for your help and your detailed answer!

[alexeagle]

Let's keep it open until we're sure the workaround is stable.

Based on https://github.com/bazelbuild/bazel/pull/14251/files I think @fmeum is our expert here - Fabian do you have an estimate of how stable these hashes are? If users put bazel-out/k8-fastbuild-ST-4a519fd6d3e4/bin into their TypeScript configuration file, would it work on future Bazel releases? Can we predict the value 4a519fd6d3e4 with some simple, stable computation on the rules_docker transition defined here: https://github.com/bazelbuild/rules_docker/pull/1963/files#diff-4f2ceedb3481ec74a75114778fa559beec50994ea7b74e6524e14299d5dbcfd0R807-R816 ?

[fmeum]

Those are just cherry-picks of commits by the real expert for this domain, @sdtwigg. But I will try to summarize what I know.

Currently, the hash is computed in FunctionTransitionUtil. It is essentially a truncated SHA256 hash of all build settings that have been changed by transitions at some point on the path from the top-level targets to the current target. Using Bazel's Fingerprint class, these hashes could be precomputed for any given set of settings. Doing this from Starlark would be difficult though due to the lack of a SHA256 function.

There are ongoing efforts to improve this scheme, e.g., make it less sensitive to configuration changes that are meaningless for the build outputs (see https://github.com/bazelbuild/bazel/issues/14023). The current status seems to be that these changes would be gated behind --experimental flags. Even if it weren't, if there is not much of a cost to add a couple new hashes to the config file per major release of Bazel, this could be a viable workaround.

However, there is one major restriction: These hashes can only be precomputed for the case where the rules_nodejs or rules_docker targets are not themselves subject to transitions coming from a higher-level target. I don't know its use cases well enough to decide whether that can happen, but if it were, the hashes would no longer be static.

Summarized: I wouldn't worry about Bazel releases breaking these hashes too much in the short- to mid-term, but the rising adoption of transitions by rulesets could become a more serious problem. I would personally suggest to keep trying to get a change landed into the TS compiler.

[abrmv]

Bumped into the similar issue when trying to upgrade from v.4 to v.5. I tried to migrate the monorepo setup with the bazel-out paths overrides in my tsconfig.json (done according to docs https://bazelbuild.github.io/rules_nodejs/TypeScript.html#ts_project)

"compilerOptions": {
    "rootDirs": [
        ".",
        "../../bazel-out/host/bin/path/to",
        "../../bazel-out/darwin-fastbuild/bin/path/to",
        "../../bazel-out/darwin_arm64-fastbuild/bin/path/to",
        // ...
    ]
}

I found that now bazel output folders include auto-generated hashes i.e. bazel-out/k8-fastbuild/bin/ is now bazel-out/k8-fastbuild-ST-6390b2712f3f/bin/ so this magic trick with rootDirs mapping doesn't work. Also my hash is different to what's been posted above by @alexeagle. Maybe there is a way to calculate this sha based on the setup? Guess it depends on the node version / rules_nodejs version / platform used..

We have a number of webpack and nextjs targets that rely on this rootDirs mapping so this issue is quite a blocker for me on upgraing from rules_nodejs to latest v.5 release,

[alexeagle]

@abrmv that hash should be stable, as @fmeum says it's

a truncated SHA256 hash of all build settings that have been changed by transitions at some point on the path from the top-level targets to the current target. so I think it's safe to add to your tsconfig for now.

We're making progress in the root cause issue https://github.com/microsoft/TypeScript/issues/37378 so I'm hopeful we'll get a release of TypeScript where this stops being necessary.

[jfirebaugh]

I can't get it to work even with the rootDirs workaround. Here's a simple test case: https://github.com/jfirebaugh/rules_nodejs_test/tree/simplified

[flolu]

@alexeagle Would using https://github.com/aspect-build/rules_ts be the solution to this problem?