From 2eaa375c598e735bc968cd5c32ec990fc1da8050 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 30 Oct 2023 14:37:17 -0700 Subject: [PATCH 1/3] Fix callers of getConditions --- src/compiler/moduleNameResolver.ts | 2 +- src/compiler/moduleSpecifiers.ts | 3 +- src/services/stringCompletions.ts | 3 +- ...eclarationEmitBundlerConditions.errors.txt | 35 +++++++++++++++++ .../declarationEmitBundlerConditions.js | 38 +++++++++++++++++++ .../declarationEmitBundlerConditions.ts | 35 +++++++++++++++++ 6 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 tests/baselines/reference/declarationEmitBundlerConditions.errors.txt create mode 100644 tests/baselines/reference/declarationEmitBundlerConditions.js create mode 100644 tests/cases/compiler/declarationEmitBundlerConditions.ts diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 273f10f375f25..10f5406a7f861 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -745,7 +745,7 @@ function getNodeResolutionFeatures(options: CompilerOptions) { } /** - * @param overrideResolutionModeAttribute + * @param esmMode true: "import" / false: "require" / undefined: default based on settings * @internal */ export function getConditions(options: CompilerOptions, esmMode?: boolean) { diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 97e8e6b5edc81..096c47df2de37 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -992,7 +992,8 @@ function tryGetModuleNameAsNodeModule({ path, isRedirect }: ModulePath, { getCan // use the actual directory name, so don't look at `packageJsonContent.name` here. const nodeModulesDirectoryName = packageRootPath.substring(parts.topLevelPackageNameIndex + 1); const packageName = getPackageNameFromTypesPackageName(nodeModulesDirectoryName); - const conditions = getConditions(options, importMode === ModuleKind.ESNext); + const useImportCondition = importMode === ModuleKind.ESNext || (importMode !== undefined ? false : undefined); + const conditions = getConditions(options, useImportCondition); const fromExports = packageJsonContent.exports ? tryGetModuleNameFromExports(options, path, packageRootPath, packageName, packageJsonContent.exports, conditions) : undefined; diff --git a/src/services/stringCompletions.ts b/src/services/stringCompletions.ts index 47ec1126728d0..b5f2d8b19124a 100644 --- a/src/services/stringCompletions.ts +++ b/src/services/stringCompletions.ts @@ -952,7 +952,8 @@ function getCompletionEntriesForNonRelativeModules( } const keys = getOwnKeys(exports); const fragmentSubpath = components.join("/") + (components.length && hasTrailingDirectorySeparator(fragment) ? "/" : ""); - const conditions = getConditions(compilerOptions, mode === ModuleKind.ESNext); + const useImportCondition = mode === ModuleKind.ESNext || (mode !== undefined ? false : undefined); + const conditions = getConditions(compilerOptions, useImportCondition); addCompletionEntriesFromPathsOrExports( result, fragmentSubpath, diff --git a/tests/baselines/reference/declarationEmitBundlerConditions.errors.txt b/tests/baselines/reference/declarationEmitBundlerConditions.errors.txt new file mode 100644 index 0000000000000..51403f7c7e51b --- /dev/null +++ b/tests/baselines/reference/declarationEmitBundlerConditions.errors.txt @@ -0,0 +1,35 @@ +/index.ts(2,14): error TS2742: The inferred type of 'c' cannot be named without a reference to './node_modules/pkg'. This is likely not portable. A type annotation is necessary. + + +==== /node_modules/pkg/package.json (0 errors) ==== + { + "name": "pkg", + "type": "module", + "exports": { + ".": { + "import": "./index.js", + "require": "./index.cjs" + } + } + } + +==== /node_modules/pkg/index.d.ts (0 errors) ==== + export declare class C { + private p; + } + +==== /node_modules/pkg/index.d.cts (0 errors) ==== + export {}; + +==== /makeC.ts (0 errors) ==== + import { C } from "pkg"; + export function makeC() { + return new C(); + } + +==== /index.ts (1 errors) ==== + import { makeC } from "./makeC"; + export const c = makeC(); + ~ +!!! error TS2742: The inferred type of 'c' cannot be named without a reference to './node_modules/pkg'. This is likely not portable. A type annotation is necessary. + \ No newline at end of file diff --git a/tests/baselines/reference/declarationEmitBundlerConditions.js b/tests/baselines/reference/declarationEmitBundlerConditions.js new file mode 100644 index 0000000000000..8af3cabe7489e --- /dev/null +++ b/tests/baselines/reference/declarationEmitBundlerConditions.js @@ -0,0 +1,38 @@ +//// [tests/cases/compiler/declarationEmitBundlerConditions.ts] //// + +//// [package.json] +{ + "name": "pkg", + "type": "module", + "exports": { + ".": { + "import": "./index.js", + "require": "./index.cjs" + } + } +} + +//// [index.d.ts] +export declare class C { + private p; +} + +//// [index.d.cts] +export {}; + +//// [makeC.ts] +import { C } from "pkg"; +export function makeC() { + return new C(); +} + +//// [index.ts] +import { makeC } from "./makeC"; +export const c = makeC(); + + + + +//// [makeC.d.ts] +import { C } from "pkg"; +export declare function makeC(): C; diff --git a/tests/cases/compiler/declarationEmitBundlerConditions.ts b/tests/cases/compiler/declarationEmitBundlerConditions.ts new file mode 100644 index 0000000000000..2cfcdbf2f862b --- /dev/null +++ b/tests/cases/compiler/declarationEmitBundlerConditions.ts @@ -0,0 +1,35 @@ +// @module: esnext +// @moduleResolution: bundler +// @declaration: true +// @emitDeclarationOnly: true +// @noTypesAndSymbols: true + +// @Filename: /node_modules/pkg/package.json +{ + "name": "pkg", + "type": "module", + "exports": { + ".": { + "import": "./index.js", + "require": "./index.cjs" + } + } +} + +// @Filename: /node_modules/pkg/index.d.ts +export declare class C { + private p; +} + +// @Filename: /node_modules/pkg/index.d.cts +export {}; + +// @Filename: /makeC.ts +import { C } from "pkg"; +export function makeC() { + return new C(); +} + +// @Filename: /index.ts +import { makeC } from "./makeC"; +export const c = makeC(); From 25aaa2aa4484ba4b0fe2d311d462e90b62a57a01 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 30 Oct 2023 14:39:26 -0700 Subject: [PATCH 2/3] Update test baselines --- ...eclarationEmitBundlerConditions.errors.txt | 35 ------------------- .../declarationEmitBundlerConditions.js | 2 ++ 2 files changed, 2 insertions(+), 35 deletions(-) delete mode 100644 tests/baselines/reference/declarationEmitBundlerConditions.errors.txt diff --git a/tests/baselines/reference/declarationEmitBundlerConditions.errors.txt b/tests/baselines/reference/declarationEmitBundlerConditions.errors.txt deleted file mode 100644 index 51403f7c7e51b..0000000000000 --- a/tests/baselines/reference/declarationEmitBundlerConditions.errors.txt +++ /dev/null @@ -1,35 +0,0 @@ -/index.ts(2,14): error TS2742: The inferred type of 'c' cannot be named without a reference to './node_modules/pkg'. This is likely not portable. A type annotation is necessary. - - -==== /node_modules/pkg/package.json (0 errors) ==== - { - "name": "pkg", - "type": "module", - "exports": { - ".": { - "import": "./index.js", - "require": "./index.cjs" - } - } - } - -==== /node_modules/pkg/index.d.ts (0 errors) ==== - export declare class C { - private p; - } - -==== /node_modules/pkg/index.d.cts (0 errors) ==== - export {}; - -==== /makeC.ts (0 errors) ==== - import { C } from "pkg"; - export function makeC() { - return new C(); - } - -==== /index.ts (1 errors) ==== - import { makeC } from "./makeC"; - export const c = makeC(); - ~ -!!! error TS2742: The inferred type of 'c' cannot be named without a reference to './node_modules/pkg'. This is likely not portable. A type annotation is necessary. - \ No newline at end of file diff --git a/tests/baselines/reference/declarationEmitBundlerConditions.js b/tests/baselines/reference/declarationEmitBundlerConditions.js index 8af3cabe7489e..89f37accc091f 100644 --- a/tests/baselines/reference/declarationEmitBundlerConditions.js +++ b/tests/baselines/reference/declarationEmitBundlerConditions.js @@ -36,3 +36,5 @@ export const c = makeC(); //// [makeC.d.ts] import { C } from "pkg"; export declare function makeC(): C; +//// [index.d.ts] +export declare const c: import("pkg").C; From 36e2f80c472d43d0d4e1d1b5b0f3195a0b649ea0 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 30 Oct 2023 15:03:15 -0700 Subject: [PATCH 3/3] Clean up getConditions API --- src/compiler/moduleNameResolver.ts | 25 ++++++++++--------------- src/compiler/moduleSpecifiers.ts | 3 +-- src/services/stringCompletions.ts | 4 +--- 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 10f5406a7f861..9dfb79bda1ee4 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -585,10 +585,8 @@ export function resolveTypeReferenceDirective(typeReferenceDirectiveName: string if (resolutionMode === ModuleKind.ESNext && (ModuleResolutionKind.Node16 <= moduleResolution && moduleResolution <= ModuleResolutionKind.NodeNext)) { features |= NodeResolutionFeatures.EsmMode; } - // true: "import" / false: "require" / undefined: default based on settings - const useImportCondition = resolutionMode === ModuleKind.ESNext || (resolutionMode !== undefined ? false : undefined); const conditions = (features & NodeResolutionFeatures.Exports) - ? getConditions(options, useImportCondition) + ? getConditions(options, resolutionMode) : []; const diagnostics: Diagnostic[] = []; const moduleResolutionState: ModuleResolutionState = { @@ -744,16 +742,13 @@ function getNodeResolutionFeatures(options: CompilerOptions) { return features; } -/** - * @param esmMode true: "import" / false: "require" / undefined: default based on settings - * @internal - */ -export function getConditions(options: CompilerOptions, esmMode?: boolean) { +/** @internal */ +export function getConditions(options: CompilerOptions, resolutionMode?: ResolutionMode) { const moduleResolution = getEmitModuleResolutionKind(options); - if (esmMode === undefined) { + if (resolutionMode === undefined) { if (moduleResolution === ModuleResolutionKind.Bundler) { // bundler always uses `import` unless explicitly overridden - esmMode ??= moduleResolution === ModuleResolutionKind.Bundler; + resolutionMode = ModuleKind.ESNext; } else if (moduleResolution === ModuleResolutionKind.Node10) { // node10 does not support package.json imports/exports without @@ -764,7 +759,7 @@ export function getConditions(options: CompilerOptions, esmMode?: boolean) { } // conditions are only used by the node16/nodenext/bundler resolvers - there's no priority order in the list, // it's essentially a set (priority is determined by object insertion order in the object we look at). - const conditions = esmMode + const conditions = resolutionMode === ModuleKind.ESNext ? ["import"] : ["require"]; if (!options.noDtsResolution) { @@ -1439,13 +1434,13 @@ export function resolveModuleName(moduleName: string, containingFile: string, co result = nodeNextModuleNameResolver(moduleName, containingFile, compilerOptions, host, cache, redirectedReference, resolutionMode); break; case ModuleResolutionKind.Node10: - result = nodeModuleNameResolver(moduleName, containingFile, compilerOptions, host, cache, redirectedReference, resolutionMode ? getConditions(compilerOptions, resolutionMode === ModuleKind.ESNext) : undefined); + result = nodeModuleNameResolver(moduleName, containingFile, compilerOptions, host, cache, redirectedReference, resolutionMode ? getConditions(compilerOptions, resolutionMode) : undefined); break; case ModuleResolutionKind.Classic: result = classicNameResolver(moduleName, containingFile, compilerOptions, host, cache, redirectedReference); break; case ModuleResolutionKind.Bundler: - result = bundlerModuleNameResolver(moduleName, containingFile, compilerOptions, host, cache, redirectedReference, resolutionMode ? getConditions(compilerOptions, resolutionMode === ModuleKind.ESNext) : undefined); + result = bundlerModuleNameResolver(moduleName, containingFile, compilerOptions, host, cache, redirectedReference, resolutionMode ? getConditions(compilerOptions, resolutionMode) : undefined); break; default: return Debug.fail(`Unexpected moduleResolution: ${moduleResolution}`); @@ -1813,7 +1808,7 @@ function nodeModuleNameResolverWorker( compilerOptions, moduleResolution === ModuleResolutionKind.Bundler || moduleResolution === ModuleResolutionKind.Node10 ? undefined - : !!(features & NodeResolutionFeatures.EsmMode), + : (features & NodeResolutionFeatures.EsmMode) ? ModuleKind.ESNext : ModuleKind.CommonJS, ); const diagnostics: Diagnostic[] = []; @@ -2225,7 +2220,7 @@ export function getEntrypointsFromPackageJsonInfo( if (features & NodeResolutionFeatures.Exports && packageJsonInfo.contents.packageJsonContent.exports) { const conditionSets = deduplicate( - [getConditions(options, /*esmMode*/ true), getConditions(options, /*esmMode*/ false)], + [getConditions(options, ModuleKind.ESNext), getConditions(options, ModuleKind.CommonJS)], arrayIsEqualTo, ); for (const conditions of conditionSets) { diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 096c47df2de37..a59fc09a674ef 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -992,8 +992,7 @@ function tryGetModuleNameAsNodeModule({ path, isRedirect }: ModulePath, { getCan // use the actual directory name, so don't look at `packageJsonContent.name` here. const nodeModulesDirectoryName = packageRootPath.substring(parts.topLevelPackageNameIndex + 1); const packageName = getPackageNameFromTypesPackageName(nodeModulesDirectoryName); - const useImportCondition = importMode === ModuleKind.ESNext || (importMode !== undefined ? false : undefined); - const conditions = getConditions(options, useImportCondition); + const conditions = getConditions(options, importMode); const fromExports = packageJsonContent.exports ? tryGetModuleNameFromExports(options, path, packageRootPath, packageName, packageJsonContent.exports, conditions) : undefined; diff --git a/src/services/stringCompletions.ts b/src/services/stringCompletions.ts index b5f2d8b19124a..b5c28a312d9ec 100644 --- a/src/services/stringCompletions.ts +++ b/src/services/stringCompletions.ts @@ -96,7 +96,6 @@ import { LiteralTypeNode, mapDefined, MapLike, - ModuleKind, moduleResolutionUsesNodeModules, ModuleSpecifierEnding, moduleSpecifiers, @@ -952,8 +951,7 @@ function getCompletionEntriesForNonRelativeModules( } const keys = getOwnKeys(exports); const fragmentSubpath = components.join("/") + (components.length && hasTrailingDirectorySeparator(fragment) ? "/" : ""); - const useImportCondition = mode === ModuleKind.ESNext || (mode !== undefined ? false : undefined); - const conditions = getConditions(compilerOptions, useImportCondition); + const conditions = getConditions(compilerOptions, mode); addCompletionEntriesFromPathsOrExports( result, fragmentSubpath,