From 5abd4628afa4895c3ae2aad391cdd560deb133f2 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 22 Dec 2020 12:45:10 -0800 Subject: [PATCH 1/5] Create symlink cache when a pnpm module is found --- src/compiler/moduleNameResolver.ts | 6 ++++++ src/compiler/program.ts | 2 +- .../cases/fourslash/server/autoImportPnpm.ts | 0 .../server/autoImportProviderPnpm.ts | 21 +++++++++++++++++++ 4 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/server/autoImportPnpm.ts create mode 100644 tests/cases/fourslash/server/autoImportProviderPnpm.ts diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 6329387b281d7..7d402de8f1820 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -1020,6 +1020,12 @@ namespace ts { export function pathContainsNodeModules(path: string): boolean { return stringContains(path, nodeModulesPathPart); } + /*@internal*/ + export const pnpmPathPart = "/node_modules/.pnpm/"; + /*@internal*/ + export function pathContainsPnpmDirectory(path: string): boolean { + return stringContains(path, pnpmPathPart); + } /** * This will be called on the successfully resolved path from `loadModuleFromFile`. diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 7b71f011700c1..bf39c96402f77 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -3760,7 +3760,7 @@ namespace ts { } function handleDirectoryCouldBeSymlink(directory: string) { - if (!host.getResolvedProjectReferences()) return; + if (!host.getResolvedProjectReferences() && !pathContainsPnpmDirectory(directory)) return; // Because we already watch node_modules, handle symlinks in there if (!originalRealpath || !stringContains(directory, nodeModulesPathPart)) return; diff --git a/tests/cases/fourslash/server/autoImportPnpm.ts b/tests/cases/fourslash/server/autoImportPnpm.ts new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/tests/cases/fourslash/server/autoImportProviderPnpm.ts b/tests/cases/fourslash/server/autoImportProviderPnpm.ts new file mode 100644 index 0000000000000..67a8b613420ba --- /dev/null +++ b/tests/cases/fourslash/server/autoImportProviderPnpm.ts @@ -0,0 +1,21 @@ +/// + +// @Filename: /tsconfig.json +//// { "compilerOptions": { "module": "commonjs" } } + +// @Filename: /package.json +//// { "dependencies": { "mobx": "*" } } + +// @Filename: /node_modules/.pnpm/mobx@6.0.4/node_modules/mobx/package.json +//// { "types": "dist/mobx.d.ts" } + +// @Filename: /node_modules/.pnpm/mobx@6.0.4/node_modules/mobx/dist/mobx.d.ts +//// export declare function autorun(): void; + +// @Filename: /index.ts +//// autorun/**/ + +// @link: /node_modules/.pnpm/mobx@6.0.4/node_modules/mobx -> /node_modules/mobx + +goTo.marker(""); +verify.importFixAtPosition([`import { autorun } from "mobx";\r\n\r\nautorun`]); From 406cb4efacc16dc145c3bdc3af1fffd66b0341f6 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 23 Dec 2020 14:00:36 -0800 Subject: [PATCH 2/5] Keep pnpm-internal symlinks out of the symlink cache --- src/compiler/program.ts | 2 +- src/compiler/utilities.ts | 9 +++++++- .../server/autoImportProviderPnpm.ts | 21 ------------------- 3 files changed, 9 insertions(+), 23 deletions(-) delete mode 100644 tests/cases/fourslash/server/autoImportProviderPnpm.ts diff --git a/src/compiler/program.ts b/src/compiler/program.ts index bf39c96402f77..ca4ad34fb2ccd 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -3760,7 +3760,7 @@ namespace ts { } function handleDirectoryCouldBeSymlink(directory: string) { - if (!host.getResolvedProjectReferences() && !pathContainsPnpmDirectory(directory)) return; + if (!host.getResolvedProjectReferences() || pathContainsPnpmDirectory(directory)) return; // Because we already watch node_modules, handle symlinks in there if (!originalRealpath || !stringContains(directory, nodeModulesPathPart)) return; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 12b815995cfa7..fa0ffb4df808b 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -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) => { + // Large, interconnected dependency graphs in pnpm will have a huge number of symlinks + // where both the realpath and the symlink path are inside node_modules/.pnpm. Since + // this path is never a candidate for a module specifier, we can ignore it entirely. + if (!pathContainsPnpmDirectory(path)) { + (symlinkedDirectories || (symlinkedDirectories = new Map())).set(path, directory); + } + } }; } diff --git a/tests/cases/fourslash/server/autoImportProviderPnpm.ts b/tests/cases/fourslash/server/autoImportProviderPnpm.ts deleted file mode 100644 index 67a8b613420ba..0000000000000 --- a/tests/cases/fourslash/server/autoImportProviderPnpm.ts +++ /dev/null @@ -1,21 +0,0 @@ -/// - -// @Filename: /tsconfig.json -//// { "compilerOptions": { "module": "commonjs" } } - -// @Filename: /package.json -//// { "dependencies": { "mobx": "*" } } - -// @Filename: /node_modules/.pnpm/mobx@6.0.4/node_modules/mobx/package.json -//// { "types": "dist/mobx.d.ts" } - -// @Filename: /node_modules/.pnpm/mobx@6.0.4/node_modules/mobx/dist/mobx.d.ts -//// export declare function autorun(): void; - -// @Filename: /index.ts -//// autorun/**/ - -// @link: /node_modules/.pnpm/mobx@6.0.4/node_modules/mobx -> /node_modules/mobx - -goTo.marker(""); -verify.importFixAtPosition([`import { autorun } from "mobx";\r\n\r\nautorun`]); From a02a2e7e17ce97bb26e7b1fe2c224fba2a551b6a Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 23 Dec 2020 15:15:01 -0800 Subject: [PATCH 3/5] Filter out pnpm path from realpath module specifier too --- src/compiler/moduleSpecifiers.ts | 9 ++++--- src/harness/fourslashImpl.ts | 2 +- tests/cases/fourslash/autoImportPnpm.ts | 24 +++++++++++++++++++ .../cases/fourslash/server/autoImportPnpm.ts | 0 4 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 tests/cases/fourslash/autoImportPnpm.ts delete mode 100644 tests/cases/fourslash/server/autoImportPnpm.ts diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index d2fc45ee9913d..727d1fb147e4a 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -278,7 +278,9 @@ namespace ts.moduleSpecifiers { const importedFileNames = [...(referenceRedirect ? [referenceRedirect] : emptyArray), importedFileName, ...redirects]; const targets = importedFileNames.map(f => getNormalizedAbsolutePath(f, cwd)); if (!preferSymlinks) { - const result = forEach(targets, p => cb(p, referenceRedirect === p)); + // Symlinks inside node_modules/.pnpm are already filtered out of the symlink cache, + // so we only need to remove them from the realpath filenames. + const result = forEach(targets, p => !pathContainsPnpmDirectory(p) && cb(p, referenceRedirect === p)); if (result) return result; } const links = host.getSymlinkCache @@ -306,8 +308,9 @@ namespace ts.moduleSpecifiers { } }); }); - return result || - (preferSymlinks ? forEach(targets, p => cb(p, p === referenceRedirect)) : undefined); + return result || (preferSymlinks + ? forEach(targets, p => pathContainsPnpmDirectory(p) ? undefined : cb(p, p === referenceRedirect)) + : undefined); } interface ModulePath { diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index 551ac9aef1889..48be5004d3cb2 100644 --- a/src/harness/fourslashImpl.ts +++ b/src/harness/fourslashImpl.ts @@ -3022,7 +3022,7 @@ namespace FourSlash { this.editScriptAndUpdateMarkers(fileName, span.start, span.start + insertedText.length, deletedText); } if (expectedTextArray.length !== actualTextArray.length) { - this.raiseError(`Expected ${expectedTextArray.length} import fixes, got ${actualTextArray.length}`); + this.raiseError(`Expected ${expectedTextArray.length} import fixes, got ${actualTextArray.length}:\n\n${actualTextArray.join("\n\n" + "-".repeat(20) + "\n\n")}`); } ts.zipWith(expectedTextArray, actualTextArray, (expected, actual, index) => { if (expected !== actual) { diff --git a/tests/cases/fourslash/autoImportPnpm.ts b/tests/cases/fourslash/autoImportPnpm.ts new file mode 100644 index 0000000000000..91d52ab798406 --- /dev/null +++ b/tests/cases/fourslash/autoImportPnpm.ts @@ -0,0 +1,24 @@ +/// + +// @Filename: /tsconfig.json +//// { "compilerOptions": { "module": "commonjs" } } + +// @Filename: /node_modules/.pnpm/mobx@6.0.4/node_modules/mobx/package.json +//// { "types": "dist/mobx.d.ts" } + +// @Filename: /node_modules/.pnpm/mobx@6.0.4/node_modules/mobx/dist/mobx.d.ts +//// export declare function autorun(): void; + +// @Filename: /index.ts +//// autorun/**/ + +// @Filename: /utils.ts +//// import "mobx"; + +// @link: /node_modules/.pnpm/mobx@6.0.4/node_modules/mobx -> /node_modules/mobx +// @link: /node_modules/.pnpm/mobx@6.0.4/node_modules/mobx -> /node_modules/.pnpm/cool-mobx-dependent@1.2.3/node_modules/mobx + +goTo.marker(""); +verify.importFixAtPosition([`import { autorun } from "mobx"; + +autorun`]); diff --git a/tests/cases/fourslash/server/autoImportPnpm.ts b/tests/cases/fourslash/server/autoImportPnpm.ts deleted file mode 100644 index e69de29bb2d1d..0000000000000 From 1780acfa31e3284a1fd13b718c409c56630ccbf1 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 12 Jan 2021 13:07:26 -0800 Subject: [PATCH 4/5] Use ignoredPaths instead of pnpm-specific path --- src/compiler/moduleNameResolver.ts | 6 ------ src/compiler/moduleSpecifiers.ts | 4 ++-- src/compiler/program.ts | 2 +- src/compiler/utilities.ts | 6 +++++- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 7d402de8f1820..6329387b281d7 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -1020,12 +1020,6 @@ namespace ts { export function pathContainsNodeModules(path: string): boolean { return stringContains(path, nodeModulesPathPart); } - /*@internal*/ - export const pnpmPathPart = "/node_modules/.pnpm/"; - /*@internal*/ - export function pathContainsPnpmDirectory(path: string): boolean { - return stringContains(path, pnpmPathPart); - } /** * This will be called on the successfully resolved path from `loadModuleFromFile`. diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 727d1fb147e4a..3b72274bc517e 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -280,7 +280,7 @@ namespace ts.moduleSpecifiers { if (!preferSymlinks) { // Symlinks inside node_modules/.pnpm are already filtered out of the symlink cache, // so we only need to remove them from the realpath filenames. - const result = forEach(targets, p => !pathContainsPnpmDirectory(p) && cb(p, referenceRedirect === p)); + const result = forEach(targets, p => !containsIgnoredPath(p) && cb(p, referenceRedirect === p)); if (result) return result; } const links = host.getSymlinkCache @@ -309,7 +309,7 @@ namespace ts.moduleSpecifiers { }); }); return result || (preferSymlinks - ? forEach(targets, p => pathContainsPnpmDirectory(p) ? undefined : cb(p, p === referenceRedirect)) + ? forEach(targets, p => containsIgnoredPath(p) ? undefined : cb(p, p === referenceRedirect)) : undefined); } diff --git a/src/compiler/program.ts b/src/compiler/program.ts index ca4ad34fb2ccd..45c541945487a 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -3760,7 +3760,7 @@ namespace ts { } function handleDirectoryCouldBeSymlink(directory: string) { - if (!host.getResolvedProjectReferences() || pathContainsPnpmDirectory(directory)) return; + if (!host.getResolvedProjectReferences() || containsIgnoredPath(directory)) return; // Because we already watch node_modules, handle symlinks in there if (!originalRealpath || !stringContains(directory, nodeModulesPathPart)) return; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index fa0ffb4df808b..cc383919dc306 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -6108,7 +6108,7 @@ namespace ts { // Large, interconnected dependency graphs in pnpm will have a huge number of symlinks // where both the realpath and the symlink path are inside node_modules/.pnpm. Since // this path is never a candidate for a module specifier, we can ignore it entirely. - if (!pathContainsPnpmDirectory(path)) { + if (!containsIgnoredPath(path)) { (symlinkedDirectories || (symlinkedDirectories = new Map())).set(path, directory); } } @@ -7074,4 +7074,8 @@ namespace ts { return false; } } + + export function containsIgnoredPath(path: string) { + return some(ignoredPaths, p => stringContains(path, p)); + } } From d3e67b69ebeb18a4e4046dd8875d34f9b1977ca0 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 12 Jan 2021 14:02:55 -0800 Subject: [PATCH 5/5] Update comment --- src/compiler/moduleSpecifiers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 3b72274bc517e..45f2edf3df988 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -278,7 +278,7 @@ namespace ts.moduleSpecifiers { const importedFileNames = [...(referenceRedirect ? [referenceRedirect] : emptyArray), importedFileName, ...redirects]; const targets = importedFileNames.map(f => getNormalizedAbsolutePath(f, cwd)); if (!preferSymlinks) { - // Symlinks inside node_modules/.pnpm are already filtered out of the symlink cache, + // Symlinks inside ignored paths are already filtered out of the symlink cache, // so we only need to remove them from the realpath filenames. const result = forEach(targets, p => !containsIgnoredPath(p) && cb(p, referenceRedirect === p)); if (result) return result;