Skip to content

Commit 36225c3

Browse files
andrewbranchandrewbranch
authored
Detect preference for Unode:-prefixed node core modules (microsoft#45080)
1 parent ddbd829 commit 36225c3

13 files changed

+313
-11
lines changed

‎src/compiler/program.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -886,6 +886,7 @@ namespace ts {
886886
let sourceFileToPackageName = new Map<Path, string>();
887887
// Key is a file name. Value is the (non-empty, or undefined) list of files that redirect to it.
888888
let redirectTargetsMap = createMultiMap<Path, string>();
889+
let usesUriStyleNodeCoreModules = false;
889890

890891
/**
891892
* map with
@@ -1087,6 +1088,7 @@ namespace ts {
10871088
getLibFileFromReference,
10881089
sourceFileToPackageName,
10891090
redirectTargetsMap,
1091+
usesUriStyleNodeCoreModules,
10901092
isEmittedFile,
10911093
getConfigFileParsingDiagnostics,
10921094
getResolvedModuleWithFailedLookupLocationsFromCache,
@@ -1635,6 +1637,7 @@ namespace ts {
16351637

16361638
sourceFileToPackageName = oldProgram.sourceFileToPackageName;
16371639
redirectTargetsMap = oldProgram.redirectTargetsMap;
1640+
usesUriStyleNodeCoreModules = oldProgram.usesUriStyleNodeCoreModules;
16381641

16391642
return StructureIsReused.Completely;
16401643
}
@@ -2327,6 +2330,9 @@ namespace ts {
23272330
// only through top - level external module names. Relative external module names are not permitted.
23282331
if (moduleNameExpr && isStringLiteral(moduleNameExpr) && moduleNameExpr.text && (!inAmbientModule || !isExternalModuleNameRelative(moduleNameExpr.text))) {
23292332
imports = append(imports, moduleNameExpr);
2333+
if (!usesUriStyleNodeCoreModules && currentNodeModulesDepth === 0 && !file.isDeclarationFile) {
2334+
usesUriStyleNodeCoreModules = startsWith(moduleNameExpr.text, "node:");
2335+
}
23302336
}
23312337
}
23322338
else if (isModuleDeclaration(node)) {

‎src/compiler/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3964,6 +3964,8 @@ namespace ts {
39643964
/* @internal */ sourceFileToPackageName: ESMap<Path, string>;
39653965
/** Set of all source files that some other source file redirects to. */
39663966
/* @internal */ redirectTargetsMap: MultiMap<Path, string>;
3967+
/** Whether any (non-external, non-declaration) source files use `node:`-prefixed module specifiers. */
3968+
/* @internal */ readonly usesUriStyleNodeCoreModules: boolean;
39673969
/** Is the file emitted file */
39683970
/* @internal */ isEmittedFile(file: string): boolean;
39693971
/* @internal */ getFileIncludeReasons(): MultiMap<Path, FileIncludeReason>;

‎src/harness/fourslashInterfaceImpl.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1409,7 +1409,7 @@ namespace FourSlashInterface {
14091409
"await",
14101410
].map(keywordEntry);
14111411

1412-
export const undefinedVarEntry: ExpectedCompletionEntry = {
1412+
export const undefinedVarEntry: ExpectedCompletionEntryObject = {
14131413
name: "undefined",
14141414
kind: "var",
14151415
sortText: SortText.GlobalsOrKeywords
@@ -1568,11 +1568,14 @@ namespace FourSlashInterface {
15681568
];
15691569

15701570
export function globalsPlus(plus: readonly ExpectedCompletionEntry[]): readonly ExpectedCompletionEntry[] {
1571+
const firstEntry = plus[0];
1572+
const afterUndefined = typeof firstEntry !== "string" && firstEntry.sortText! > undefinedVarEntry.sortText!;
15711573
return [
15721574
globalThisEntry,
15731575
...globalsVars,
1574-
...plus,
1576+
...afterUndefined ? ts.emptyArray : plus,
15751577
undefinedVarEntry,
1578+
...afterUndefined ? plus : ts.emptyArray,
15761579
...globalKeywords];
15771580
}
15781581

‎src/jsTyping/jsTyping.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@ namespace ts.JsTyping {
2929
return availableVersion.compareTo(cachedTyping.version) <= 0;
3030
}
3131

32-
export const nodeCoreModuleList: readonly string[] = [
32+
const unprefixedNodeCoreModuleList = [
3333
"assert",
34+
"assert/strict",
3435
"async_hooks",
3536
"buffer",
3637
"child_process",
@@ -39,14 +40,18 @@ namespace ts.JsTyping {
3940
"constants",
4041
"crypto",
4142
"dgram",
43+
"diagnostics_channel",
4244
"dns",
45+
"dns/promises",
4346
"domain",
4447
"events",
4548
"fs",
49+
"fs/promises",
4650
"http",
4751
"https",
4852
"http2",
4953
"inspector",
54+
"module",
5055
"net",
5156
"os",
5257
"path",
@@ -57,17 +62,27 @@ namespace ts.JsTyping {
5762
"readline",
5863
"repl",
5964
"stream",
65+
"stream/promises",
6066
"string_decoder",
6167
"timers",
68+
"timers/promises",
6269
"tls",
70+
"trace_events",
6371
"tty",
6472
"url",
6573
"util",
74+
"util/types",
6675
"v8",
6776
"vm",
77+
"wasi",
78+
"worker_threads",
6879
"zlib"
6980
];
7081

82+
export const prefixedNodeCoreModuleList = unprefixedNodeCoreModuleList.map(name => `node:${name}`);
83+
84+
export const nodeCoreModuleList: readonly string[] = [...unprefixedNodeCoreModuleList, ...prefixedNodeCoreModuleList];
85+
7186
export const nodeCoreModules = new Set(nodeCoreModuleList);
7287

7388
export function nonRelativeModuleNameForTypingCache(moduleName: string) {

‎src/services/codefixes/importFixes.ts‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ namespace ts.codefix {
212212

213213
function getImportFixForSymbol(sourceFile: SourceFile, exportInfos: readonly SymbolExportInfo[], moduleSymbol: Symbol, symbolName: string, program: Program, position: number | undefined, preferTypeOnlyImport: boolean, useRequire: boolean, host: LanguageServiceHost, preferences: UserPreferences) {
214214
Debug.assert(exportInfos.some(info => info.moduleSymbol === moduleSymbol), "Some exportInfo should match the specified moduleSymbol");
215-
return getBestFix(getImportFixes(exportInfos, symbolName, position, preferTypeOnlyImport, useRequire, program, sourceFile, host, preferences), sourceFile, host, preferences);
215+
return getBestFix(getImportFixes(exportInfos, symbolName, position, preferTypeOnlyImport, useRequire, program, sourceFile, host, preferences), sourceFile, program, host, preferences);
216216
}
217217

218218
function codeFixActionToCodeAction({ description, changes, commands }: CodeFixAction): CodeAction {
@@ -290,7 +290,7 @@ namespace ts.codefix {
290290
host,
291291
preferences,
292292
fromCacheOnly);
293-
const result = getBestFix(fixes, importingFile, host, preferences);
293+
const result = getBestFix(fixes, importingFile, program, host, preferences);
294294
return result && { ...result, computedWithoutCacheCount };
295295
}
296296

@@ -506,34 +506,41 @@ namespace ts.codefix {
506506
const info = errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code
507507
? getFixesInfoForUMDImport(context, symbolToken)
508508
: isIdentifier(symbolToken) ? getFixesInfoForNonUMDImport(context, symbolToken, useAutoImportProvider) : undefined;
509-
return info && { ...info, fixes: sortFixes(info.fixes, context.sourceFile, context.host, context.preferences) };
509+
return info && { ...info, fixes: sortFixes(info.fixes, context.sourceFile, context.program, context.host, context.preferences) };
510510
}
511511

512-
function sortFixes(fixes: readonly ImportFix[], sourceFile: SourceFile, host: LanguageServiceHost, preferences: UserPreferences): readonly ImportFix[] {
512+
function sortFixes(fixes: readonly ImportFix[], sourceFile: SourceFile, program: Program, host: LanguageServiceHost, preferences: UserPreferences): readonly ImportFix[] {
513513
const { allowsImportingSpecifier } = createPackageJsonImportFilter(sourceFile, preferences, host);
514-
return sort(fixes, (a, b) => compareValues(a.kind, b.kind) || compareModuleSpecifiers(a, b, allowsImportingSpecifier));
514+
return sort(fixes, (a, b) => compareValues(a.kind, b.kind) || compareModuleSpecifiers(a, b, sourceFile, program, allowsImportingSpecifier));
515515
}
516516

517-
function getBestFix<T extends ImportFix>(fixes: readonly T[], sourceFile: SourceFile, host: LanguageServiceHost, preferences: UserPreferences): T | undefined {
517+
function getBestFix<T extends ImportFix>(fixes: readonly T[], sourceFile: SourceFile, program: Program, host: LanguageServiceHost, preferences: UserPreferences): T | undefined {
518518
if (!some(fixes)) return;
519519
// These will always be placed first if available, and are better than other kinds
520520
if (fixes[0].kind === ImportFixKind.UseNamespace || fixes[0].kind === ImportFixKind.AddToExisting) {
521521
return fixes[0];
522522
}
523523
const { allowsImportingSpecifier } = createPackageJsonImportFilter(sourceFile, preferences, host);
524524
return fixes.reduce((best, fix) =>
525-
compareModuleSpecifiers(fix, best, allowsImportingSpecifier) === Comparison.LessThan ? fix : best
525+
compareModuleSpecifiers(fix, best, sourceFile, program, allowsImportingSpecifier) === Comparison.LessThan ? fix : best
526526
);
527527
}
528528

529-
function compareModuleSpecifiers(a: ImportFix, b: ImportFix, allowsImportingSpecifier: (specifier: string) => boolean): Comparison {
529+
function compareModuleSpecifiers(a: ImportFix, b: ImportFix, importingFile: SourceFile, program: Program, allowsImportingSpecifier: (specifier: string) => boolean): Comparison {
530530
if (a.kind !== ImportFixKind.UseNamespace && b.kind !== ImportFixKind.UseNamespace) {
531531
return compareBooleans(allowsImportingSpecifier(a.moduleSpecifier), allowsImportingSpecifier(b.moduleSpecifier))
532+
|| compareNodeCoreModuleSpecifiers(a.moduleSpecifier, b.moduleSpecifier, importingFile, program)
532533
|| compareNumberOfDirectorySeparators(a.moduleSpecifier, b.moduleSpecifier);
533534
}
534535
return Comparison.EqualTo;
535536
}
536537

538+
function compareNodeCoreModuleSpecifiers(a: string, b: string, importingFile: SourceFile, program: Program): Comparison {
539+
if (startsWith(a, "node:") && !startsWith(b, "node:")) return shouldUseUriStyleNodeCoreModules(importingFile, program) ? Comparison.LessThan : Comparison.GreaterThan;
540+
if (startsWith(b, "node:") && !startsWith(a, "node:")) return shouldUseUriStyleNodeCoreModules(importingFile, program) ? Comparison.GreaterThan : Comparison.LessThan;
541+
return Comparison.EqualTo;
542+
}
543+
537544
function getFixesInfoForUMDImport({ sourceFile, program, host, preferences }: CodeFixContextBase, token: Node): FixesInfo | undefined {
538545
const checker = program.getTypeChecker();
539546
const umdSymbol = getUmdSymbol(token, checker);

‎src/services/completions.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1959,6 +1959,10 @@ namespace ts.Completions {
19591959
function isImportableExportInfo(info: SymbolExportInfo) {
19601960
const moduleFile = tryCast(info.moduleSymbol.valueDeclaration, isSourceFile);
19611961
if (!moduleFile) {
1962+
const moduleName = stripQuotes(info.moduleSymbol.name);
1963+
if (JsTyping.nodeCoreModules.has(moduleName) && startsWith(moduleName, "node:") !== shouldUseUriStyleNodeCoreModules(sourceFile, program)) {
1964+
return false;
1965+
}
19621966
return packageJsonFilter
19631967
? packageJsonFilter.allowsImportingAmbientModule(info.moduleSymbol, getModuleSpecifierResolutionHost(info.isFromPackageJson))
19641968
: true;

‎src/services/utilities.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3176,5 +3176,14 @@ namespace ts {
31763176
return !!(getCombinedNodeFlagsAlwaysIncludeJSDoc(decl) & ModifierFlags.Deprecated);
31773177
}
31783178

3179+
export function shouldUseUriStyleNodeCoreModules(file: SourceFile, program: Program): boolean {
3180+
const decisionFromFile = firstDefined(file.imports, node => {
3181+
if (JsTyping.nodeCoreModules.has(node.text)) {
3182+
return startsWith(node.text, "node:");
3183+
}
3184+
});
3185+
return decisionFromFile ?? program.usesUriStyleNodeCoreModules;
3186+
}
3187+
31793188
// #endregion
31803189
}

‎tests/cases/fourslash/completionsImport_uriStyleNodeCoreModules1.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @module: commonjs
4+
5+
// @Filename: /node_modules/@types/node/index.d.ts
6+
//// declare module "fs" { function writeFile(): void }
7+
//// declare module "fs/promises" { function writeFile(): Promise<void> }
8+
//// declare module "node:fs" { export * from "fs"; }
9+
//// declare module "node:fs/promises" { export * from "fs/promises"; }
10+
11+
// @Filename: /index.ts
12+
//// write/**/
13+
14+
verify.completions({
15+
marker: "",
16+
exact: completion.globalsPlus([{
17+
name: "writeFile",
18+
source: "fs",
19+
hasAction: true,
20+
sortText: completion.SortText.AutoImportSuggestions
21+
}, {
22+
name: "writeFile",
23+
source: "fs/promises",
24+
hasAction: true,
25+
sortText: completion.SortText.AutoImportSuggestions
26+
}]),
27+
preferences: {
28+
includeCompletionsForModuleExports: true,
29+
},
30+
});

‎tests/cases/fourslash/completionsImport_uriStyleNodeCoreModules2.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @module: commonjs
4+
5+
// @Filename: /node_modules/@types/node/index.d.ts
6+
//// declare module "fs" { function writeFile(): void }
7+
//// declare module "fs/promises" { function writeFile(): Promise<void> }
8+
//// declare module "node:fs" { export * from "fs"; }
9+
//// declare module "node:fs/promises" { export * from "fs/promises"; }
10+
11+
// @Filename: /other.ts
12+
//// import "node:fs/promises";
13+
14+
// @Filename: /index.ts
15+
//// write/**/
16+
17+
verify.completions({
18+
marker: "",
19+
exact: completion.globalsPlus([{
20+
name: "writeFile",
21+
source: "node:fs",
22+
hasAction: true,
23+
sortText: completion.SortText.AutoImportSuggestions
24+
}, {
25+
name: "writeFile",
26+
source: "node:fs/promises",
27+
hasAction: true,
28+
sortText: completion.SortText.AutoImportSuggestions
29+
}]),
30+
preferences: {
31+
includeCompletionsForModuleExports: true,
32+
},
33+
});

‎tests/cases/fourslash/completionsImport_uriStyleNodeCoreModules3.ts‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @module: commonjs
4+
5+
// @Filename: /node_modules/@types/node/index.d.ts
6+
//// declare module "path" { function join(...segments: readonly string[]): string; }
7+
//// declare module "node:path" { export * from "path"; }
8+
//// declare module "fs" { function writeFile(): void }
9+
//// declare module "fs/promises" { function writeFile(): Promise<void> }
10+
//// declare module "node:fs" { export * from "fs"; }
11+
//// declare module "node:fs/promises" { export * from "fs/promises"; }
12+
13+
// @Filename: /other.ts
14+
//// import "node:fs/promises";
15+
16+
// @Filename: /noPrefix.ts
17+
//// import "path";
18+
//// write/*noPrefix*/
19+
20+
// @Filename: /prefix.ts
21+
//// import "node:path";
22+
//// write/*prefix*/
23+
24+
// @Filename: /mixed1.ts
25+
//// import "path";
26+
//// import "node:path";
27+
//// write/*mixed1*/
28+
29+
// @Filename: /mixed2.ts
30+
//// import "node:path";
31+
//// import "path";
32+
//// write/*mixed2*/
33+
34+
verify.completions({
35+
marker: "noPrefix",
36+
exact: completion.globalsPlus([{
37+
name: "writeFile",
38+
source: "fs",
39+
hasAction: true,
40+
sortText: completion.SortText.AutoImportSuggestions
41+
}, {
42+
name: "writeFile",
43+
source: "fs/promises",
44+
hasAction: true,
45+
sortText: completion.SortText.AutoImportSuggestions
46+
}]),
47+
preferences: {
48+
includeCompletionsForModuleExports: true,
49+
},
50+
});
51+
52+
verify.completions({
53+
marker: "prefix",
54+
exact: completion.globalsPlus([{
55+
name: "writeFile",
56+
source: "node:fs",
57+
hasAction: true,
58+
sortText: completion.SortText.AutoImportSuggestions
59+
}, {
60+
name: "writeFile",
61+
source: "node:fs/promises",
62+
hasAction: true,
63+
sortText: completion.SortText.AutoImportSuggestions
64+
}]),
65+
preferences: {
66+
includeCompletionsForModuleExports: true,
67+
},
68+
});
69+
70+
// We're doing as little work as possible to decide which module specifiers
71+
// to use, so we just take the *first* recognized node core module in the file
72+
// and copy its style.
73+
74+
verify.completions({
75+
marker: "mixed1",
76+
exact: completion.globalsPlus([{
77+
name: "writeFile",
78+
source: "fs",
79+
hasAction: true,
80+
sortText: completion.SortText.AutoImportSuggestions
81+
}, {
82+
name: "writeFile",
83+
source: "fs/promises",
84+
hasAction: true,
85+
sortText: completion.SortText.AutoImportSuggestions
86+
}]),
87+
preferences: {
88+
includeCompletionsForModuleExports: true,
89+
},
90+
});
91+
92+
verify.completions({
93+
marker: "mixed2",
94+
exact: completion.globalsPlus([{
95+
name: "writeFile",
96+
source: "node:fs",
97+
hasAction: true,
98+
sortText: completion.SortText.AutoImportSuggestions
99+
}, {
100+
name: "writeFile",
101+
source: "node:fs/promises",
102+
hasAction: true,
103+
sortText: completion.SortText.AutoImportSuggestions
104+
}]),
105+
preferences: {
106+
includeCompletionsForModuleExports: true,
107+
},
108+
});

0 commit comments

Comments
 (0)