Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ts_project breaks on relative imports to .d.ts source files #3120

Closed
lencioni opened this issue Dec 6, 2021 · 1 comment
Closed

ts_project breaks on relative imports to .d.ts source files #3120

lencioni opened this issue Dec 6, 2021 · 1 comment

Comments

@lencioni
Copy link
Contributor

lencioni commented Dec 6, 2021

🐞 bug report

Affected Rule

The issue is caused by the rule: ts_project

Is this a regression?

No, I don't think so

Description

I am working in a monorepo that uses ts_project roughly as described by #2298

We have run into a build failure when using a relative import to a .d.ts source file.

🔬 Minimal Reproduction

We have a structure like the following:

frontend/a
├── BUILD.bazel
├── index.ts
└── tsconfig.json
frontend/b
├── BUILD.bazel
├── foo.d.ts
├── index.ts
└── tsconfig.json

Here are the contents of those files:

// frontend/a/index.ts
import { PageData } from ':b';

const a: PageData = {
  components: [],
};

export const imageComponent = (a?.components || []).find(
  (component) => component?.componentType === 'IMAGE',
);
// frontend/b/index.ts
import type { ContentPage } from './foo';

export type PageData = ContentPage;
// frontend/b/foo.d.ts
export interface Components {
  readonly componentAttributes: string | null;
  readonly componentType: string | null;
  readonly id: integer | null;
  readonly uuid: string | null;
}

export interface ContentPage {
  readonly components: (Components | null)[] | null;
}

🔥 Exception or Error

In this example, frontend/b is able to build ts_project successfully, but when building frontend/a, which depends on frontend/b for some of its types, we get an error like the following:


frontend/a/index.ts:9:4 - error TS7006: Parameter 'component' implicitly has an 'any' type.

9   (component) => component?.componentType === 'IMAGE',
     ~~~~~~~~~

This error does not happen when running TypeScript outside of bazel.

Looking at the structure of the sandbox, I believe this happens because of the relative import from frontend/b/index.ts to frontend/b/foo.d.ts-- the generated frontend/b/index.d.ts file ends up in the output tree but the frontend/b/foo.d.ts file stays in the source tree. This means that when frontend/b is consumed as a dependency, the relative import from frontend/b/index.ts to ./foo fails, which causes the types to be cast to any, which triggers the error above.

Possibly relevant:

I looked into using copy_file or copy_to_bin in my macro that wraps ts_project for any .d.ts source files, but I didn't have any luck getting it to work the way I wanted and I struggled to find any examples of it being used in this way.

One workaround I found is to add "../../" to compilerOptions.rootDirs, which allows TypeScript to resolve relative imports from the output tree to the source tree. However, I don't want that configuration when not running in Bazel since I think that would cause TypeScript when running outside of Bazel (e.g. in VSCode) to resolve files outside of the repo (and possibly from multiple other repos). One possibility would be to generate tsconfigs with this extra configuration in our macro, but I'd prefer to avoid that complexity if possible (plus, it seems that this may not work with the tsconfig option today since .d.ts files are filtered out in the generated tsconfig file https://github.com/bazelbuild/rules_nodejs/blob/d30c366127c208d14d08adb74f4a847f87558c92/packages/typescript/internal/ts_project.bzl#L672).

At this point I see a few possible resolutions:

  1. Should ts_project just handle .d.ts source files automatically? (e.g. build in copy_to_bin to the rule for .d.ts source files) I think this would be the most ergonomic outcome.
  2. If we don't want ts_project to handle this automatically, it would be really helpful to add an example to the docs of how to compose ts_project with copy_to_bin or something similar (js_library maybe?) to make this scenario work.
  3. If my understanding of copy_to_bin doesn't make sense (which is likely) then it probably makes sense to update the ts_project docs to include "../../" to the compilerOptions.rootDirs configuration along with an example of how to use this safely in a generated tsconfig.json file.

🌍 Your Environment

Operating System:

MacOS 12.0.1

Output of bazel version:

Bazelisk version: v1.10.1
Build target: bazel-out/darwin-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Thu Jan 01 00:00:00 1970 (0)
Build timestamp: Thu Jan 01 00:00:00 1970 (0)
Build timestamp as int: 0

Rules_nodejs version:

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

4.4.6

Anything else relevant?

@lencioni
Copy link
Contributor Author

lencioni commented Dec 6, 2021

We've dug into this more and I believe that my original assumption about compilerOptions.rootDirs having ../../ in it would allow TypeScript to escape the repo was incorrect. The TypeScript docs mention this:

All relative paths found in the configuration file will be resolved relative to the configuration file they originated in.

https://www.typescriptlang.org/tsconfig#extends

I've tested adding "../.." and removing "." from our rootDirs config and checked it with --traceResolution and I believe this is the correct fix for this issue.

I'll update my example in #2298 to match.

@lencioni lencioni closed this as completed Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant