Skip to content

Consistently avoid module resolution errors when using getSymbolAtLocation #58668

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
Jun 24, 2024
Merged

Consistently avoid module resolution errors when using getSymbolAtLocation #58668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a821a2b
Consistently avoid module reoslution errors when using `getSymbolAtLo…
Andarist May 26, 2024
908ddd0
fixed missing module resolution errors
Andarist May 26, 2024
ca9be69
fixed errors on specifier-less imports
Andarist May 27, 2024
c69d9cc
fmt
Andarist May 27, 2024
60a95d8
Revert "fmt"
Andarist May 30, 2024
1c41b56
Revert "fixed errors on specifier-less imports"
Andarist May 30, 2024
a68b4cc
Revert "fixed missing module resolution errors"
Andarist May 30, 2024
039a66e
Merge remote-tracking branch 'origin/main' into fix/crash-error-on-sy…
Andarist Jun 7, 2024
64837f5
Merge remote-tracking branch 'origin/main' into fix/crash-error-on-sy…
Andarist Jun 22, 2024
88dba91
add an extra test case
Andarist Jun 22, 2024
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
30 changes: 17 additions & 13 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2783,7 +2783,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const moduleNotFoundError = !(moduleName.parent.parent.flags & NodeFlags.Ambient)
? Diagnostics.Invalid_module_name_in_augmentation_module_0_cannot_be_found
: undefined;
let mainModule = resolveExternalModuleNameWorker(moduleName, moduleName, moduleNotFoundError, /*isForAugmentation*/ true);
let mainModule = resolveExternalModuleNameWorker(moduleName, moduleName, moduleNotFoundError, /*ignoreErrors*/ false, /*isForAugmentation*/ true);
if (!mainModule) {
return;
}
Expand Down Expand Up @@ -4547,17 +4547,17 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const errorMessage = isClassic ?
Diagnostics.Cannot_find_module_0_Did_you_mean_to_set_the_moduleResolution_option_to_nodenext_or_to_add_aliases_to_the_paths_option
: Diagnostics.Cannot_find_module_0_or_its_corresponding_type_declarations;
return resolveExternalModuleNameWorker(location, moduleReferenceExpression, ignoreErrors ? undefined : errorMessage);
return resolveExternalModuleNameWorker(location, moduleReferenceExpression, ignoreErrors ? undefined : errorMessage, ignoreErrors);
}

function resolveExternalModuleNameWorker(location: Node, moduleReferenceExpression: Expression, moduleNotFoundError: DiagnosticMessage | undefined, isForAugmentation = false): Symbol | undefined {
function resolveExternalModuleNameWorker(location: Node, moduleReferenceExpression: Expression, moduleNotFoundError: DiagnosticMessage | undefined, ignoreErrors = false, isForAugmentation = false): Symbol | undefined {
return isStringLiteralLike(moduleReferenceExpression)
? resolveExternalModule(location, moduleReferenceExpression.text, moduleNotFoundError, moduleReferenceExpression, isForAugmentation)
? resolveExternalModule(location, moduleReferenceExpression.text, moduleNotFoundError, !ignoreErrors ? moduleReferenceExpression : undefined, isForAugmentation)
: undefined;
}

function resolveExternalModule(location: Node, moduleReference: string, moduleNotFoundError: DiagnosticMessage | undefined, errorNode: Node, isForAugmentation = false): Symbol | undefined {
if (startsWith(moduleReference, "@types/")) {
function resolveExternalModule(location: Node, moduleReference: string, moduleNotFoundError: DiagnosticMessage | undefined, errorNode: Node | undefined, isForAugmentation = false): Symbol | undefined {
if (errorNode && startsWith(moduleReference, "@types/")) {
const diag = Diagnostics.Cannot_import_type_declaration_files_Consider_importing_0_instead_of_1;
const withoutAtTypePrefix = removePrefix(moduleReference, "@types/");
error(errorNode, diag, withoutAtTypePrefix, moduleReference);
Expand All @@ -4581,7 +4581,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const mode = contextSpecifier && isStringLiteralLike(contextSpecifier) ? host.getModeForUsageLocation(currentSourceFile, contextSpecifier) : currentSourceFile.impliedNodeFormat;
const moduleResolutionKind = getEmitModuleResolutionKind(compilerOptions);
const resolvedModule = host.getResolvedModule(currentSourceFile, moduleReference, mode)?.resolvedModule;
const resolutionDiagnostic = resolvedModule && getResolutionDiagnostic(compilerOptions, resolvedModule, currentSourceFile);
const resolutionDiagnostic = errorNode && resolvedModule && getResolutionDiagnostic(compilerOptions, resolvedModule, currentSourceFile);
const sourceFile = resolvedModule
&& (!resolutionDiagnostic || resolutionDiagnostic === Diagnostics.Module_0_was_resolved_to_1_but_jsx_is_not_set)
&& host.getSourceFile(resolvedModule.resolvedFileName);
Expand All @@ -4594,7 +4594,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (resolvedModule.resolvedUsingTsExtension && isDeclarationFileName(moduleReference)) {
const importOrExport = findAncestor(location, isImportDeclaration)?.importClause ||
findAncestor(location, or(isImportEqualsDeclaration, isExportDeclaration));
if (importOrExport && !importOrExport.isTypeOnly || findAncestor(location, isImportCall)) {
if (errorNode && importOrExport && !importOrExport.isTypeOnly || findAncestor(location, isImportCall)) {
error(
errorNode,
Diagnostics.A_declaration_file_cannot_be_imported_without_import_type_Did_you_mean_to_import_an_implementation_file_0_instead,
Expand All @@ -4605,17 +4605,17 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
else if (resolvedModule.resolvedUsingTsExtension && !shouldAllowImportingTsExtension(compilerOptions, currentSourceFile.fileName)) {
const importOrExport = findAncestor(location, isImportDeclaration)?.importClause ||
findAncestor(location, or(isImportEqualsDeclaration, isExportDeclaration));
if (!(importOrExport?.isTypeOnly || findAncestor(location, isImportTypeNode))) {
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);
}
}

if (sourceFile.symbol) {
if (resolvedModule.isExternalLibraryImport && !resolutionExtensionIsTSOrJson(resolvedModule.extension)) {
if (errorNode && resolvedModule.isExternalLibraryImport && !resolutionExtensionIsTSOrJson(resolvedModule.extension)) {
errorOnImplicitAnyModule(/*isError*/ false, errorNode, currentSourceFile, mode, resolvedModule, moduleReference);
}
if (moduleResolutionKind === ModuleResolutionKind.Node16 || moduleResolutionKind === ModuleResolutionKind.NodeNext) {
if (errorNode && (moduleResolutionKind === ModuleResolutionKind.Node16 || moduleResolutionKind === ModuleResolutionKind.NodeNext)) {
const isSyncImport = (currentSourceFile.impliedNodeFormat === ModuleKind.CommonJS && !findAncestor(location, isImportCall)) || !!findAncestor(location, isImportEqualsDeclaration);
const overrideHost = findAncestor(location, l => isImportTypeNode(l) || isExportDeclaration(l) || isImportDeclaration(l));
// An override clause will take effect for type-only imports and import types, and allows importing the types across formats, regardless of
Expand Down Expand Up @@ -4680,7 +4680,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// merged symbol is module declaration symbol combined with all augmentations
return getMergedSymbol(sourceFile.symbol);
}
if (moduleNotFoundError) {
if (errorNode && moduleNotFoundError) {
// report errors only if it was requested
error(errorNode, Diagnostics.File_0_is_not_a_module, sourceFile.fileName);
}
Expand All @@ -4702,6 +4702,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
}

if (!errorNode) {
return undefined;
}

// May be an untyped module. If so, ignore resolutionDiagnostic.
if (resolvedModule && !resolutionExtensionIsTSOrJson(resolvedModule.extension) && resolutionDiagnostic === undefined || resolutionDiagnostic === Diagnostics.Could_not_find_a_declaration_file_for_module_0_1_implicitly_has_an_any_type) {
if (isForAugmentation) {
Expand Down Expand Up @@ -33078,7 +33082,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
? Diagnostics.Cannot_find_module_0_Did_you_mean_to_set_the_moduleResolution_option_to_nodenext_or_to_add_aliases_to_the_paths_option
: Diagnostics.Cannot_find_module_0_or_its_corresponding_type_declarations;
const specifier = getJSXRuntimeImportSpecifier(file, runtimeImportSpecifier);
const mod = resolveExternalModule(specifier || location!, runtimeImportSpecifier, errorMessage, location!);
const mod = resolveExternalModule(specifier || location!, runtimeImportSpecifier, errorMessage, location);
const result = mod && mod !== unknownSymbol ? getMergedSymbol(resolveSymbol(mod)) : undefined;
if (links) {
links.jsxImplicitImportContainer = result || false;
Expand Down
54 changes: 54 additions & 0 deletions src/testRunner/unittests/tsc/composite.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,58 @@ describe("unittests:: tsc:: composite::", () => {
},
],
});

verifyTsc({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

those are tests related to the referenced issue

scenario: "composite",
subScenario: "synthetic jsx import of ESM module from CJS module no crash no jsx element",
fs: () =>
loadProjectFromFiles({
"/src/main.ts": "export default 42;",
"/tsconfig.json": jsonToReadableText({
compilerOptions: {
composite: true,
module: "Node16",
jsx: "react-jsx",
jsxImportSource: "solid-js",
},
}),
"/node_modules/solid-js/package.json": jsonToReadableText({
name: "solid-js",
type: "module",
}),
"/node_modules/solid-js/jsx-runtime.d.ts": Utils.dedent`
export namespace JSX {
type IntrinsicElements = { div: {}; };
}
`,
}),
commandLineArgs: [],
});

verifyTsc({
scenario: "composite",
subScenario: "synthetic jsx import of ESM module from CJS module error on jsx element",
fs: () =>
loadProjectFromFiles({
"/src/main.tsx": "export default <div/>;",
"/tsconfig.json": jsonToReadableText({
compilerOptions: {
composite: true,
module: "Node16",
jsx: "react-jsx",
jsxImportSource: "solid-js",
},
}),
"/node_modules/solid-js/package.json": jsonToReadableText({
name: "solid-js",
type: "module",
}),
"/node_modules/solid-js/jsx-runtime.d.ts": Utils.dedent`
export namespace JSX {
type IntrinsicElements = { div: {}; };
}
`,
}),
commandLineArgs: [],
});
});
42 changes: 42 additions & 0 deletions src/testRunner/unittests/tscWatch/programUpdates.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2164,6 +2164,48 @@ import { x } from "../b";`,
],
});

verifyTscWatch({
scenario,
subScenario: "when changing `allowImportingTsExtensions` of config file 2",
commandLineArgs: ["-w", "-p", ".", "--extendedDiagnostics"],
sys: () => {
const module1: File = {
path: `/user/username/projects/myproject/a.ts`,
content: `export const foo = 10;`,
};
const module2: File = {
path: `/user/username/projects/myproject/b.ts`,
content: `export * as a from "./a.ts";`,
};
const config: File = {
path: `/user/username/projects/myproject/tsconfig.json`,
content: jsonToReadableText({
compilerOptions: {
noEmit: true,
allowImportingTsExtensions: false,
},
}),
};
return createWatchedSystem([module1, module2, config, libFile], { currentDirectory: "/user/username/projects/myproject" });
},
edits: [
{
caption: "Change allowImportingTsExtensions to true",
edit: sys =>
sys.writeFile(
`/user/username/projects/myproject/tsconfig.json`,
jsonToReadableText({
compilerOptions: {
noEmit: true,
allowImportingTsExtensions: true,
},
}),
),
timeouts: sys => sys.runQueuedTimeoutCallbacks(),
},
],
});

verifyTscWatch({
scenario,
subScenario: "when changing checkJs of config file",
Expand Down
Loading