Repository navigation
Automatic Type Acquisition based on filename matching pulls in too many irrelevant typings #40123
Description
Consider a simple JavaScript project with files like the following
/flight.js
/platform.js
/myfile.js
The names flight and platform seem innocuous enough, but their mere presence causes Automatic Type Acquisition to pull in @types/flight and @types/platform. Because these words are so common, I would argue it's highly unlikely that the user actually needs the typings for these libraries - much more likely that the files are just regular files in the user's project. I think ATA's behavior is a little too eager, and causes unnecessary files and symbols to be added to the project context. There's also no visual indication that this is actually happening (but you can confirm it by checking TS Server logs).
This behavior happens in VS Code if you have the files open in the editor. But it occurs in VS if you have them present in your project at all.
I propose one or more of the following solutions:
- Disable this filename-matching behavior entirely
- Trim back the allow-list considerably to only include obvious libraries like
jquery. - Give a visual indicator to the user that ATA is doing this, and give them a way to disable it
Activity
sheetalkamat commentedon Aug 19, 2020
This has heuristic that is at:
TypeScript/src/server/project.ts
Line 1711 in 31f4957
where it does ATA if the resolved module is not ts or json and is referenced using non relative name. This is been around for some time but i think the reason was that eg if you resolved to
node_modules\mochayou instead want.d.tsfile for it instead. May be adding check forisExternalLibraryImportmight be a good addition to check there.minestarks commentedon Aug 19, 2020
It's been like this forever yes, I think the original intent (Ryan Cavanaugh (@RyanCavanaugh) would know) was to detect third party libraries in non-module based code, e.g. a project that just has
jquery.jsorjquery.min.jscopied in. There are lots of existing VS projects that do this. How do we detect that that file is a dependency, and not user code? At the time we assumed filename matching with an allow-list might be a good heuristic. I'm not so sure of that anymore.sheetalkamat commentedon Aug 19, 2020
isExternalLibraryImportproperty of the resolution would let us know that since that indicates the resolution is fromnode_modulesminestarks commentedon Aug 19, 2020
Thanks for the pointer. To be clear, in the scenario I describe, there is no
node_modules, and we still intend to do ATA.jessetrinity commentedon Sep 21, 2020
I think a good solution is a combination of bullet points 2 + 3.
VSCode already shows a status bar for this message and I intend to add one to VS:
Contributors for other editor will likely find this issue and if they are also trying to address ATA notifications.
We have data from npm and VS that tells us which packages are downloaded most frequently. We can shorten the list to the top 10-20 from those lists. These include the expected popular packages like lodash, jquery, and bootstrap.
With both changes, users would be getting typings only for popular packages that are probably the jqueryies they are looking for, and if not, be aware of what they are getting through the notification.