types in transitive dependencies causing errors after upgrade from 3.1: The inferred type of ... cannot be named without a reference to

[ravenscar]

TypeScript Version: 3.4.3

Search Terms:
transitive dependency inferred type cannot be named without a reference
Code

Sorry that this is a Dockerfile, since the problem relates to multiple modules being installed as dependencies to each other it's difficult to express in a single piece of code. There's only 16 lines of code total, near as minimal as I could make it.

https://github.com/ravenscar/ts29221

docker build -t ts29221 https://raw.githubusercontent.com/ravenscar/ts29221/master/Dockerfile

Expected behavior:
tsc can compile the code

Actual behavior:
tsc fails with the messge:

src/index.ts(3,14): error TS2742: The inferred type of 'thingyTuple' cannot be named without a reference to '../../b/node_modules/ts29221-a/dist'. This is likely not portable. A type annotation is necessary.

This used to work fine in TS < 3.2 and seems to have broken since then, looking at the breaking changes I can't see anything mentioned which would apply here.

At first we thought we were experiencing issues due to symlinks created by lerna bootstrap and wanted to create an extremely minimal reproduction based on #29221, however we discovered that symlinks aren't the problem and have produced a Dockerfile to show that:

docker build -t ts29221 https://raw.githubusercontent.com/ravenscar/ts29221/master/DockerfileNoSymlinks

We also have a Dockerfile showing this working in 3.1:

docker build -t ts29221 https://raw.githubusercontent.com/ravenscar/ts29221/master/Dockerfile31

What happens here is that module ts29221-a exports a type definition, module ts29221-b uses that type definition in the return type of a function, then module ts29221-c uses that function to assign a value to a const.

This is using the new build system, with refs in the tsconfig, and is bootstrapped by lerna.

What is happening here is that when tsc is compiling ts29221-c it imports the types from ts29221-b which in turn imports them from ts29221-a. It then sees that they types are at a relative position of ../b/node_modules/ts29221-a/dist compared to ts29221-c's package.json.

They are actually also at node_modules/ts29221-b/node_modules/ts29221-a/dist relative to ts29221-c's package.json.

This is for example where they may be if they were installed by a package manager that didn't flatten dependencies (e.g. npm2 or pnpm).

The only way around this is for c to somehow know that b uses a and to add a as a direct dependency of c. Then to import the used type definition from a before importing b, even though c doesn't directly use a (so will need a tslint:ignore too).

Or do something even worse such as put this workaround at the top of c/src/index.ts:

import * as _ from '../node_modules/ts29221-b/node_modules/ts29221-a';

I think it is unreasonable for modules to need to know the dependencies of any of their submodules when it comes to types, although that is what we are currently doing in dozens of places.

I understand that the module resolution shows that it will only go up for node_modules but it really seems like something went wrong here as this pretty much makes TS broken for monorepos that use symlinks, if this is intentional then it's hard to imagine how the new build/watch features (which are awesome BTW) are supposed to work when the ts module dependencies are multiple levels deep.

Related Issues:
#29221
#29808
#26985

[ravenscar]

FYI I have also create a fork of the lerna example from @RyanCavanaugh but with the code for the above modules: https://github.com/ravenscar/learn-a

It fails with the same error:

The inferred type of 'thingyTuple' cannot be named without a reference to '../../pkg2/node_modules/@ryancavanaugh/pkg1/lib'. This is likely not portable. A type annotation is necessary.

[arthur-chan-leapyear]

Having similar issues on my team when we're writing any new library. If I use some package, lets say, react-select, and react-select has type annotations from @types/react, then I will have to copy and paste any type signature that I use from react-select.

In this specific case, react-select lets you define some styles like such:

export const SelectStyles = {
control: (base: React.CSSProperties) => ({ ...base, minHeight: '32px', padding: 0, margin: 0 }),
  dropdownIndicator: (base: React.CSSProperties) => ({ ...base, padding: 4 }),
  clearIndicator: (base: React.CSSProperties) => ({
    ...base,
    padding: 4
  }),
  ...
}

In the latest Typescript, I need to also need to explicit declare the type of SelectStyles:

type StyleFunctions = {
  control: CSSPropertiesFn
  dropdownIndicator: CSSPropertiesFn
  clearIndicator: CSSPropertiesFn
  ...
}

export const SelectStyles: StyleFunctions = ...

This is a bit onerous, and feels like it's something the compiler should be able to generate in the worst case. Making @react/types a dependency of the package did not help, and I checked that the affected packages are using the same version of @react/types.

This is just one particular example. There have been others since upgrading past 3.1.

[Roaders]

We're getting many examples of this on multiple projects that we're updating to angular 8 (ans as a result TS 3.4)

[Roaders]

Although... Having said that I only get this problem when compiling using angular. It does not happen when I just use tsc

[leemhenson]

I see this in a monorepo that uses pnpm to link the packages together. Under Mac OS the project reports a few of these sorts of errors in files A and B, but if I build via a Ubuntu image in Docker then I see the error reported in files C and D. I can work around the issue by adding explicit types at each location mentioned in the errors.

[arthur-chan-leapyear]

We're currently intending to get around the error (in our HOCs) by using React Hooks instead of HOCs. But this doesn't help the folks who've got large codebases that need HOCs, or frameworks that are still using that paradigm.

[pauldraper]

This error started happening in 7a71887 by @weswigham.

It errors if "node_modules" appears in the generated import, as that will probably not work when the .d.ts files are distributed. This is controled by NodeBuilderFlags.AllowNodeModulesRelativePaths.

Another repo case: https://github.com/pauldraper/ts-infer-import

I ran into this with bazel-contrib/rules_nodejs#1013 .

The Bazel build tool doesn't give control over the file layout (makes dependency and heremiticity guarantees easier); so that project uses a path mapping to the location of node_modules. "*": "../some-dir/node_modules/*".

So the import from example-lib has types that produce imports from ../some-dir/node_modules/example-lib.

(Ideally, TypeScript would choose a non-relative path mapping here.)

Until TS 3.2, this was not a problem since Bazel users typically do not distribute their declarations via npm.

[weswigham]

Ideally, TypeScript would choose a non-relative path mapping here.

In such a scenario, a non-relative mapping does not yet exist as far as the compiler is aware. If A has B in it's modules folder, and B has C in it's modules folder, and generating the types for A ends up relying on types from C, there's no way to write a non-relative reference to that (since it's not in A's modules folder). Now, changing your code so it can be referenced in a non-relative way is usually pretty trivial, like adding a direct dependency between A and C, or by not needing to reference C by manually annotating with a type that you can name.

[pauldraper]

Correct. A non-relative mapping is not always possible.

In the repro case it was, but yes not generally.

The code first tries for a node_modules-based non-relative path, and then falls back to non-node_modules relative path, preferring relative.

In my case, Bazel was effectively trying to relocate node_modules via a * path mapping. Had it actually been real node_modules, it would have generated a non-relative path and succeeded. But it wasn't, so it didn't.

A "solution" for this case would be to try both node_modules non-relative and non-node_modules non-relative paths first. I say "solution" because IDK if it is the best approach. I think Bazel just needs to get more aggressive.

Could resolveModuleNames and resolveTypeReferenceDirectives hooks override the behavior of getSpecifierForModuleSymbol?

[ravenscar]

@weswigham

I get there is a workaround in your example for A to directly import C. We have about 100 modules in our monorepo and this problem now comes up regularly and we have an increasing number of import * as ignoreThisTypescriptBug from 'C' in a bunch of our "A" modules.

An practical example for us would be we have a module which works with the database C1, and one which creates types from a graphql schema C2. These are both pretty complicated and I generally don't want to have and of my A1 ...A20 modules deal with it, or even know they exist. I have a B module which has some complex typescript typings to convert between these C1 and C2 and create a lovely simple facade for all of the A* modules to use.

Unfortunately because of this bug all the A* modules now have a bunch of direct imports and dependencies littered through them due to this bug.

Now I really really really don't think it is at all reasonable for A to need to know about C at all. I don't know if the TS devs are considering this an actual bug and intend to fix it, or just a weird feature of the compiler which has a workaround for people using monorepos.

The reason why many people are not experiencing this is because they are using aftermarket typings from the @types package.

Imagine if there were first party typings for all the database modules, then imagine that knex (quite legitimately) used these types in its own typings e.g. for the db connection setup. Is it reasonable to tell people that despite using knex to isolate themselves from low level database modules they still need to import them all over the place?

[ravenscar]

@weswigham

Another point I thought of was that what if B used version 1.0.0 of C but A was using version 2.0.0 of C already, with incompatible types? This is fine in the normal node/npm world but I think it would cause issues here.

I still think there is something fundamentally wrong with how the paths are resolve. I think it should be absolutely fine for A to use B which uses C where C is in the node_modules of B and not A. If this isn't the case then it seems to me that the typescript compiler isn't compliant with the NPM structure.

[weswigham]

This is fine in the normal node/npm world but I think it would cause issues here.

It can and does. In fact, you'd find out as much once you started writing the imports by hand once you discovered that the types mismatch (if they do).

I have a B module which has some complex typescript typings to convert between these C1 and C2 and create a lovely simple facade for all of the A* modules to use.

Have you considered reexporting C1's support types in C2? That can lead to a cleaner setup.

Now I really really really don't think it is at all reasonable for A to need to know about C at all. I don't know if the TS devs are considering this an actual bug and intend to fix it, or just a weird feature of the compiler which has a workaround for people using monorepos.

It's very much not a bug. Your code has an inferred type at some location that is defined in some other package that it does not have direct access to. The compiler needs to serialize a reference to that type. This error is here to get the programmer to resolve that issue, since there's no way for the compiler to do it automatically in a safe way.

I think it should be absolutely fine for A to use B which uses C where C is in the node_modules of B and not A.

A -> B -> C is fine. The issue is that the (would be) generated types imply A -> C. That's no good, and is why the error occurs.

[mmkal]

@weswigham I've come across this a few times, and I agree with this explanation that it's not a bug. But I do think typescript could do something more helpful than throwing an error when it crops up.

A dead-simple solution I can think of is, instead of throwing an error, declare the un-nameable transitive dependency type as unknown. There could even be a comment saying where it would need to find the reference to be typed properly, if it's possible to do that in declaration files.

I've usually come up against this when I'm trying to export a utility function within an internal module of a library, but one that isn't intended to re-exported by the library's main module. Since the library defines its own domain, the main module typically has self-sufficient types without any dependencies. If the d.ts file for the internal module has a few unknowns in it - I don't care at all. I'm never using the declarations for that particular export, only the source directly.

Another solution would be to allow emitDeclaration to take a whitelist/blacklist to avoid even trying to export declarations for certain modules, but that seems much more complex, and more overhead to maintain.

I can't speak for others, but I think falling back to unknown would solve all the cases I've seen this in without causing other issues. And logically it seems like a sensible action - given what you (rightly) said about getting the full dependency type:

The compiler needs to serialize a reference to that type... there's no way for the compiler to do it automatically in a safe way.

[andrewbranch]

This seems to be “fixed” by #33567 (cc @sheetalkamat). I’ve confirmed the repro provided by @ravenscar now compiles without errors (the inferred module specifier from C to A is now ../../a/dist instead of ../../b/node_modules/ts29221-a/dist).

I’m not sure if this is actually desirable in a Lerna setup, though—my guess is that you shouldn’t actually want TypeScript resolving cross-package paths as relative. And if that’s the case, it’s clear that what @weswigham is saying is right—there is literally no safe path the compiler can use, so you absolutely should get an error, which should prompt you to re-export the transitive dependencies’ types through the direct dependency, or else declare an explicit dependency upon the transitive dependencies.

So I’m really not sure how to call this. It seems like the 3.6 behavior is “working as intended” to protect you from shipping packages with bad paths in their typings, although we’re learning in this issue that it can be painful, and that protection isn’t necessary for folks who aren’t going to publish their individual Lerna packages separately (I guess this is a use case for most of you in this thread?). Now, we’ve eliminated that pain (perhaps inadvertently—it looks like #33567 was considering npm link dev scenarios, not symlinked monorepos) by serializing a path that we don’t error on, but might still be unsafe. So I’m not sure whether the 3.7 behavior is a bugfix or a regression.

[weswigham]

Regression, imo (Cross-project-ref relative specifiers probably? shouldn't be generated for modules? Or maybe just not relative paths that leave the project root? I'm unsure.), but that's probably obvious from my explaination above.

[andrewbranch]

Cross-project-ref relative specifiers probably? shouldn't be generated for modules? Or maybe just not relative paths that leave the project root?

But both of these things are common in project references that aren’t symlinked into each others’ node_modules. It feels like it’s only because the node_modules symlink exists that we should be clued into the fact that we shouldn’t escape the root.

[andrewbranch]

So this was a weird one, but I’m going to close this as “fixed” by #33567. For anyone who simply has a symlinked monorepo where they never split up and publish the individual packages, this is a quality of life improvement. If, on the other hand, someone runs into a problem with non-portable paths that is now uncaught due to #33567, we’ll consider more sophisticated logic for catching that problem as a feature request, and at that point we’ll try to implement it without messing up the “my monorepo works fine, don’t make my life harder by giving me errors that aren’t relevant to me” scenario.

[flyskywhy]

I solved it by move project position to a clean folder which has no parent node_modules/. For example, if

npm install @textile/js-http-client
cd node_modules/@textile/js-http-client/
npm install
./node_modules/.bin/tsc

will get error:

src/modules/account.ts:53:3 - error TS2742: The inferred type of 'sync' cannot be named without a reference to '@textile/js-http-client/node_modules/web-streams-polyfill'. This is likely not portable. A type annotation is necessary.

53   sync(apply?: boolean, wait?: number) {

but if

mv node_modules/@textile/js-http-client/ ../
cd ../js-http-client/
npm install
./node_modules/.bin/tsc

everything is OK.

[albertgnetwork]

If you want to quickly test a new dependency I can recommend a new feature in npm v7: https://docs.npmjs.com/cli/v7/using-npm/workspaces
It did the job for me with this error