Skip to content

Commit

Permalink
Consistently avoid module resolution errors when using `getSymbolAtLo…
Browse files Browse the repository at this point in the history 
…cation` (microsoft#58668)
  • Loading branch information
@Andarist
Andarist authored Jun 24, 2024
1 parent 5d70bf8 commit 3743fbc
Show file tree
Hide file tree
Showing 8 changed files with 602 additions and 21 deletions.
30 changes: 17 additions & 13 deletions src/compiler/checker.ts
View file
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
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,58 @@ describe("unittests:: tsc:: composite::", () => {
},
],
});

verifyTsc({
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
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

0 comments on commit 3743fbc

Please sign in to comment.