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

Type resolution in linked packages #32970

Closed
simonfox opened this issue Aug 19, 2019 · 16 comments · Fixed by #33567
Closed

Type resolution in linked packages #32970

simonfox opened this issue Aug 19, 2019 · 16 comments · Fixed by #33567
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@simonfox
Copy link

simonfox commented Aug 19, 2019

This is essentially a duplicate issue however there has been no response on the existing issues which have been incorrectly closed so I am opening again.

Related issues:
#29221
#29808

Merged PR which hasn't resolved the issue:
#31571

Problem occurs when using yarn link for development purposes.
Window 10 Pro v1903
Typescript 3.5.3
yarn 1.17.3

I've created a repro and described steps over on #29808 but basic steps are below

If you clone https://github.com/simonfox/repro-plugin-one and https://github.com/simonfox/repro-plugin-two and run yarn for both. Then run yarn link in plugin two root, and then link one to the local two with yarn link plugin-two from one. Then you will see the issue in src/features/feature-one/actions.ts (you may need to reload the VS Code window after linking).

This results in error TS2742: The inferred type of 'actions' cannot be named without a reference to 'drive-common/node_modules/typescript-fsa'. This is likely not portable. A type annotation is necessary.

image

UPDATE Should explicitly mention that this is in typings generation. With "declaration": false this error does not occur.

@weswigham
Copy link
Member

If I'm not mistaken, this is incredibly annoying from our perspective, because as far as TS knows, you've loaded two copies of typescript-fsa into the compilation - one in plugin-one's modules, and one in plugin-two's, and we're typechecking by loading both, as is appropriate for each use site. If a type at a position comes from the one which if not your immediate dep, you then get an error like this.

Now, I thought we already had some fancy logic that unified dependencies that were exactly the same version (and maybe we are) - it's possible that logic is missing a bit of code to propegate its gleaned knowledge of the project paths into the specifier generation process.

@simonfox
Copy link
Author

@weswigham sorry can you clarify the following

If a type at a position comes from the one which if not your immediate dep, you then get an error like this

Here I am getting the error from a src file that is part of plugin-one that imports typescript-fsa directly with no reference to anything from plugin-two....which is different to the case you are describing?

This was not an issue in 3.1.x - we started to experience on anything from 3.2.x on. Maybe something with the unification logic got broken in there? Where might I find that logic if I wanted to have a look myself?

@weswigham
Copy link
Member

Somewhere in moduleNameResolver.ts we end up merging things with similar package ids (identically versioned packages found in different locations), I believe causing us to "forget" some of the paths the module could be found at by the time we're trying to generate a specifier in moduleSpecifiers.ts.

@antl3x
Copy link

antl3x commented Aug 20, 2019

@simonfox can you try to compile adding in tsconfig.json:

"incremental": true

After that, try to build. You should get the error,.

Now, try to build again without remove the .tsbuildinfo file, and check if compiles correctly.

I think this info can help us to find the real issue.

For some reason, after have the .tsbuildinfo file, project compiles normally, but not in the first build.

PS: Version used to reproduce this was 3.5.3

@simonfox
Copy link
Author

@nthypes I think that is just because you haven't made any changes between builds so the incremental build finds no work to do?

@antl3x
Copy link

antl3x commented Aug 20, 2019

@nthypes I think that is just because you haven't made any changes between builds so the incremental build finds no work to do?

Nope, because in the first try no dist folder is created, but in the second try the dist appears with transpiled files.

@simonfox
Copy link
Author

simonfox commented Aug 20, 2019

@nthypes but this problem is in typings generation - you will get transpiled .js but there is no .d.ts for the files that caused errors (besides I get the transpiled js on the first run).

With "declaration": false you don't get the problem.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Aug 21, 2019
@yordis
Copy link

yordis commented Sep 11, 2019

This is happening to me as well

The inferred type of 'print' cannot be named without a reference to '@straw-hat/cli-next/node_modules/chalk'. This is likely not portable. A type annotation is necessary.
// my print.ts file
import chalk from 'chalk';
const colors = {
  highlight: chalk.cyan,
  info: chalk.blue,
  debug: chalk.gray,
  warning: chalk.yellow,
  success: chalk.green,
  error: chalk.red,
  line: chalk.grey,
  muted: chalk.grey,
};

export const print = {
  colors,
};
// my cli.ts file
import { print } from '../print';

export class Toolbox {
  // it fails here
  public print = print;
}

I did yarn link if that matters.

I ended up disabling declaration to turn it off

@sheetalkamat
Copy link
Member

@simonfox I am getting error trying to repro this when I try to install the dependencies:

c:\temp\repro-plugin-one>yarn
yarn install v1.17.3
[1/4] Resolving packages...
[2/4] Fetching packages...
error Command failed.
Exit code: 128
Command: git
Arguments: ls-remote --tags --heads git@github.com:simonfox/repro-plugin-two.git
Directory: c:\temp\repro-plugin-one
Output:
git@github.com: Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

@sheetalkamat sheetalkamat added Needs More Info The issue still hasn't been fully clarified and removed Needs Investigation This issue needs a team member to investigate its status. labels Sep 11, 2019
@weswigham
Copy link
Member

I am getting error trying to repro this when I try to install the dependencies:

The git@ url in the dependencies needs to be replaced with an https: equivalent that can be used without auth.

@simonfox
Copy link
Author

@sheetalkamat have updated the source as above

image

@sheetalkamat sheetalkamat added Needs Investigation This issue needs a team member to investigate its status. Bug A bug in TypeScript and removed Needs More Info The issue still hasn't been fully clarified Needs Investigation This issue needs a team member to investigate its status. labels Sep 12, 2019
@sheetalkamat sheetalkamat added the Fix Available A PR has been opened for this issue label Sep 23, 2019
@pleerock
Copy link

Really hope this PR will fix this annoying issue.

@ericmdantas
Copy link

I'm using Typescript 3.7.5 and I'm still facing this issue.

@weswigham mentioned:

[...] as far as TS knows, you've loaded two copies of typescript-fsa into the compilation - one in plugin-one's modules, and one in plugin-two's, and we're typechecking by loading both, as is appropriate for each use site. If a type at a position comes from the one which if not your immediate dep, you then get an error like this. [...]

That's pretty much what's going on for me. I have this lib-core that's used in a bunch of separated lower level applications that are used in some higher level applications. Now, everytime lib-core is update I also have to update its version both in the other lower level applications as in the higher level applications. It's being incredibly annoying to do it by hand, especially because my team and I still have some other lower level applications to write, so it'll only get worse.

Also, I can't set declaration to false because I need the typings.

@simonfox
Copy link
Author

I still face this issue as well......the linked PR has fixed it in the repro I supplied however our app still suffers this issue.

We still see the issue when the problematic package is at the same version across our linked packages.

@simonfox
Copy link
Author

Actually I've just modified the repro slightly and it does still exhibit this problem.
@sheetalkamat @weswigham @RyanCavanaugh can this be looked at again?

@sheetalkamat
Copy link
Member

@simonfox This issue was fixed. Can you please open new issue with exact steps to repro the issue. We will investigate that one. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants