Skip to content

Commit

Permalink
Fix ESM/CJS resolution cache collision in non-nodenext resolution mod…
Browse files Browse the repository at this point in the history
…es (#60910)
  • Loading branch information
andrewbranch authored Jan 21, 2025
1 parent 8da951c commit b78f466
Show file tree
Hide file tree
Showing 8 changed files with 578 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/compiler/moduleNameResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2989,7 +2989,7 @@ function loadModuleFromNearestNodeModulesDirectoryTypesScope(moduleName: string,
}

function loadModuleFromNearestNodeModulesDirectoryWorker(extensions: Extensions, moduleName: string, directory: string, state: ModuleResolutionState, typesScopeOnly: boolean, cache: ModuleResolutionCache | undefined, redirectedReference: ResolvedProjectReference | undefined): SearchResult<Resolved> {
const mode = state.features === 0 ? undefined : state.features & NodeResolutionFeatures.EsmMode ? ModuleKind.ESNext : ModuleKind.CommonJS;
const mode = state.features === 0 ? undefined : (state.features & NodeResolutionFeatures.EsmMode || state.conditions.includes("import")) ? ModuleKind.ESNext : ModuleKind.CommonJS;
// Do (up to) two passes through node_modules:
// 1. For each ancestor node_modules directory, try to find:
// i. TS/DTS files in the implementation package
Expand Down
23 changes: 23 additions & 0 deletions src/testRunner/unittests/tsserver/inferredProjects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -322,4 +322,27 @@ describe("unittests:: tsserver:: inferredProjects::", () => {
}], session);
baselineTsserverLogs("inferredProjects", "when existing inferred project has no root files", session);
});

it("closing file with shared resolutions", () => {
const host = TestServerHost.createServerHost({
"/user/username/projects/myproject/unrelated.ts": dedent`
export {};
`,
"/user/username/projects/myproject/app.ts": dedent`
import type { y } from "pkg" assert { "resolution-mode": "require" };
import type { x } from "pkg" assert { "resolution-mode": "import" };
`,
});
const session = new TestSession(host);
openFilesForSession([{
file: "/user/username/projects/myproject/unrelated.ts",
projectRootPath: "/user/username/projects/myproject",
}], session);
openFilesForSession([{
file: "/user/username/projects/myproject/app.ts",
projectRootPath: "/user/username/projects/myproject",
}], session);
closeFilesForSession(["/user/username/projects/myproject/app.ts"], session);
baselineTsserverLogs("inferredProjects", "closing file with shared resolutions", session);
});
});
35 changes: 35 additions & 0 deletions tests/baselines/reference/resolutionModeCache.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/index.ts(3,1): error TS1361: 'pkgRequire' cannot be used as a value because it was imported using 'import type'.
/index.ts(4,1): error TS1361: 'pkgImport' cannot be used as a value because it was imported using 'import type'.


==== /node_modules/pkg/package.json (0 errors) ====
{
"name": "pkg",
"version": "1.0.0",
"exports": {
".": {
"import": "./index.mjs",
"require": "./index.js"
}
}
}

==== /node_modules/pkg/index.d.mts (0 errors) ====
declare const _default: "esm";
export default _default;

==== /node_modules/pkg/index.d.ts (0 errors) ====
declare const _exports: "cjs";
export = _exports;

==== /index.ts (2 errors) ====
import type pkgRequire from "pkg" with { "resolution-mode": "require" };
import type pkgImport from "pkg" with { "resolution-mode": "import" };
pkgRequire;
~~~~~~~~~~
!!! error TS1361: 'pkgRequire' cannot be used as a value because it was imported using 'import type'.
!!! related TS1376 /index.ts:1:8: 'pkgRequire' was imported here.
pkgImport;
~~~~~~~~~
!!! error TS1361: 'pkgImport' cannot be used as a value because it was imported using 'import type'.
!!! related TS1376 /index.ts:2:8: 'pkgImport' was imported here.
29 changes: 29 additions & 0 deletions tests/baselines/reference/resolutionModeCache.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//// [tests/cases/conformance/moduleResolution/resolutionModeCache.ts] ////

=== /node_modules/pkg/index.d.mts ===
declare const _default: "esm";
>_default : Symbol(_default, Decl(index.d.mts, 0, 13))

export default _default;
>_default : Symbol(_default, Decl(index.d.mts, 0, 13))

=== /node_modules/pkg/index.d.ts ===
declare const _exports: "cjs";
>_exports : Symbol(_exports, Decl(index.d.ts, 0, 13))

export = _exports;
>_exports : Symbol(_exports, Decl(index.d.ts, 0, 13))

=== /index.ts ===
import type pkgRequire from "pkg" with { "resolution-mode": "require" };
>pkgRequire : Symbol(pkgRequire, Decl(index.ts, 0, 6))

import type pkgImport from "pkg" with { "resolution-mode": "import" };
>pkgImport : Symbol(pkgImport, Decl(index.ts, 1, 6))

pkgRequire;
>pkgRequire : Symbol(pkgRequire, Decl(index.ts, 0, 6))

pkgImport;
>pkgImport : Symbol(pkgImport, Decl(index.ts, 1, 6))

112 changes: 112 additions & 0 deletions tests/baselines/reference/resolutionModeCache.trace.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
[
"Found 'package.json' at '/node_modules/pkg/package.json'.",
"======== Resolving module 'pkg' from '/index.ts'. ========",
"Explicitly specified module resolution kind: 'Bundler'.",
"Resolving in CJS mode with conditions 'require', 'types'.",
"File '/package.json' does not exist.",
"Loading module 'pkg' from 'node_modules' folder, target file types: TypeScript, JavaScript, Declaration, JSON.",
"Searching all ancestor node_modules directories for preferred extensions: TypeScript, Declaration.",
"File '/node_modules/pkg/package.json' exists according to earlier cached lookups.",
"Entering conditional exports.",
"Saw non-matching condition 'import'.",
"Matched 'exports' condition 'require'.",
"Using 'exports' subpath '.' with target './index.js'.",
"File name '/node_modules/pkg/index.js' has a '.js' extension - stripping it.",
"File '/node_modules/pkg/index.ts' does not exist.",
"File '/node_modules/pkg/index.tsx' does not exist.",
"File '/node_modules/pkg/index.d.ts' exists - use it as a name resolution result.",
"'package.json' does not have a 'peerDependencies' field.",
"Resolved under condition 'require'.",
"Exiting conditional exports.",
"Resolving real path for '/node_modules/pkg/index.d.ts', result '/node_modules/pkg/index.d.ts'.",
"======== Module name 'pkg' was successfully resolved to '/node_modules/pkg/index.d.ts' with Package ID 'pkg/[email protected]'. ========",
"======== Resolving module 'pkg' from '/index.ts'. ========",
"Explicitly specified module resolution kind: 'Bundler'.",
"Resolving in CJS mode with conditions 'import', 'types'.",
"File '/package.json' does not exist according to earlier cached lookups.",
"Loading module 'pkg' from 'node_modules' folder, target file types: TypeScript, JavaScript, Declaration, JSON.",
"Searching all ancestor node_modules directories for preferred extensions: TypeScript, Declaration.",
"File '/node_modules/pkg/package.json' exists according to earlier cached lookups.",
"Entering conditional exports.",
"Matched 'exports' condition 'import'.",
"Using 'exports' subpath '.' with target './index.mjs'.",
"File name '/node_modules/pkg/index.mjs' has a '.mjs' extension - stripping it.",
"File '/node_modules/pkg/index.mts' does not exist.",
"File '/node_modules/pkg/index.d.mts' exists - use it as a name resolution result.",
"Resolved under condition 'import'.",
"Exiting conditional exports.",
"Resolving real path for '/node_modules/pkg/index.d.mts', result '/node_modules/pkg/index.d.mts'.",
"======== Module name 'pkg' was successfully resolved to '/node_modules/pkg/index.d.mts' with Package ID 'pkg/[email protected]'. ========",
"======== Resolving module '@typescript/lib-es5' from '/.src/__lib_node_modules_lookup_lib.es5.d.ts__.ts'. ========",
"Explicitly specified module resolution kind: 'Node10'.",
"Loading module '@typescript/lib-es5' from 'node_modules' folder, target file types: TypeScript, Declaration.",
"Searching all ancestor node_modules directories for preferred extensions: TypeScript, Declaration.",
"Directory '/.src/node_modules' does not exist, skipping all lookups in it.",
"Scoped package detected, looking in 'typescript__lib-es5'",
"Directory '/node_modules/@types' does not exist, skipping all lookups in it.",
"Scoped package detected, looking in 'typescript__lib-es5'",
"Loading module '@typescript/lib-es5' from 'node_modules' folder, target file types: JavaScript.",
"Searching all ancestor node_modules directories for fallback extensions: JavaScript.",
"Directory '/.src/node_modules' does not exist, skipping all lookups in it.",
"======== Module name '@typescript/lib-es5' was not resolved. ========",
"======== Resolving module '@typescript/lib-decorators' from '/.src/__lib_node_modules_lookup_lib.decorators.d.ts__.ts'. ========",
"Explicitly specified module resolution kind: 'Node10'.",
"Loading module '@typescript/lib-decorators' from 'node_modules' folder, target file types: TypeScript, Declaration.",
"Searching all ancestor node_modules directories for preferred extensions: TypeScript, Declaration.",
"Directory '/.src/node_modules' does not exist, skipping all lookups in it.",
"Scoped package detected, looking in 'typescript__lib-decorators'",
"Directory '/node_modules/@types' does not exist, skipping all lookups in it.",
"Scoped package detected, looking in 'typescript__lib-decorators'",
"Loading module '@typescript/lib-decorators' from 'node_modules' folder, target file types: JavaScript.",
"Searching all ancestor node_modules directories for fallback extensions: JavaScript.",
"Directory '/.src/node_modules' does not exist, skipping all lookups in it.",
"======== Module name '@typescript/lib-decorators' was not resolved. ========",
"======== Resolving module '@typescript/lib-decorators/legacy' from '/.src/__lib_node_modules_lookup_lib.decorators.legacy.d.ts__.ts'. ========",
"Explicitly specified module resolution kind: 'Node10'.",
"Loading module '@typescript/lib-decorators/legacy' from 'node_modules' folder, target file types: TypeScript, Declaration.",
"Searching all ancestor node_modules directories for preferred extensions: TypeScript, Declaration.",
"Directory '/.src/node_modules' does not exist, skipping all lookups in it.",
"Scoped package detected, looking in 'typescript__lib-decorators/legacy'",
"Directory '/node_modules/@types' does not exist, skipping all lookups in it.",
"Scoped package detected, looking in 'typescript__lib-decorators/legacy'",
"Loading module '@typescript/lib-decorators/legacy' from 'node_modules' folder, target file types: JavaScript.",
"Searching all ancestor node_modules directories for fallback extensions: JavaScript.",
"Directory '/.src/node_modules' does not exist, skipping all lookups in it.",
"======== Module name '@typescript/lib-decorators/legacy' was not resolved. ========",
"======== Resolving module '@typescript/lib-dom' from '/.src/__lib_node_modules_lookup_lib.dom.d.ts__.ts'. ========",
"Explicitly specified module resolution kind: 'Node10'.",
"Loading module '@typescript/lib-dom' from 'node_modules' folder, target file types: TypeScript, Declaration.",
"Searching all ancestor node_modules directories for preferred extensions: TypeScript, Declaration.",
"Directory '/.src/node_modules' does not exist, skipping all lookups in it.",
"Scoped package detected, looking in 'typescript__lib-dom'",
"Directory '/node_modules/@types' does not exist, skipping all lookups in it.",
"Scoped package detected, looking in 'typescript__lib-dom'",
"Loading module '@typescript/lib-dom' from 'node_modules' folder, target file types: JavaScript.",
"Searching all ancestor node_modules directories for fallback extensions: JavaScript.",
"Directory '/.src/node_modules' does not exist, skipping all lookups in it.",
"======== Module name '@typescript/lib-dom' was not resolved. ========",
"======== Resolving module '@typescript/lib-webworker/importscripts' from '/.src/__lib_node_modules_lookup_lib.webworker.importscripts.d.ts__.ts'. ========",
"Explicitly specified module resolution kind: 'Node10'.",
"Loading module '@typescript/lib-webworker/importscripts' from 'node_modules' folder, target file types: TypeScript, Declaration.",
"Searching all ancestor node_modules directories for preferred extensions: TypeScript, Declaration.",
"Directory '/.src/node_modules' does not exist, skipping all lookups in it.",
"Scoped package detected, looking in 'typescript__lib-webworker/importscripts'",
"Directory '/node_modules/@types' does not exist, skipping all lookups in it.",
"Scoped package detected, looking in 'typescript__lib-webworker/importscripts'",
"Loading module '@typescript/lib-webworker/importscripts' from 'node_modules' folder, target file types: JavaScript.",
"Searching all ancestor node_modules directories for fallback extensions: JavaScript.",
"Directory '/.src/node_modules' does not exist, skipping all lookups in it.",
"======== Module name '@typescript/lib-webworker/importscripts' was not resolved. ========",
"======== Resolving module '@typescript/lib-scripthost' from '/.src/__lib_node_modules_lookup_lib.scripthost.d.ts__.ts'. ========",
"Explicitly specified module resolution kind: 'Node10'.",
"Loading module '@typescript/lib-scripthost' from 'node_modules' folder, target file types: TypeScript, Declaration.",
"Searching all ancestor node_modules directories for preferred extensions: TypeScript, Declaration.",
"Directory '/.src/node_modules' does not exist, skipping all lookups in it.",
"Scoped package detected, looking in 'typescript__lib-scripthost'",
"Directory '/node_modules/@types' does not exist, skipping all lookups in it.",
"Scoped package detected, looking in 'typescript__lib-scripthost'",
"Loading module '@typescript/lib-scripthost' from 'node_modules' folder, target file types: JavaScript.",
"Searching all ancestor node_modules directories for fallback extensions: JavaScript.",
"Directory '/.src/node_modules' does not exist, skipping all lookups in it.",
"======== Module name '@typescript/lib-scripthost' was not resolved. ========"
]
37 changes: 37 additions & 0 deletions tests/baselines/reference/resolutionModeCache.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//// [tests/cases/conformance/moduleResolution/resolutionModeCache.ts] ////

=== /node_modules/pkg/index.d.mts ===
declare const _default: "esm";
>_default : "esm"
> : ^^^^^

export default _default;
>_default : "esm"
> : ^^^^^

=== /node_modules/pkg/index.d.ts ===
declare const _exports: "cjs";
>_exports : "cjs"
> : ^^^^^

export = _exports;
>_exports : "cjs"
> : ^^^^^

=== /index.ts ===
import type pkgRequire from "pkg" with { "resolution-mode": "require" };
>pkgRequire : any
> : ^^^

import type pkgImport from "pkg" with { "resolution-mode": "import" };
>pkgImport : any
> : ^^^

pkgRequire;
>pkgRequire : "cjs"
> : ^^^^^

pkgImport;
>pkgImport : "esm"
> : ^^^^^

Loading

0 comments on commit b78f466

Please sign in to comment.