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

The inferred type of "X" cannot be named without a reference to "Y". This is likely not portable. #508

Open
GuillaumeLagrange opened this issue Feb 3, 2023 · 11 comments
Labels
bug Something isn't working external dependency Workaround provided, waiting for an external dependency to fully resolve

Comments

@GuillaumeLagrange
Copy link
Collaborator

GuillaumeLagrange commented Feb 3, 2023

Description

With how PNPM manages packages, in a swarmion project we can easily run into the following issue:

  • Package @my-app/shared-foo packages has package-a as dependency, exporting a TypeA type
    • In this package, define a function sharedFunction whose return type depends directly on TypeA
  • Service @my-app/backend-user has @my-app/shared-foo as dependency and uses sharedFunction.

In @my-app/backend-user:

const bar = sharedFunction();

This will create an LSP warning
The inferred type of 'bar' cannot be named without a reference to '../path/to/shared-foo/node_modules/package-a'. This is likely not portable. A type annotation is necessary.

Minimal reproduction on the full stack example: 8442a7b (line 15@examples/swarmion-full-stack/frontend/app/src/main.tsx)

Imperfect fix

  • Add package-a in devDependencies of @my-app/backend-user, importe the TypeA and explicitely type the return value of the functions
    • This is very imperfect because we should need to worry about the dependencies of our dependencies, and types are not always straightforward to import

Possible leads

  • Investigate why the issue happens only in the LSP and node when running test-type
  • Typescript options ?
  • PNPM hoisting options experimentations ? It would be a shame to get rid of the virtual store though

Related issues

@GuillaumeLagrange GuillaumeLagrange added the bug Something isn't working label Feb 3, 2023
@GuillaumeLagrange
Copy link
Collaborator Author

After investigation, I have found a less imperfect fix:

In @my-app/shared-foo, reexport TypeA from package-a

It suppresses warnings, but is imperfect because there is no way to know there is an issue until you encounter it by consuming your package

@adriencaccia
Copy link
Member

Thanks for the issue @GuillaumeLagrange, you did a great job pinpointing the issue to this simple cause 💪

  • Investigate why the issue happens only in the LSP and node when running test-type

Here it would be better to put: ... and not when running tsc directly in the cli through the test-type script

Can you open an issue on the pnpm repo and link this issue? I think this is the best way forward, hopefully someone there would be able to help us 🙂

In the meantime, the fix you mentioned can be used. And I would recommend adding a comment linking to this issue above the export type ... statement.

@fargito
Copy link
Member

fargito commented Feb 14, 2023

@GuillaumeLagrange is this fixed by #541 ?

@GuillaumeLagrange
Copy link
Collaborator Author

GuillaumeLagrange commented Feb 16, 2023

Nope, I have managed to reproduce the issue in a minimal repo, and opened an issue on pnpm repo to see if anybody has an idea of what is happening.

Interestingly enough, the issue occurs when running tsc, but does not when running tsc --noEmit, hence the "only appears in LSP but not in cli type checks" syndrom if the issue occurs in a service and not a package

@GuillaumeLagrange
Copy link
Collaborator Author

An answer on the pnpm issue pointed out a decent workaround that will do until the root cause is fixed:

image

@fargito
Copy link
Member

fargito commented Feb 17, 2023

This seems sensible, we'll keep the issue open during this time

@fargito fargito added the external dependency Workaround provided, waiting for an external dependency to fully resolve label Feb 17, 2023
@adriencaccia
Copy link
Member

adriencaccia commented Mar 6, 2023

I just had the same problem on a project, but this time on a service package, not a shared code library.
Since this project is not aimed to be shared, I could solve the error by removing all the properties related to declaration:

image

It seems that this issue is happening more and more on more and more monorepos with pnpm and TypeScript.

Hopefully, this fix can help when the above fix mentioned by @GuillaumeLagrange does not work.

The following properties are important: composite, noEmit, declaration, declarationMap and emitDeclarationOnly:

{
  "extends": "../../tsconfig.options.json",
  "compilerOptions": {
    "baseUrl": ".",
    "composite": false,
    "noEmit": true,
    "declaration": false,
    "declarationMap": false,
    "emitDeclarationOnly": false,
  },
  "references": [ ... ],
  "include": ["./**/*.ts"]
}

@adriencaccia
Copy link
Member

@GuillaumeLagrange we ought to try this microsoft/TypeScript#42873 (comment)

@pkerschbaum
Copy link

FYI I added a comment in TypeScript#47663 which has workarounds for this issue when using pnpm monorepos.

@GuillaumeLagrange
Copy link
Collaborator Author

Thank you for this, it really sums up quite clearly the different workarounds and their eventual downsides.

Not much more we can do than wait on a root fix from either Typescript or pnpm now, not sure how they would go about that though.

@adriencaccia
Copy link
Member

Thanks @pkerschbaum for the link to the detailed explanation!

Since the problem occurs only on packages not meant to be consumed by others, we will go with the solution of setting declaration: false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working external dependency Workaround provided, waiting for an external dependency to fully resolve
Projects
None yet
Development

No branches or pull requests

4 participants