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

Portability error issued when reachable source file is redirected to an unreachable target #49171

Open
jordanmbell opened this issue May 19, 2022 · 2 comments · Fixed by #56087
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files

Comments

@jordanmbell
Copy link
Contributor

Bug Report

🔎 Search Terms

resolution, symlinks, references, incorrect dependency

🕗 Version & Regression Information

I have tested [email protected] and typescript@next and both experience this.

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about module resolution

⏯ Playground Link

I can't give a playground link as this issue revolves around dependencies

💻 Code

I have created a repo demonstrating this here: https://github.com/jordanmbell/ts-resolution-issue

It is difficult to demonstrate this issue in a few lines because it revolves around dependencies, but the errors are reported here.

import { BAD_TYPE, OTHER_BAD } from "fake-dep";

export const bad = (arg: BAD_TYPE) => {
  return arg;
};

const other_bad: OTHER_BAD = 1;

console.log(bad);

The problem

Screenshot of the issue

Here we see that there are two errors reported, one saying:
The inferred type of 'bad' cannot be named without a reference to 'subpackage-a/node_modules/fake-dep/dist/BadType'. This is likely not portable. A type annotation is necessary.ts(2742)
and one saying:
Type 'number' is not assignable to type 'string'.ts(2322)

However from the screenshots of the dependency inside that package, it would seem this error should not be a problem, as OTHER_BAD has type number, and there is a direct reference to BAD_TYPE. Strangely though, typescript tells us it is actually using the version of fake-dep from a completely different subpackage.

image

image

I believe the problem is that typescript is caching dependencies when it first finds them, and uses the same located version of that dependency when it finds it used later, even if that dependency should not be reachable. This repo demonstrates that inside of subpackage-d, which has an index file that imports fake-dep and creates some variables. tsc reports a problem though, that other_bad should be a string, and that bad cannot be named without a reference to 'subpackage-a/node_modules/fake-dep/dist/BadType'. These errors occur because rather than subpackage-d using the version of fake-dep found inside its node_modules, it uses the version found inside of subpackage-a's node_modules.

Through testing it seems that subpackage-a's version is being used because subpackage-d references and imports subpackage-c, and following the chain of references and imports eventually reaches the import for fake-dep inside of subpackage-a. This seems to then be cached, and when TypeScript finally gets back to subpackage-d's index file, it uses the cached version for fake-dep, rather than looking for a version from the current folder. I believe this to be a caching issue because if the file inside subpackage-d that imports subpackage-c (indew.js) is alphabetically located after the index file, such as renaming it to indey.js, the issue no longer exists.

Why this is a problem

For most purposes, the caching of a dependency like fake-dep would not matter. In fact one of the two issues, the one with other-bad saying it should be a string is not an issue that would normally come up, it only occurs here because I have manually edited the files inside node_modules to be different even though subpackage-d and subpackage-a have dependencies on the same version, and thus the files should all be the same.

The second issue with bad cannot be named without a reference to 'subpackage-a/node_modules/fake-dep/dist/BadType' is a real problem though. Because TypeScript has cached the initial location of where fake-dep was resolved to inside subpackage-a, it believes that types can not be inferred without a reference to that location, as it does not see that subpackage-d has any direct reference to subpackage-a/node_modules/fake-dep/dist/BadType. This is likely the reason for many of the ts(2742) issues people have had in the past, as it is erroneous and very confusing to track down why an error is being reported.

What should happen instead

Rather then directly using the cached version of a dependency, TypeScript should use use the same computed exported types but change the location where it has marked the types as being resolved from. This would keep the benefit of not recalculating types for the same package, but would fix the error where it thinks types are being used without a reference.

@andrewbranch
Copy link
Member

For most purposes, the caching of a dependency like fake-dep would not matter. In fact one of the two issues, the one with other-bad saying it should be a string is not an issue that would normally come up, it only occurs here because I have manually edited the files inside node_modules to be different even though subpackage-d and subpackage-a have dependencies on the same version, and thus the files should all be the same.

Indeed, this is working as intended. The other part does seem to be a bug with redirected source files.

@andrewbranch andrewbranch added the Bug A bug in TypeScript label May 24, 2022
@andrewbranch andrewbranch changed the title Modules resolving to the wrong dependency when using symlinks and references Portability error issued when reachable source file is redirected to an unreachable target May 24, 2022
@andrewbranch andrewbranch added the Domain: Declaration Emit The issue relates to the emission of d.ts files label May 24, 2022
@jakebailey
Copy link
Member

#56087 has been reverted.

@jakebailey jakebailey reopened this Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants