Enable path completions for node12/nodenext #47836
Conversation
typescript-bot
added
Author: Team
For Milestone Bug
|
Oh, and @andrewbranch - it looks like path completions don't use any caching when looking up |
There was a problem hiding this comment.
I think what’s going to happen here is that while the package name will be completed correctly, if you type a / afterwards, you’re going to get a listing of directories, which may be blocked due to the export map. There’s some logic in this file that specifically deals with completing directories that needs to be swapped for completing export map subpaths when relevant.
weswigham
force-pushed
the
path-completions-for-nodenext
branch
from
0c14ad8 to
f608488
Compare
|
@andrewbranch done - in |
| } | ||
| const packageFile = combinePaths(ancestor, "node_modules", packagePath, "package.json"); | ||
| if (tryFileExists(host, packageFile)) { | ||
| const packageJson = readJson(packageFile, host as { readFile: (filename: string) => string | undefined }); |
There was a problem hiding this comment.
Did you know this function creates a SourceFile with parse diagnostics?? 😵
There was a problem hiding this comment.
Hm, I buy it - our json document parsing is more lenient than the JSON.parse one because we allow comments and trailing commas, so I can understand preferring to use it.
There was a problem hiding this comment.
npm crashes on comments and trailing commas. My strong suspicion is that readJson leaked into package.json-reading code only because it nicely avoids the need for a try/catch and it’s totally non-obvious that it’s doing so much extra work. I’ve been meaning to rename it and use a wrapped JSON.parse on package.jsons. It might make a little bit of a perf difference in module resolution when you have a lot of node_modules.
There was a problem hiding this comment.
That's probably true; but I mean, it's probably most important to just use the cached package.json results where possible first 😅
Fixes #47011
Just a matter of swapping some checks in
stringCompletionsto look for any node-like mode rather than justnode. Did a quick check, and there don't seem to be any other resolution-mode dependent codepaths in services.