Fix panic when moduleSuffixes resolves to declaration files#2694
Fix panic when moduleSuffixes resolves to declaration files#2694
Conversation
…action Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
|
@copilot You didn't make a test and show that it failed. This may well be the right change, but without a test we can't verify the solution. |
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
…action When using moduleSuffixes with package.json resolution, ResolvedUsingTsExtension can be true even when the module reference (e.g., "my-lib") doesn't have a TS extension. Instead of panicking, skip the error since the user didn't use a TS extension in their import statement. Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Added a failing test case in The fix in |
internal/checker/checker.go
Outdated
| tsExtension := tspath.TryExtractTSExtension(moduleReference) | ||
| if tsExtension == "" { | ||
| panic("should be able to extract TS extension from string that passes IsDeclarationFileName") | ||
| // If tsExtension is empty, it means the module reference doesn't actually contain a TS extension | ||
| // in the specifier (e.g., resolution happened via moduleSuffixes or package.json field resolution). | ||
| // In this case, skip the error since the user didn't use a TS extension in their import statement. | ||
| if tsExtension != "" { |
There was a problem hiding this comment.
@copilot This fix doesn't make any sense.
The original code says:
else if (resolvedModule.resolvedUsingTsExtension && !shouldAllowImportingTsExtension(compilerOptions, currentSourceFile.fileName)) {
const importOrExport = findAncestor(location, isImportDeclaration)?.importClause ||
findAncestor(location, or(isImportEqualsDeclaration, isExportDeclaration));
if (errorNode && !(importOrExport?.isTypeOnly || findAncestor(location, isImportTypeNode))) {
const tsExtension = Debug.checkDefined(tryExtractTSExtension(moduleReference));
error(errorNode, Diagnostics.An_import_path_can_only_end_with_a_0_extension_when_allowImportingTsExtensions_is_enabled, tsExtension);
}
}checkDefined would have thrown before, so silencing this panic is wrong. The issue must be somewhere earlier. Note the difference in the if condition; that may be a part of it, but it may be something else.
There was a problem hiding this comment.
You're right - the issue was earlier in the resolver. In loadFileNameFromPackageJSONField, the code was extracting the extension from the transformed path instead of the original candidate.
When moduleSuffixes transforms ./index.ts to ./index.d.ts:
- Before:
TryExtractTSExtension(path)returns.d.ts, then!endsWith("./index.ts", ".d.ts")=true, soresolvedUsingTsExtension = true - TypeScript:
tryExtractTSExtension(candidate)returns.ts, then!endsWith("./index.ts", ".ts")=false
Fixed in 8574259 by using candidate instead of path for extension extraction, matching TypeScript line 2110.
…e.json resolution The root cause was in loadFileNameFromPackageJSONField: when moduleSuffixes transforms "./index.ts" to "./index.d.ts", the code was extracting the extension from the transformed path (.d.ts) instead of the original candidate (.ts). This caused resolvedUsingTsExtension to be true when it shouldn't be, since the package.json value "./index.ts" ends with the original candidate extension (.ts) but not the transformed path's extension (.d.ts). TypeScript uses the original candidate for extension extraction, so we now match that behavior. Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
.dsuffixloadFileNameFromPackageJSONField, the code was extracting the extension from the transformed path instead of the original candidatecandidateinstead ofpathfor extension extraction to match TypeScript behaviorSummary
This PR fixes a panic that occurs when using
moduleSuffixeswith patterns like[".d", ""]combined with package.json resolution.Root Cause
In
loadFileNameFromPackageJSONField, whenmoduleSuffixestransforms./index.tsto./index.d.ts:.d.tsfrom the transformedpath, then compared withpackageJSONValue("./index.ts") which doesn't end with.d.ts, settingresolvedUsingTsExtension = true.tsfrom the originalcandidate, then compares withpackageJSONValuewhich ends with.ts, settingresolvedUsingTsExtension = undefined(false)Fix
Changed
extension := tspath.TryExtractTSExtension(path)toextension := tspath.TryExtractTSExtension(candidate)to match TypeScript behavior.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.