rules_ts icon indicating copy to clipboard operation
rules_ts copied to clipboard

"The inferred type of X cannot be named without a reference to Y" (TS2742)

Open tinganho opened this issue 2 years ago • 16 comments

npm_link_package labels in data and deps attributes in ts_project is causing typescript to issue portability errors.

Here is a reproduction and more info in the README: https://github.com/tinganho/transitive-deps-bug

tinganho avatar Sep 28 '22 09:09 tinganho

This might be an issue with symlinks and not only rules_ts? See https://github.com/microsoft/TypeScript/issues/29808#issuecomment-581795107

jbedard avatar Oct 08 '22 03:10 jbedard

Yup. Looks like this is a Typescript + symlinked node_modules structure issue and not specific to rules_js or Bazel.

Issue in TypeScript repo is very active: https://github.com/microsoft/TypeScript/issues/47663

@mrmeku has a repro outside of Bazel with pnpm, which also uses a symlinked node_modules structure like rules_js: https://github.com/mrmeku/portable_types_repro (from https://github.com/microsoft/TypeScript/issues/47663#issuecomment-1267631748)

gregmagolan avatar Oct 18 '22 23:10 gregmagolan

One of the work-arounds in https://github.com/microsoft/TypeScript/issues/47663 is to import type {} from "transitive": https://github.com/microsoft/TypeScript/issues/47663#issuecomment-1270716220.

I was able to get this reproduction to work by doing that for the transitive closure with this change:

$ git diff
diff --git a/packages/a/BUILD.bazel b/packages/a/BUILD.bazel
index 0744ca1..9be754c 100644
--- a/packages/a/BUILD.bazel
+++ b/packages/a/BUILD.bazel
@@ -5,5 +5,8 @@ package(default_visibility = ["//visibility:public"])
 ts_test(
     name = "a",
     srcs = ["foo.ts"],
-    deps = ["//packages/b:b"]
+    deps = [
+        "//packages/b:b",
+        "//packages/c:c",
+    ]
 )
\ No newline at end of file
diff --git a/packages/a/foo.ts b/packages/a/foo.ts
index faf0838..c227a19 100644
--- a/packages/a/foo.ts
+++ b/packages/a/foo.ts
@@ -1,4 +1,6 @@
 import { Foo } from '@myorg/b/bar';
+import type {} from '@myorg/b/bar';
+import type {} from '@myorg/c/qux';
 
 export interface Offline {
     pluginRegistry: Foo | undefined;
diff --git a/packages/b/bar.ts b/packages/b/bar.ts
index 44d3558..d8e7756 100644
--- a/packages/b/bar.ts
+++ b/packages/b/bar.ts
@@ -1,4 +1,5 @@
 import { Bar } from '@myorg/c/qux';
+import type {} from '@myorg/c/qux';
 
 export interface Foo {
     bar: Bar[];

gregmagolan avatar Oct 19 '22 00:10 gregmagolan

Until the underlying problem with Typescript and the symlinked node_modules structure is fixed, a work-around will be needed.

gregmagolan avatar Oct 19 '22 00:10 gregmagolan

Seems like indeed it is a TypeScript bug. Described more in here:

You get this error any time the declaration emitter can't synthesize a workable specifier for a module which it needs to name a type from. For example, if it appears that the only legal path is ../../other_module/foo via some file that's in <<outDir>>/whatever, then that's not likely to work because the disk layout of the produced artifacts don't really have implicit dependencies on what peer directories of the output directory have. The logic to synthesize these specifiers starts with the easy route of "Has this already been imported?", in which case re-use is easy and fine. Immediately past that lie many dragons and it's easy to get into a novel corner case where there is a speakable name to a module but TS just can't figure it out. Adding the import yourself is the easiest way to resolve the situation.

https://github.com/microsoft/TypeScript/issues/48212#issuecomment-1205888829

I think the issue boils down to: transitive deps being resolved to outside the project?

tinganho avatar Oct 19 '22 12:10 tinganho

@gregmagolan I had closer look at the the minimum reproducible project today. I noticed the symlink structure is not exactly described in PNPM:

Note, that under node_modules/.aspect_rules_js/@[email protected]/node_modules/@myorg the symlink c is not pointing to ../../@[email protected]/node_modules/@myorg/c as in PNPM docs:

image

The project is under sandbox/darwin-sandbox/3 but the symlink target is outside of the project subtree?

Note the c package is stored node_modules/.aspect_rules_js/@[email protected]/node_modules/@myorg/c, which the b package doesn't symlink to?

image

tinganho avatar Oct 27 '22 12:10 tinganho

I'm thinking if the symlink target is being kept in the project's subtree. It might fix the issue?

tinganho avatar Oct 27 '22 13:10 tinganho

That is an issue with Bazel itself wich controls how symlinks are layed out on disk. Historically the sandbox and runfiles trees created symlinks as absolute paths pointing out of the trees instead of relative within the trees.

However, this problem is now resolved in Bazel 6 with some help from the community. Bazel 6 is currently at 6.0.0rc1. If you use the latest rules_js with Bazel 6.0.0rc1 and set the --experimental_allow_unresolved_symlinks flag in your .bazelrc then the symlinks should be layed out on disk correctly by Bazel.

gregmagolan avatar Oct 27 '22 18:10 gregmagolan

The bug still appears, although now inside the project subtree

The inferred type of 'Offline' cannot be named without a reference to '.aspect_rules_js/@[email protected]/node_modules/@myorg/c/qux'. This is likely not portable. A type annotation is necessary.

tinganho avatar Oct 28 '22 07:10 tinganho

@gregmagolan I'm still having this problem. And --allow_experimental_unresolves_symlinks is unrecognized by Bazel.

flolu avatar Nov 28 '22 18:11 flolu

@gregmagolan could you provide more details for --allow_experimental_unresolves_symlinks? Adding it to .bazelrc doesn't fix the issue for me.

Is there any plan to get this fixed?

flolu avatar Feb 20 '23 17:02 flolu

@flolu Bazel 6 the feature is on by default so it is only with Bazel 5 that --experimental_allow_unresolved_symlinks needs to be set explicitly. What version of Bazel are you using?

gregmagolan avatar Feb 20 '23 18:02 gregmagolan

@gregmagolan I'm using Bazel 6:

Bazelisk version: v1.13.2
Build label: 6.0.0
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Mon Dec 19 15:52:35 2022 (1671465155)
Build timestamp: 1671465155
Build timestamp as int: 1671465155

You can reproduce the issue like this:

git clone https://github.com/flolu/aspect-typescript
git checkout inferred-type-error
pnpm i
bazelisk test ...

It throws:

app/app.ts(4,7): error TS2742: The inferred type of 'app' cannot be named without a reference to '.aspect_rules_js/@[email protected]/node_modules/@types/express-serve-static-core'. This is likely not portable. A type annotation is necessary.

flolu avatar Feb 20 '23 18:02 flolu

The underlying with Typescript and the symlinked node_modules structure is still open https://github.com/microsoft/TypeScript/issues/47663, although it looks like the discussion is still active and someone has proposed a set of patches. The issue has also been reported on the pnpm repo 4 days ago https://github.com/pnpm/pnpm/issues/6089 since pnpm also uses a symlinked node_modules structure.

For now a work-around is needed for this issue both with rules_js and pnpm. https://github.com/microsoft/TypeScript/issues/47663#issuecomment-1270716220 is one from the typescript issue and the reporter of the pnpm issue suggested one as well https://github.com/pnpm/pnpm/issues/6089#issuecomment-1433207437.

gregmagolan avatar Feb 20 '23 18:02 gregmagolan

@gregmagolan allright, thanks!

flolu avatar Feb 20 '23 18:02 flolu

Same issue here

Sayed-Helmy avatar Apr 01 '24 06:04 Sayed-Helmy