Skip to content

Commit 476054e

Browse files
authored
Fix auto import crashes caused by unrelocatable symbols (microsoft#45055)
* Add failing test * Fix auto import crashes caused by unrelocatable symbols * Use util for testing if symbol is an external module
1 parent 36225c3 commit 476054e

File tree

3 files changed

+107
-2
lines changed

3 files changed

+107
-2
lines changed

src/services/exportInfoMap.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,9 @@ namespace ts {
327327
const seenExports = new Map<Symbol, true>();
328328
const checker = program.getTypeChecker();
329329
const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions);
330-
if (defaultInfo && !checker.isUndefinedSymbol(defaultInfo.symbol)) {
330+
// Note: I think we shouldn't actually see resolved module symbols here, but weird merges
331+
// can cause it to happen: see 'completionsImport_mergedReExport.ts'
332+
if (defaultInfo && !checker.isUndefinedSymbol(defaultInfo.symbol) && !isExternalModuleSymbol(defaultInfo.symbol)) {
331333
cache.add(
332334
importingFile.path,
333335
defaultInfo.symbol,
@@ -339,7 +341,7 @@ namespace ts {
339341
checker);
340342
}
341343
for (const exported of checker.getExportsAndPropertiesOfModule(moduleSymbol)) {
342-
if (exported !== defaultInfo?.symbol && addToSeen(seenExports, exported)) {
344+
if (exported !== defaultInfo?.symbol && !isKnownSymbol(exported) && !isExternalModuleSymbol(exported) && addToSeen(seenExports, exported)) {
343345
cache.add(
344346
importingFile.path,
345347
exported,
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/// <reference path="../fourslash.ts" />
2+
3+
// @Filename: /tsconfig.json
4+
//// { "compilerOptions": { "module": "commonjs" } }
5+
6+
// @Filename: /node_modules/@types/ts-node/index.d.ts
7+
//// export {};
8+
//// declare const REGISTER_INSTANCE: unique symbol;
9+
//// declare global {
10+
//// namespace NodeJS {
11+
//// interface Process {
12+
//// [REGISTER_INSTANCE]?: Service;
13+
//// }
14+
//// }
15+
//// }
16+
17+
// @Filename: /node_modules/@types/node/index.d.ts
18+
//// declare module "process" {
19+
//// global {
20+
//// var process: NodeJS.Process;
21+
//// namespace NodeJS {
22+
//// interface Process {
23+
//// argv: string[];
24+
//// }
25+
//// }
26+
//// }
27+
//// export = process;
28+
//// }
29+
30+
// @Filename: /index.ts
31+
//// I/**/
32+
33+
verify.completions({
34+
marker: "",
35+
preferences: {
36+
includeCompletionsForModuleExports: true,
37+
},
38+
});
39+
40+
edit.insert("N");
41+
42+
// Should not crash
43+
verify.completions({
44+
marker: "",
45+
preferences: {
46+
includeCompletionsForModuleExports: true,
47+
},
48+
});
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/// <reference path="../fourslash.ts" />
2+
3+
// @Filename: /tsconfig.json
4+
//// { "compilerOptions": { "module": "commonjs" } }
5+
6+
// @Filename: /package.json
7+
//// { "dependencies": { "@jest/types": "*", "ts-jest": "*" } }
8+
9+
// @Filename: /node_modules/@jest/types/index.d.ts
10+
//// import type * as Config from "./Config";
11+
//// export type { Config };
12+
13+
// @Filename: /node_modules/@jest/types/Config.d.ts
14+
//// export interface ConfigGlobals {
15+
//// [K: string]: unknown;
16+
//// }
17+
18+
// @Filename: /node_modules/ts-jest/index.d.ts
19+
//// export {};
20+
//// declare module "@jest/types" {
21+
//// namespace Config {
22+
//// interface ConfigGlobals {
23+
//// 'ts-jest': any;
24+
//// }
25+
//// }
26+
//// }
27+
28+
// @Filename: /index.ts
29+
//// C/**/
30+
31+
verify.completions({
32+
marker: "",
33+
preferences: {
34+
includeCompletionsForModuleExports: true,
35+
},
36+
});
37+
38+
edit.insert("o");
39+
40+
// Should not crash
41+
verify.completions({
42+
marker: "",
43+
preferences: {
44+
includeCompletionsForModuleExports: true,
45+
},
46+
});
47+
48+
// Because of the way `Config` is merged, we are actually not including it
49+
// in completions here, though it would be better if we could. The `exports`
50+
// of "@jest/types/index" would contain an alias symbol named `Config` without
51+
// the merge from ts-jest, but with the merge, the `exports` contains the merge
52+
// of `namespace Config` and the "@jest/types/Config" module symbol. This is
53+
// unexpected (to me) and difficult to work with, and might be wrong? My
54+
// expectation would have been to preserve the export alias symbol, but let it
55+
// *resolve* to the merge of the SourceFile and the namespace declaration.

0 commit comments

Comments
 (0)