Do not suggest paths inside node_modules/.pnpm as module specifiers #42095
Conversation
typescript-bot
added
Author: Team
For Uncommitted Bug
| @@ -6104,7 +6104,14 @@ namespace ts { | |||
| getSymlinkedFiles: () => symlinkedFiles, | |||
| getSymlinkedDirectories: () => symlinkedDirectories, | |||
| setSymlinkedFile: (path, real) => (symlinkedFiles || (symlinkedFiles = new Map())).set(path, real), | |||
| setSymlinkedDirectory: (path, directory) => (symlinkedDirectories || (symlinkedDirectories = new Map())).set(path, directory), | |||
| setSymlinkedDirectory: (path, directory) => { | |||
There was a problem hiding this comment.
This should be handled when it is used for module specifier resolution instead of in general in my opinion since we still want to resolve the cache for answering if file is from referenced project for project construction ?
There was a problem hiding this comment.
I think something inside node_modules/.pnpm/ should never be a project reference redirect, which seems to be the only reason beside module specifier resolution that this is used (fileOrDirectoryExistsUsingSource).
There was a problem hiding this comment.
You are right because it would resolve completely to actual path even if equivalent of npm link kind of thing is used .
|
@weswigham it seems plausible for there to be a user-created hidden directory that should work as expected, but I think anything starting with |
|
@andrewbranch we have |
|
@zkochan might want to weigh in here. |
|
LGTM |
I mean, it does if you do |
| @@ -7067,4 +7074,8 @@ namespace ts { | |||
| return false; | |||
| } | |||
| } | |||
|
|
|||
| export function containsIgnoredPath(path: string) { | |||
| return some(ignoredPaths, p => stringContains(path, p)); | |||
There was a problem hiding this comment.
Does this need to look at canonical path? eg. look at isIgnoredPath and isInPath in sys.ts
There was a problem hiding this comment.
I did look at that, but I noticed that where this is used, we were already doing a containsNodeModulesPathPart check without calling getCanonicalFileName or anything.
…icrosoft#42095) * Create symlink cache when a pnpm module is found * Keep pnpm-internal symlinks out of the symlink cache * Filter out pnpm path from realpath module specifier too * Use ignoredPaths instead of pnpm-specific path
Fixes the superfluous import fix suggestions in #40584 and improves the performance of the fixes by ~60%. Additional work is still necessary to improve perf further, but this is a first step that stands on its own.