Skip to content
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

Add additional diagnostic for packages that only resolve under non-exports-respecting modes #52173

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
43 changes: 26 additions & 17 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,7 @@ import {
nodeIsPresent,
nodeIsSynthesized,
NodeLinks,
nodeModulesPathPart,
nodeStartsNewLexicalEnvironment,
NodeWithTypeArguments,
NonNullChain,
Expand Down Expand Up @@ -4767,7 +4768,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {

if (sourceFile.symbol) {
if (resolvedModule.isExternalLibraryImport && !resolutionExtensionIsTSOrJson(resolvedModule.extension)) {
errorOnImplicitAnyModule(/*isError*/ false, errorNode, resolvedModule, moduleReference);
errorOnImplicitAnyModule(/*isError*/ false, errorNode, currentSourceFile, mode, resolvedModule, moduleReference);
}
if (moduleResolutionKind === ModuleResolutionKind.Node16 || moduleResolutionKind === ModuleResolutionKind.NodeNext) {
const isSyncImport = (currentSourceFile.impliedNodeFormat === ModuleKind.CommonJS && !findAncestor(location, isImportCall)) || !!findAncestor(location, isImportEqualsDeclaration);
Expand Down Expand Up @@ -4855,7 +4856,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
error(errorNode, diag, moduleReference, resolvedModule!.resolvedFileName);
}
else {
errorOnImplicitAnyModule(/*isError*/ noImplicitAny && !!moduleNotFoundError, errorNode, resolvedModule!, moduleReference);
errorOnImplicitAnyModule(/*isError*/ noImplicitAny && !!moduleNotFoundError, errorNode, currentSourceFile, mode, resolvedModule!, moduleReference);
}
// Failed imports and untyped modules are both treated in an untyped manner; only difference is whether we give a diagnostic first.
return undefined;
Expand Down Expand Up @@ -4925,25 +4926,33 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
}

function errorOnImplicitAnyModule(isError: boolean, errorNode: Node, { packageId, resolvedFileName }: ResolvedModuleFull, moduleReference: string): void {
const errorInfo = !isExternalModuleNameRelative(moduleReference) && packageId
? typesPackageExists(packageId.name)
function errorOnImplicitAnyModule(isError: boolean, errorNode: Node, sourceFile: SourceFile, mode: ResolutionMode, { packageId, resolvedFileName }: ResolvedModuleFull, moduleReference: string): void {
let errorInfo;
if (!isExternalModuleNameRelative(moduleReference) && packageId) {
const node10Result = sourceFile.resolvedModules?.get(moduleReference, mode)?.node10Result;
errorInfo = node10Result
? chainDiagnosticMessages(
/*details*/ undefined,
Diagnostics.If_the_0_package_actually_exposes_this_module_consider_sending_a_pull_request_to_amend_https_Colon_Slash_Slashgithub_com_SlashDefinitelyTyped_SlashDefinitelyTyped_Slashtree_Slashmaster_Slashtypes_Slash_1,
packageId.name, mangleScopedPackageName(packageId.name))
: packageBundlesTypes(packageId.name)
Diagnostics.There_are_types_at_0_but_this_result_could_not_be_resolved_when_respecting_package_json_exports_The_1_library_may_need_to_update_its_package_json_or_typings,
node10Result,
node10Result.indexOf(nodeModulesPathPart + "@types/") > -1 ? `@types/${mangleScopedPackageName(packageId.name)}` : packageId.name)
: typesPackageExists(packageId.name)
? chainDiagnosticMessages(
/*details*/ undefined,
Diagnostics.If_the_0_package_actually_exposes_this_module_try_adding_a_new_declaration_d_ts_file_containing_declare_module_1,
packageId.name,
moduleReference)
: chainDiagnosticMessages(
/*details*/ undefined,
Diagnostics.Try_npm_i_save_dev_types_Slash_1_if_it_exists_or_add_a_new_declaration_d_ts_file_containing_declare_module_0,
moduleReference,
mangleScopedPackageName(packageId.name))
: undefined;
Diagnostics.If_the_0_package_actually_exposes_this_module_consider_sending_a_pull_request_to_amend_https_Colon_Slash_Slashgithub_com_SlashDefinitelyTyped_SlashDefinitelyTyped_Slashtree_Slashmaster_Slashtypes_Slash_1,
packageId.name, mangleScopedPackageName(packageId.name))
: packageBundlesTypes(packageId.name)
? chainDiagnosticMessages(
/*details*/ undefined,
Diagnostics.If_the_0_package_actually_exposes_this_module_try_adding_a_new_declaration_d_ts_file_containing_declare_module_1,
packageId.name,
moduleReference)
: chainDiagnosticMessages(
/*details*/ undefined,
Diagnostics.Try_npm_i_save_dev_types_Slash_1_if_it_exists_or_add_a_new_declaration_d_ts_file_containing_declare_module_0,
moduleReference,
mangleScopedPackageName(packageId.name));
}
errorOrSuggestion(isError, errorNode, chainDiagnosticMessages(
errorInfo,
Diagnostics.Could_not_find_a_declaration_file_for_module_0_1_implicitly_has_an_any_type,
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -5235,6 +5235,14 @@
"category": "Message",
"code": 6276
},
"Resolution of non-relative name failed; trying with modern Node resolution features disabled to see if npm library needs configuration update.": {
Copy link
Member

Choose a reason for hiding this comment

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

a non-relative name is one like "foo", not "./foo.js", right?

I guess this is output for traceResolution, so it's probably OK to be technical, but is 'non-relative' the friendliest term we have? I have to think about it a bit every time I see the term. Could be that I was ruined by reading an old, old "how Typescript implements Node resolution" document, though.

Choose a reason for hiding this comment

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

I've also found myself wondering if there's a good term for imports that are neither absolute nor relative to the containing file, but rather relative to some configured root or resolved via aliasing. Floating? Inferred? I guess "absolute" could be reused since giving full filesystem paths for imports should be vanishingly rare, but it doesn't feel accurate either.

"category": "Message",
"code": 6277
},
"There are types at '{0}', but this result could not be resolved when respecting package.json \"exports\". The '{1}' library may need to update its package.json or typings.": {
"category": "Message",
"code": 6278
},

"Enable project compilation": {
"category": "Message",
Expand Down
42 changes: 35 additions & 7 deletions src/compiler/moduleNameResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ function createResolvedModuleWithFailedLookupLocations(
failedLookupLocations: string[],
affectingLocations: string[],
diagnostics: Diagnostic[],
resultFromCache: ResolvedModuleWithFailedLookupLocations | undefined
resultFromCache: ResolvedModuleWithFailedLookupLocations | undefined,
legacyResult?: string,
): ResolvedModuleWithFailedLookupLocations {
if (resultFromCache) {
resultFromCache.failedLookupLocations = updateResolutionField(resultFromCache.failedLookupLocations, failedLookupLocations);
Expand All @@ -232,6 +233,7 @@ function createResolvedModuleWithFailedLookupLocations(
failedLookupLocations: initializeResolutionField(failedLookupLocations),
affectingLocations: initializeResolutionField(affectingLocations),
resolutionDiagnostics: initializeResolutionField(diagnostics),
node10Result: legacyResult,
};
}
function initializeResolutionField<T>(value: T[]): T[] | undefined {
Expand Down Expand Up @@ -1666,12 +1668,37 @@ function nodeModuleNameResolverWorker(features: NodeResolutionFeatures, moduleNa
const priorityExtensions = extensions & (Extensions.TypeScript | Extensions.Declaration);
const secondaryExtensions = extensions & ~(Extensions.TypeScript | Extensions.Declaration);
result =
priorityExtensions && tryResolve(priorityExtensions) ||
secondaryExtensions && tryResolve(secondaryExtensions) ||
priorityExtensions && tryResolve(priorityExtensions, state) ||
secondaryExtensions && tryResolve(secondaryExtensions, state) ||
undefined;
}
else {
result = tryResolve(extensions);
result = tryResolve(extensions, state);
}

// For non-relative names that resolved to JS but no types in modes that look up an "import" condition in package.json "exports",
// try again with "exports" disabled to try to detect if this is likely a configuration error in a dependency's package.json.
let legacyResult;
if (result?.value?.isExternalLibraryImport
&& !isConfigLookup
&& extensions & (Extensions.TypeScript | Extensions.Declaration)
&& features & NodeResolutionFeatures.Exports
&& !isExternalModuleNameRelative(moduleName)
&& !extensionIsOk(Extensions.TypeScript | Extensions.Declaration, result.value.resolved.extension)
&& conditions.indexOf("import") > -1
) {
traceIfEnabled(state, Diagnostics.Resolution_of_non_relative_name_failed_trying_with_modern_Node_resolution_features_disabled_to_see_if_npm_library_needs_configuration_update);
const diagnosticState = {
...state,
features: state.features & ~NodeResolutionFeatures.Exports,
failedLookupLocations: [],
affectingLocations: [],
reportDiagnostic: noop,
};
const diagnosticResult = tryResolve(extensions & (Extensions.TypeScript | Extensions.Declaration), diagnosticState);
if (diagnosticResult?.value?.isExternalLibraryImport) {
legacyResult = diagnosticResult.value.resolved.path;
}
}

return createResolvedModuleWithFailedLookupLocations(
Expand All @@ -1680,10 +1707,11 @@ function nodeModuleNameResolverWorker(features: NodeResolutionFeatures, moduleNa
failedLookupLocations,
affectingLocations,
diagnostics,
state.resultFromCache
state.resultFromCache,
legacyResult,
);

function tryResolve(extensions: Extensions): SearchResult<{ resolved: Resolved, isExternalLibraryImport: boolean }> {
function tryResolve(extensions: Extensions, state: ModuleResolutionState): SearchResult<{ resolved: Resolved, isExternalLibraryImport: boolean }> {
const loader: ResolutionKindSpecificLoader = (extensions, candidate, onlyRecordFailures, state) => nodeLoadModuleByRelativeName(extensions, candidate, onlyRecordFailures, state, /*considerPackageJson*/ true);
const resolved = tryLoadModuleUsingOptionalResolutionSettings(extensions, moduleName, containingDirectory, loader, state);
if (resolved) {
Expand Down Expand Up @@ -2279,7 +2307,7 @@ function resolvedIfExtensionMatches(extensions: Extensions, path: string, resolv
}

/** True if `extension` is one of the supported `extensions`. */
function extensionIsOk(extensions: Extensions, extension: Extension): boolean {
function extensionIsOk(extensions: Extensions, extension: string): boolean {
return extensions & Extensions.JavaScript && (extension === Extension.Js || extension === Extension.Jsx || extension === Extension.Mjs || extension === Extension.Cjs)
|| extensions & Extensions.TypeScript && (extension === Extension.Ts || extension === Extension.Tsx || extension === Extension.Mts || extension === Extension.Cts)
|| extensions & Extensions.Declaration && (extension === Extension.Dts || extension === Extension.Dmts || extension === Extension.Dcts)
Expand Down
6 changes: 6 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7606,6 +7606,12 @@ export interface ResolvedModuleWithFailedLookupLocations {
affectingLocations?: string[];
/** @internal */
resolutionDiagnostics?: Diagnostic[]
/**
* @internal
* Used to issue a diagnostic if typings for a non-relative import couldn't be found
* while respecting package.json `exports`, but were found when disabling `exports`.
*/
node10Result?: string;
}

export interface ResolvedTypeReferenceDirective {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ error TS6504: File '/node_modules/exports-and-types-versions/dist/foo.js' is a J
/main.cts(2,16): error TS2307: Cannot find module 'exports-and-types-versions/nope' or its corresponding type declarations.
/main.cts(5,16): error TS2307: Cannot find module 'exports-and-types-versions/versioned-nah' or its corresponding type declarations.
/main.mts(1,16): error TS7016: Could not find a declaration file for module 'exports-and-types-versions/foo'. '/node_modules/exports-and-types-versions/dist/foo.js' implicitly has an 'any' type.
If the 'exports-and-types-versions' package actually exposes this module, try adding a new declaration (.d.ts) file containing `declare module 'exports-and-types-versions/foo';`
There are types at '/node_modules/exports-and-types-versions/types/foo.d.ts', but this result could not be resolved when respecting package.json "exports". The 'exports-and-types-versions' library may need to update its package.json or typings.
/main.mts(2,16): error TS2307: Cannot find module 'exports-and-types-versions/nope' or its corresponding type declarations.
/main.mts(5,16): error TS2307: Cannot find module 'exports-and-types-versions/versioned-nah' or its corresponding type declarations.

Expand Down Expand Up @@ -79,7 +79,7 @@ error TS6504: File '/node_modules/exports-and-types-versions/dist/foo.js' is a J
import {} from "exports-and-types-versions/foo";
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS7016: Could not find a declaration file for module 'exports-and-types-versions/foo'. '/node_modules/exports-and-types-versions/dist/foo.js' implicitly has an 'any' type.
!!! error TS7016: If the 'exports-and-types-versions' package actually exposes this module, try adding a new declaration (.d.ts) file containing `declare module 'exports-and-types-versions/foo';`
!!! error TS7016: There are types at '/node_modules/exports-and-types-versions/types/foo.d.ts', but this result could not be resolved when respecting package.json "exports". The 'exports-and-types-versions' library may need to update its package.json or typings.
import {} from "exports-and-types-versions/nope";
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2307: Cannot find module 'exports-and-types-versions/nope' or its corresponding type declarations.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,16 @@
"File name '/node_modules/exports-and-types-versions/dist/foo.js' has a '.js' extension - stripping it.",
"File '/node_modules/exports-and-types-versions/dist/foo.js' exist - use it as a name resolution result.",
"Resolving real path for '/node_modules/exports-and-types-versions/dist/foo.js', result '/node_modules/exports-and-types-versions/dist/foo.js'.",
"Resolution of non-relative name failed; trying with modern Node resolution features disabled to see if npm library needs configuration update.",
"File '/package.json' does not exist according to earlier cached lookups.",
"Loading module 'exports-and-types-versions/foo' from 'node_modules' folder, target file types: TypeScript, Declaration.",
"File '/node_modules/exports-and-types-versions/package.json' exists according to earlier cached lookups.",
"'package.json' has a 'typesVersions' field with version-specific path mappings.",
"'package.json' has a 'typesVersions' entry '*' that matches compiler version '3.1.0-dev', looking for a pattern to match module name 'foo'.",
"Module name 'foo', matched pattern 'foo'.",
"Trying substitution './types/foo.d.ts', candidate module location: './types/foo.d.ts'.",
"File '/node_modules/exports-and-types-versions/types/foo.d.ts' exist - use it as a name resolution result.",
"Resolving real path for '/node_modules/exports-and-types-versions/types/foo.d.ts', result '/node_modules/exports-and-types-versions/types/foo.d.ts'.",
"======== Module name 'exports-and-types-versions/foo' was successfully resolved to '/node_modules/exports-and-types-versions/dist/foo.js' with Package ID 'exports-and-types-versions/dist/[email protected]'. ========",
"======== Resolving module 'exports-and-types-versions/nope' from '/main.mts'. ========",
"Module resolution kind is not specified, using 'Node16'.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ error TS6504: File '/node_modules/exports-and-types-versions/dist/foo.js' is a J
/main.cts(2,16): error TS2307: Cannot find module 'exports-and-types-versions/nope' or its corresponding type declarations.
/main.cts(5,16): error TS2307: Cannot find module 'exports-and-types-versions/versioned-nah' or its corresponding type declarations.
/main.mts(1,16): error TS7016: Could not find a declaration file for module 'exports-and-types-versions/foo'. '/node_modules/exports-and-types-versions/dist/foo.js' implicitly has an 'any' type.
If the 'exports-and-types-versions' package actually exposes this module, try adding a new declaration (.d.ts) file containing `declare module 'exports-and-types-versions/foo';`
There are types at '/node_modules/exports-and-types-versions/types/foo.d.ts', but this result could not be resolved when respecting package.json "exports". The 'exports-and-types-versions' library may need to update its package.json or typings.
/main.mts(2,16): error TS2307: Cannot find module 'exports-and-types-versions/nope' or its corresponding type declarations.
/main.mts(5,16): error TS2307: Cannot find module 'exports-and-types-versions/versioned-nah' or its corresponding type declarations.

Expand Down Expand Up @@ -79,7 +79,7 @@ error TS6504: File '/node_modules/exports-and-types-versions/dist/foo.js' is a J
import {} from "exports-and-types-versions/foo";
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS7016: Could not find a declaration file for module 'exports-and-types-versions/foo'. '/node_modules/exports-and-types-versions/dist/foo.js' implicitly has an 'any' type.
!!! error TS7016: If the 'exports-and-types-versions' package actually exposes this module, try adding a new declaration (.d.ts) file containing `declare module 'exports-and-types-versions/foo';`
!!! error TS7016: There are types at '/node_modules/exports-and-types-versions/types/foo.d.ts', but this result could not be resolved when respecting package.json "exports". The 'exports-and-types-versions' library may need to update its package.json or typings.
import {} from "exports-and-types-versions/nope";
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2307: Cannot find module 'exports-and-types-versions/nope' or its corresponding type declarations.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,16 @@
"File name '/node_modules/exports-and-types-versions/dist/foo.js' has a '.js' extension - stripping it.",
"File '/node_modules/exports-and-types-versions/dist/foo.js' exist - use it as a name resolution result.",
"Resolving real path for '/node_modules/exports-and-types-versions/dist/foo.js', result '/node_modules/exports-and-types-versions/dist/foo.js'.",
"Resolution of non-relative name failed; trying with modern Node resolution features disabled to see if npm library needs configuration update.",
"File '/package.json' does not exist according to earlier cached lookups.",
"Loading module 'exports-and-types-versions/foo' from 'node_modules' folder, target file types: TypeScript, Declaration.",
"File '/node_modules/exports-and-types-versions/package.json' exists according to earlier cached lookups.",
"'package.json' has a 'typesVersions' field with version-specific path mappings.",
"'package.json' has a 'typesVersions' entry '*' that matches compiler version '3.1.0-dev', looking for a pattern to match module name 'foo'.",
"Module name 'foo', matched pattern 'foo'.",
"Trying substitution './types/foo.d.ts', candidate module location: './types/foo.d.ts'.",
"File '/node_modules/exports-and-types-versions/types/foo.d.ts' exist - use it as a name resolution result.",
"Resolving real path for '/node_modules/exports-and-types-versions/types/foo.d.ts', result '/node_modules/exports-and-types-versions/types/foo.d.ts'.",
"======== Module name 'exports-and-types-versions/foo' was successfully resolved to '/node_modules/exports-and-types-versions/dist/foo.js' with Package ID 'exports-and-types-versions/dist/[email protected]'. ========",
"======== Resolving module 'exports-and-types-versions/nope' from '/main.mts'. ========",
"Module resolution kind is not specified, using 'NodeNext'.",
Expand Down
Loading