Incorrect type import in generated d.ts in monorepo

[pyrocat101]

🐞 bug report

Affected Rule

The issue is caused by the rule: ts_project

Is this a regression?

No.

Description

This issue is similar to #2298.

I have been trying to set up Bazel with ts_project in a large monorepo. The monorepo contains many packages that were previously type checked using TypeScript project references:

// packages can depend on each other
<repo root>/package-a
<repo root>/package-b
<repo root>/package-c

Non-relative imports are resolved to be relative to repo root using TypeScript baseUrl setting. We don't allow relative path import so that there is only one canonical import specifier for a file:

// Resolves to `<repo root>/package-a/foo.ts`
import 'package-a/foo';

When using Bazel, I modified the tsconfig.json to use the following setting:

{
  "compilerOptions": {
      // As suggested by ts_project doc.
      // This seems to affect the relative import path in the emitted files.
      "rootDirs": [
          ".",
          "./bazel-out/darwin-fastbuild/bin",
          "./bazel-out/k8-fastbuild/bin",
          "./bazel-out/x64_windows-fastbuild/bin",
          "./bazel-out/darwin-dbg/bin",
          "./bazel-out/k8-dbg/bin",
          "./bazel-out/x64_windows-dbg/bin"
      ],
      "baseUrl": ".",
      "paths": {
          "*": [
              // find files in the same package.
              "./*",
              // find d.ts of other packages in the output folder
              "./bazel-out/darwin-fastbuild/bin/*",
              "./bazel-out/k8-fastbuild/bin/*",
              "./bazel-out/x64_windows-fastbuild/bin/*",
              "./bazel-out/darwin-dbg/bin/*",
              "./bazel-out/k8-dbg/bin/*",
              "./bazel-out/x64_windows-dbg/bin/*"
          ]
      }
  },
}

Now if we have two package foo and bar. If bar depends on a type exported by foo, we have seen the emitted bar/index.d.ts containing invalid import path like this.

// It should `../foo/foo`!
declare const _default: ((foo: import("../bazel-out/darwin-fastbuild/bin/foo/foo").Foo) => any)[];
export default _default;

🔬 Minimal Reproduction

See https://github.com/pyrocat101/ts-bazel-repro

Try to build target //bar:types and check bazel-bin/bar/index.d.ts.

🌍 Your Environment

Operating System:

  
macOS 10.15.7
  

Output of bazel version:

  
bazel 3.7.1
  

Rules_nodejs version:

(Please check that you have matching versions between WORKSPACE file and @bazel/* npm packages.)

  
2.3.1
  

Anything else relevant?

[pyrocat101]

Update: the workaround I ended up using is to move . to the bottom instead of the first item.

TypeScript's declaration file emitter calls tryGetModuleNameFromRootDirs to determine the emitted module import path based on rootDir settings. It finds the first match in rootDirs array instead of finding the longest match. So a target file bazel-out/darwin-fastbuild/bin/foo/foo.d.ts will match . instead of ./bazel-out/darwin-fastbuild/bin. Moving . to the bottom fixes the issue for us.

[alexeagle]

Thanks for digging into the root cause here to understand the problem, @pyrocat101 ! Can you suggest what change we could make in rules_nodejs that would have helped you get this right?
Maybe we could produce an error (sniff whether written .d.ts files contain that awkward import("../bazel-out ?) or maybe there's a place where we could document this better?

[pyrocat101]

@alexeagle

Maybe we could produce an error (sniff whether written .d.ts files contain that awkward import("../bazel-out ?)

I like this idea! It would certainly help me catch the misconfiguration sooner.

or maybe there's a place where we could document this better?

This is the issue only when the tsconfig.json is placed at the workspace root, where the rootDir entry that starts with bazel-out is a subdirectory of . -- I am not sure what is the best way to help developers catch this, maybe we can swap the order of rootDirs items in the note of ts_project:

"compilerOptions": {
    "rootDirs": [
        // If tsconfig.json is placed at the workspace root, the current folder
        // should go last so that declaration file emitter will correctly generate
        // the relative import path. See: https://github.com/bazelbuild/rules_nodejs/issues/2333
        ".",
        "../../bazel-out/darwin-fastbuild/bin/path/to",
        "../../bazel-out/k8-fastbuild/bin/path/to",
        "../../bazel-out/x64_windows-fastbuild/bin/path/to",
        "../../bazel-out/darwin-dbg/bin/path/to",
        "../../bazel-out/k8-dbg/bin/path/to",
        "../../bazel-out/x64_windows-dbg/bin/path/to",
    ]
}

That being said, have you considered providing d.ts files of other TypeScript targets in the execroot (in the same location as if they are the source files) for ts_project? This way, the tsc module resolver will find modules in the "right" place, without having to use the rootDirs hack.

[alexeagle]

want to send a PR to update that note?

It's an interesting idea to patch the .d.ts files into the execroot as if they were laid out in the source tree. In fact we do this already if there's a "LinkablePackageInfo" (e.g. js_library) that provides the typings, and they are imported with that package name. I think we'd probably point to the latter rather than make our linker more complex.

[github-actions]

This issue has been automatically marked as stale because it has not had any activity for 90 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?"