Skip to content

Support resolution-mode overrides in type-only constructs in all moduleResolution modes #55725

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.

Sign up for GitHub

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 10 commits into from
Sep 26, 2023
Merged

Support resolution-mode overrides in type-only constructs in all moduleResolution modes #55725

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9bcb0a9
Support resolution-mode overrides in type-only constructs in all modu…
andrewbranch Sep 12, 2023
812c1e9
Ensure imports are needed in declaration emit for tests
andrewbranch Sep 12, 2023
990af58
Make bundlerModuleNameResolver signature change internal
andrewbranch Sep 12, 2023
53adb19
Make worker functions take conditions instead of resolutionModeOverride
andrewbranch Sep 19, 2023
94b33be
Fix type error
andrewbranch Sep 19, 2023
a4eb335
Update old baseline
andrewbranch Sep 19, 2023
618ca6a
Remove useless assertion
andrewbranch Sep 19, 2023
6feb82a
Make parameter non-optional
andrewbranch Sep 19, 2023
25717bc
Merge branch 'main' into feature/55579
andrewbranch Sep 26, 2023
f989f5a
Fix merge conflict resolution error
andrewbranch Sep 26, 2023
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
6 changes: 0 additions & 6 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39662,9 +39662,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (!isNightly()) {
grammarErrorOnNode(node.assertions.assertClause, Diagnostics.resolution_mode_assertions_are_unstable_Use_nightly_TypeScript_to_silence_this_error_Try_updating_with_npm_install_D_typescript_next);
}
if (getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.Node16 && getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.NodeNext) {
grammarErrorOnNode(node.assertions.assertClause, Diagnostics.resolution_mode_assertions_are_only_supported_when_moduleResolution_is_node16_or_nodenext);
}
}
}

Expand Down Expand Up @@ -45104,9 +45101,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
grammarErrorOnNode(declaration.assertClause, Diagnostics.resolution_mode_assertions_are_unstable_Use_nightly_TypeScript_to_silence_this_error_Try_updating_with_npm_install_D_typescript_next);
}

if (getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.Node16 && getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.NodeNext) {
return grammarErrorOnNode(declaration.assertClause, Diagnostics.resolution_mode_assertions_are_only_supported_when_moduleResolution_is_node16_or_nodenext);
}
return; // Other grammar checks do not apply to type-only imports with resolution mode assertions
}

Expand Down
4 changes: 0 additions & 4 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1480,10 +1480,6 @@
"category": "Error",
"code": 1451
},
"'resolution-mode' assertions are only supported when `moduleResolution` is `node16` or `nodenext`.": {
"category": "Error",
"code": 1452
},
"`resolution-mode` should be either `require` or `import`.": {
"category": "Error",
"code": 1453
Expand Down
73 changes: 51 additions & 22 deletions src/compiler/moduleNameResolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ export interface ModuleResolutionState {
reportDiagnostic: DiagnosticReporter;
isConfigLookup: boolean;
candidateIsFromPackageJsonField: boolean;
overrideResolutionMode: ResolutionMode;
}

/** Just the fields that we use for module resolution.
Expand Down Expand Up @@ -558,18 +559,21 @@ export function resolveTypeReferenceDirective(typeReferenceDirectiveName: string

const failedLookupLocations: string[] = [];
const affectingLocations: string[] = [];
// Allow type reference directives to opt into `exports` resolution in any resolution mode
// when a `resolution-mode` override is present.
let features = getNodeResolutionFeatures(options);
// Unlike `import` statements, whose mode-calculating APIs are all guaranteed to return `undefined` if we're in an un-mode-ed module resolution
// setting, type references will return their target mode regardless of options because of how the parser works, so we guard against the mode being

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

// set in a non-modal module resolution setting here. Do note that our behavior is not particularly well defined when these mode-overriding imports
// are present in a non-modal project; while in theory we'd like to either ignore the mode or provide faithful modern resolution, depending on what we feel is best,
// in practice, not every cache has the options available to intelligently make the choice to ignore the mode request, and it's unclear how modern "faithful modern
// resolution" should be (`node16`? `nodenext`?). As such, witnessing a mode-overriding triple-slash reference in a non-modal module resolution
// context should _probably_ be an error - and that should likely be handled by the `Program` (which is what we do).
if (resolutionMode === ModuleKind.ESNext && (getEmitModuleResolutionKind(options) === ModuleResolutionKind.Node16 || getEmitModuleResolutionKind(options) === ModuleResolutionKind.NodeNext)) {
if (resolutionMode !== undefined) {
features |= NodeResolutionFeatures.ResolutionModeAttributeFeatures;
}
const moduleResolution = getEmitModuleResolutionKind(options);
if (resolutionMode === ModuleKind.ESNext && (ModuleResolutionKind.Node16 <= moduleResolution && moduleResolution <= ModuleResolutionKind.NodeNext)) {
features |= NodeResolutionFeatures.EsmMode;
}
const conditions = features & NodeResolutionFeatures.Exports ? getConditions(options, !!(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)
: [];
const diagnostics: Diagnostic[] = [];
const moduleResolutionState: ModuleResolutionState = {
compilerOptions: options,
Expand All @@ -584,6 +588,7 @@ export function resolveTypeReferenceDirective(typeReferenceDirectiveName: string
reportDiagnostic: diag => void diagnostics.push(diag),
isConfigLookup: false,
candidateIsFromPackageJsonField: false,
overrideResolutionMode: resolutionMode,
};
let resolved = primaryLookup();
let primary = true;
Expand Down Expand Up @@ -728,7 +733,7 @@ function getNodeResolutionFeatures(options: CompilerOptions) {
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 || getEmitModuleResolutionKind(options) === ModuleResolutionKind.Bundler
const conditions = esmMode || (esmMode !== false && getEmitModuleResolutionKind(options) === ModuleResolutionKind.Bundler)
? ["import"]
: ["require"];
if (!options.noDtsResolution) {
Expand Down Expand Up @@ -1411,13 +1416,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);
result = nodeModuleNameResolver(moduleName, containingFile, compilerOptions, host, cache, redirectedReference, resolutionMode);
break;
case ModuleResolutionKind.Classic:
result = classicNameResolver(moduleName, containingFile, compilerOptions, host, cache, redirectedReference);
break;
case ModuleResolutionKind.Bundler:
result = bundlerModuleNameResolver(moduleName, containingFile, compilerOptions, host, cache, redirectedReference);
result = bundlerModuleNameResolver(moduleName, containingFile, compilerOptions, host, cache, redirectedReference, resolutionMode);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In node16/nodenext, this resolutionMode is always set. In bundler and node10, it’s only set when the syntax has an explicit override.

It feels like it would be much nicer to have access to the syntax that triggered the request, or at least a serialization of all the import attributes, as part of this API. I can imagine other resolution-affecting options in import attributes in the future. But this change was possible without an API change. Open to doing something different though.

break;
default:
return Debug.fail(`Unexpected moduleResolution: ${moduleResolution}`);
Expand Down Expand Up @@ -1670,6 +1675,10 @@ export enum NodeResolutionFeatures {

BundlerDefault = Imports | SelfName | Exports | ExportsPatternTrailers,

// features required to resolve `resolution-mode` overrides in
// type reference directives, import() types, and type-only imports
ResolutionModeAttributeFeatures = Imports | SelfName | Exports | ExportsPatternTrailers,

EsmMode = 1 << 5,
}

Expand Down Expand Up @@ -1708,7 +1717,7 @@ function nodeNextModuleNameResolverWorker(features: NodeResolutionFeatures, modu
if (getResolveJsonModule(compilerOptions)) {
extensions |= Extensions.Json;
}
return nodeModuleNameResolverWorker(features | esmMode, moduleName, containingDirectory, compilerOptions, host, cache, extensions, /*isConfigLookup*/ false, redirectedReference);
return nodeModuleNameResolverWorker(features | esmMode, moduleName, containingDirectory, compilerOptions, host, cache, extensions, /*isConfigLookup*/ false, redirectedReference, /*overrideResolutionMode*/ undefined);
}

function tryResolveJSModuleWorker(moduleName: string, initialDir: string, host: ModuleResolutionHost): ResolvedModuleWithFailedLookupLocations {
Expand All @@ -1722,21 +1731,22 @@ function tryResolveJSModuleWorker(moduleName: string, initialDir: string, host:
Extensions.JavaScript,
/*isConfigLookup*/ false,
/*redirectedReference*/ undefined,
/*overrideResolutionMode*/ undefined,
);
}

export function bundlerModuleNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache?: ModuleResolutionCache, redirectedReference?: ResolvedProjectReference): ResolvedModuleWithFailedLookupLocations {
export function bundlerModuleNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache?: ModuleResolutionCache, redirectedReference?: ResolvedProjectReference, resolutionMode?: ResolutionMode): ResolvedModuleWithFailedLookupLocations {
const containingDirectory = getDirectoryPath(containingFile);
let extensions = compilerOptions.noDtsResolution ? Extensions.ImplementationFiles : Extensions.TypeScript | Extensions.JavaScript | Extensions.Declaration;
if (getResolveJsonModule(compilerOptions)) {
extensions |= Extensions.Json;
}
return nodeModuleNameResolverWorker(getNodeResolutionFeatures(compilerOptions), moduleName, containingDirectory, compilerOptions, host, cache, extensions, /*isConfigLookup*/ false, redirectedReference);
return nodeModuleNameResolverWorker(getNodeResolutionFeatures(compilerOptions), moduleName, containingDirectory, compilerOptions, host, cache, extensions, /*isConfigLookup*/ false, redirectedReference, resolutionMode);
}

export function nodeModuleNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache?: ModuleResolutionCache, redirectedReference?: ResolvedProjectReference): ResolvedModuleWithFailedLookupLocations;
/** @internal */ export function nodeModuleNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache?: ModuleResolutionCache, redirectedReference?: ResolvedProjectReference, lookupConfig?: boolean): ResolvedModuleWithFailedLookupLocations; // eslint-disable-line @typescript-eslint/unified-signatures
export function nodeModuleNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache?: ModuleResolutionCache, redirectedReference?: ResolvedProjectReference, isConfigLookup?: boolean): ResolvedModuleWithFailedLookupLocations {
/** @internal */ export function nodeModuleNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache?: ModuleResolutionCache, redirectedReference?: ResolvedProjectReference, resolutionMode?: ResolutionMode, lookupConfig?: boolean): ResolvedModuleWithFailedLookupLocations; // eslint-disable-line @typescript-eslint/unified-signatures
export function nodeModuleNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache?: ModuleResolutionCache, redirectedReference?: ResolvedProjectReference, resolutionMode?: ResolutionMode, isConfigLookup?: boolean): ResolvedModuleWithFailedLookupLocations {
let extensions;
if (isConfigLookup) {
extensions = Extensions.Json;
Expand All @@ -1750,20 +1760,35 @@ export function nodeModuleNameResolver(moduleName: string, containingFile: strin
? Extensions.TypeScript | Extensions.JavaScript | Extensions.Declaration | Extensions.Json
: Extensions.TypeScript | Extensions.JavaScript | Extensions.Declaration;
}
return nodeModuleNameResolverWorker(NodeResolutionFeatures.None, moduleName, getDirectoryPath(containingFile), compilerOptions, host, cache, extensions, !!isConfigLookup, redirectedReference);
return nodeModuleNameResolverWorker(NodeResolutionFeatures.None, moduleName, getDirectoryPath(containingFile), compilerOptions, host, cache, extensions, !!isConfigLookup, redirectedReference, resolutionMode);
}

/** @internal */
export function nodeNextJsonConfigResolver(moduleName: string, containingFile: string, host: ModuleResolutionHost): ResolvedModuleWithFailedLookupLocations {
return nodeModuleNameResolverWorker(NodeResolutionFeatures.NodeNextDefault, moduleName, getDirectoryPath(containingFile), { moduleResolution: ModuleResolutionKind.NodeNext }, host, /*cache*/ undefined, Extensions.Json, /*isConfigLookup*/ true, /*redirectedReference*/ undefined);
return nodeModuleNameResolverWorker(NodeResolutionFeatures.NodeNextDefault, moduleName, getDirectoryPath(containingFile), { moduleResolution: ModuleResolutionKind.NodeNext }, host, /*cache*/ undefined, Extensions.Json, /*isConfigLookup*/ true, /*redirectedReference*/ undefined, /*overrideResolutionMode*/ undefined);
}

function nodeModuleNameResolverWorker(features: NodeResolutionFeatures, moduleName: string, containingDirectory: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache: ModuleResolutionCache | undefined, extensions: Extensions, isConfigLookup: boolean, redirectedReference: ResolvedProjectReference | undefined): ResolvedModuleWithFailedLookupLocations {
function nodeModuleNameResolverWorker(
features: NodeResolutionFeatures,
moduleName: string,
containingDirectory: string,
compilerOptions: CompilerOptions,
host: ModuleResolutionHost,
cache: ModuleResolutionCache | undefined,
extensions: Extensions,
isConfigLookup: boolean,
redirectedReference: ResolvedProjectReference | undefined,
overrideResolutionMode: ResolutionMode,
): ResolvedModuleWithFailedLookupLocations {
const traceEnabled = isTraceEnabled(compilerOptions, host);

const failedLookupLocations: string[] = [];
const affectingLocations: string[] = [];
const conditions = getConditions(compilerOptions, !!(features & NodeResolutionFeatures.EsmMode));
if (overrideResolutionMode) {
features |= NodeResolutionFeatures.ResolutionModeAttributeFeatures;
}
const useImportCondition = !!(features & NodeResolutionFeatures.EsmMode) || (overrideResolutionMode ? overrideResolutionMode === ModuleKind.ESNext : undefined);
const conditions = getConditions(compilerOptions, useImportCondition);

const diagnostics: Diagnostic[] = [];
const state: ModuleResolutionState = {
Expand All @@ -1779,6 +1804,7 @@ function nodeModuleNameResolverWorker(features: NodeResolutionFeatures, moduleNa
reportDiagnostic: diag => void diagnostics.push(diag),
isConfigLookup,
candidateIsFromPackageJsonField: false,
overrideResolutionMode,
};

if (traceEnabled && moduleResolutionSupportsPackageJsonExportsAndImports(getEmitModuleResolutionKind(compilerOptions))) {
Expand Down Expand Up @@ -2288,6 +2314,7 @@ export function getTemporaryModuleResolutionState(packageJsonInfoCache: PackageJ
reportDiagnostic: noop,
isConfigLookup: false,
candidateIsFromPackageJsonField: false,
overrideResolutionMode: undefined,
};
}

Expand Down Expand Up @@ -2657,7 +2684,7 @@ function getLoadModuleFromTargetImportOrExport(extensions: Extensions, state: Mo
const combinedLookup = pattern ? target.replace(/\*/g, subpath) : target + subpath;
traceIfEnabled(state, Diagnostics.Using_0_subpath_1_with_target_2, "imports", key, combinedLookup);
traceIfEnabled(state, Diagnostics.Resolving_module_0_from_1, combinedLookup, scope.packageDirectory + "/");
const result = nodeModuleNameResolverWorker(state.features, combinedLookup, scope.packageDirectory + "/", state.compilerOptions, state.host, cache, extensions, /*isConfigLookup*/ false, redirectedReference);
const result = nodeModuleNameResolverWorker(state.features, combinedLookup, scope.packageDirectory + "/", state.compilerOptions, state.host, cache, extensions, /*isConfigLookup*/ false, redirectedReference, state.overrideResolutionMode);
return toSearchResult(
result.resolvedModule ? {
path: result.resolvedModule.resolvedFileName,
Expand Down Expand Up @@ -3140,6 +3167,7 @@ export function classicNameResolver(moduleName: string, containingFile: string,
reportDiagnostic: diag => void diagnostics.push(diag),
isConfigLookup: false,
candidateIsFromPackageJsonField: false,
overrideResolutionMode: undefined,
};

const resolved = tryResolve(Extensions.TypeScript | Extensions.Declaration) ||
Expand Down Expand Up @@ -3239,6 +3267,7 @@ export function loadModuleFromGlobalCache(moduleName: string, projectName: strin
reportDiagnostic: diag => void diagnostics.push(diag),
isConfigLookup: false,
candidateIsFromPackageJsonField: false,
overrideResolutionMode: undefined,
};
const resolved = loadModuleFromImmediateNodeModulesDirectory(Extensions.Declaration, moduleName, globalCache, state, /*typesScopeOnly*/ false, /*cache*/ undefined, /*redirectedReference*/ undefined);
return createResolvedModuleWithFailedLookupLocations(
Expand Down
Loading