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

No import suggestions for transitive dependencies, even after initial import #38768

Closed
jweisman opened this issue May 25, 2020 · 4 comments
Closed
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@jweisman
Copy link

TypeScript Version: 3.8.3

Search Terms:

Code

import { CloudAppRestService } from '@exlibris/exl-cloudapp-angular-lib';

export class Test {
  rest: CloudAppRestService;
  config: CloudAppConfigService;
}

Expected behavior:
According to this comment from @andrewbranch , once an import is made anywhere in the project, other imports from the same package should be recommended. We're not seeing that behavior.

Actual behavior:
Second import (even in the same file!) is not recommended

Screen

Related Issues:
#36042, #30033, #35560

@andrewbranch
Copy link
Member

Transitive dependencies are intentionally filtered out, which is why #35560 is marked "working as intended." It does seem reasonable to stop filtering them out within the same file, although I’m not sure how big of a performance penalty that will incur (but it will incur one). But the idea is that you should put your top-level dependencies in your package.json. By directly importing transitive dependencies, you’re putting your app at the mercy of your package manager’s file system structuring algorithm, decisions made within the (potentially private) APIs of your direct dependencies, and the subtle interactions between those. For example, if you installed another dependency that depended on a different major version of @exlibris/exl-cloudapp-angular-lib than what you already have, you could all of a sudden be importing a totally different version of that dependency than you meant to. And what’s worse, which version you get could depend on whether you’re using npm, pnpm, or yarn. I feel reasonably confident in saying that you should just add your dependencies to your package.json, and TypeScript shouldn’t take a performance hit to streamline an unsafe and (I think) quite uncommon practice. Unless there’s a good reason to do this that I’m totally missing, which is possible?

@jweisman
Copy link
Author

jweisman commented Jun 3, 2020

Hi @andrewbranch - thanks for your response!

I understand you're not in favor of our approach. It works for us as we're providing an SDK for our customers to build apps, and we find it helpful to require them to reference our library and to hide the rest of the dependencies.

Regardless, the point of this issue is to clarify the expected behavior. I understood your comment here to mean that once I've imported a reference anywhere in my project, subsequent references to types from the same library should appear as suggestions. Is my understanding mistaken?

Thanks.

@andrewbranch
Copy link
Member

I understood your comment here to mean that once I've imported a reference anywhere in my project, subsequent references to types from the same library should appear as suggestions. Is my understanding mistaken?

Yes, because my comment was not phrased 100% accurately. To explain, you can imagine the auto-import list generation process as a series of filters:

All TS/JS(X)/JSON files on your hard drive
  (1)> Files in your current TS program
  (2)> Files that are reachable from the file you’re editing
  (3)> Files that are either not in node_modules or part of packages listed in your package.json deps

A huge chunk of all complaints and confusion we get about auto-imports is people misunderstanding Filter 1 in this diagram: the module they want to auto-import is in their node_modules, but TypeScript literally has no idea it exists yet because it doesn’t eagerly search all of node_modules. That’s what my comment was in reference to: once you import a module once, it is definitely in your program, so Filter 1 will definitely not filter that module out ever again.

However, that doesn’t keep the other filters from kicking in. In your case, you’re hitting Filter 3 in this diagram. Filter 3 has been in place for almost a year and this is the first negative feedback about it I’ve heard.

we find it helpful to require them to reference our library and to hide the rest of the dependencies

It feels to me like these dependencies aren’t really hidden if they need to know the names of the packages. Have you considered re-exporting the transitive dependencies from the top-level dependency?

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jun 4, 2020
@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

4 participants