Skip to content

Fix declaration emit in --moduleResolution bundler using wrong exports conditions #56265

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 10 additions & 15 deletions src/compiler/moduleNameResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -744,16 +742,13 @@ function getNodeResolutionFeatures(options: CompilerOptions) {
return features;
}

/**
* @param overrideResolutionModeAttribute
* @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
Expand All @@ -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) {
Expand Down Expand Up @@ -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}`);
Expand Down Expand Up @@ -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[] = [];
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/moduleSpecifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +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 conditions = getConditions(options, importMode === ModuleKind.ESNext);
const conditions = getConditions(options, importMode);
const fromExports = packageJsonContent.exports
? tryGetModuleNameFromExports(options, path, packageRootPath, packageName, packageJsonContent.exports, conditions)
: undefined;
Expand Down
3 changes: 1 addition & 2 deletions src/services/stringCompletions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ import {
LiteralTypeNode,
mapDefined,
MapLike,
ModuleKind,
moduleResolutionUsesNodeModules,
ModuleSpecifierEnding,
moduleSpecifiers,
Expand Down Expand Up @@ -952,7 +951,7 @@ function getCompletionEntriesForNonRelativeModules(
}
const keys = getOwnKeys(exports);
const fragmentSubpath = components.join("/") + (components.length && hasTrailingDirectorySeparator(fragment) ? "/" : "");
const conditions = getConditions(compilerOptions, mode === ModuleKind.ESNext);
const conditions = getConditions(compilerOptions, mode);
addCompletionEntriesFromPathsOrExports(
result,
fragmentSubpath,
Expand Down
40 changes: 40 additions & 0 deletions tests/baselines/reference/declarationEmitBundlerConditions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//// [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;
//// [index.d.ts]
export declare const c: import("pkg").C;
35 changes: 35 additions & 0 deletions tests/cases/compiler/declarationEmitBundlerConditions.ts
Original file line number Diff line number Diff line change
@@ -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();