Simplify handling of node:-prefixed modules in auto-imports (#59702)

microsoft · Aug 21, 2024 · a5eec24 · a5eec24
1 parent f6ec916
commit a5eec24
Show file tree

Hide file tree

Showing 16 changed files with 941 additions and 189 deletions.
diff --git a/src/compiler/program.ts b/src/compiler/program.ts
@@ -75,6 +75,7 @@ import {
     ensureTrailingDirectorySeparator,
     equateStringsCaseInsensitive,
     equateStringsCaseSensitive,
+    exclusivelyPrefixedNodeCoreModules,
     explainIfFileIsRedirectAndImpliedFormat,
     ExportAssignment,
     ExportDeclaration,
@@ -323,6 +324,7 @@ import {
     TypeChecker,
     typeDirectiveIsEqualTo,
     TypeReferenceDirectiveResolutionCache,
+    unprefixedNodeCoreModules,
     VariableDeclaration,
     VariableStatement,
     Version,
@@ -1758,7 +1760,7 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
     let sourceFileToPackageName = new Map<Path, string>();
     // Key is a file name. Value is the (non-empty, or undefined) list of files that redirect to it.
     let redirectTargetsMap = createMultiMap<Path, string>();
-    let usesUriStyleNodeCoreModules = false;
+    let usesUriStyleNodeCoreModules: boolean | undefined;
 
     /**
      * map with
@@ -3499,7 +3501,14 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
                     setParentRecursive(node, /*incremental*/ false); // we need parent data on imports before the program is fully bound, so we ensure it's set here
                     imports = append(imports, moduleNameExpr);
                     if (!usesUriStyleNodeCoreModules && currentNodeModulesDepth === 0 && !file.isDeclarationFile) {
-                        usesUriStyleNodeCoreModules = startsWith(moduleNameExpr.text, "node:");
+                        if (startsWith(moduleNameExpr.text, "node:") && !exclusivelyPrefixedNodeCoreModules.has(moduleNameExpr.text)) {
+                            // Presence of `node:` prefix takes precedence over unprefixed node core modules
+                            usesUriStyleNodeCoreModules = true;
+                        }
+                        else if (usesUriStyleNodeCoreModules === undefined && unprefixedNodeCoreModules.has(moduleNameExpr.text)) {
+                            // Avoid `unprefixedNodeCoreModules.has` for every import
+                            usesUriStyleNodeCoreModules = false;
+                        }
                     }
                 }
             }

diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -4866,11 +4866,14 @@ export interface Program extends ScriptReferenceHost {
      */
     redirectTargetsMap: MultiMap<Path, string>;
     /**
-     * Whether any (non-external, non-declaration) source files use `node:`-prefixed module specifiers.
+     * Whether any (non-external, non-declaration) source files use `node:`-prefixed module specifiers
+     * (except for those that are not available without the prefix).
+     * `false` indicates that an unprefixed builtin module was seen; `undefined` indicates that no
+     * builtin modules (or only modules exclusively available with the prefix) were seen.
      *
      * @internal
      */
-    readonly usesUriStyleNodeCoreModules: boolean;
+    readonly usesUriStyleNodeCoreModules: boolean | undefined;
     /**
      * Map from libFileName to actual resolved location of the lib
      * @internal

diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -11796,3 +11796,83 @@ export function isSideEffectImport(node: Node): boolean {
     const ancestor = findAncestor(node, isImportDeclaration);
     return !!ancestor && !ancestor.importClause;
 }
+
+// require('module').builtinModules.filter(x => !x.startsWith('_'))
+const unprefixedNodeCoreModulesList = [
+    "assert",
+    "assert/strict",
+    "async_hooks",
+    "buffer",
+    "child_process",
+    "cluster",
+    "console",
+    "constants",
+    "crypto",
+    "dgram",
+    "diagnostics_channel",
+    "dns",
+    "dns/promises",
+    "domain",
+    "events",
+    "fs",
+    "fs/promises",
+    "http",
+    "http2",
+    "https",
+    "inspector",
+    "inspector/promises",
+    "module",
+    "net",
+    "os",
+    "path",
+    "path/posix",
+    "path/win32",
+    "perf_hooks",
+    "process",
+    "punycode",
+    "querystring",
+    "readline",
+    "readline/promises",
+    "repl",
+    "stream",
+    "stream/consumers",
+    "stream/promises",
+    "stream/web",
+    "string_decoder",
+    "sys",
+    "test/mock_loader",
+    "timers",
+    "timers/promises",
+    "tls",
+    "trace_events",
+    "tty",
+    "url",
+    "util",
+    "util/types",
+    "v8",
+    "vm",
+    "wasi",
+    "worker_threads",
+    "zlib",
+];
+
+/** @internal */
+export const unprefixedNodeCoreModules = new Set(unprefixedNodeCoreModulesList);
+
+// await fetch('https://nodejs.org/docs/latest/api/all.json').then(r => r.text()).then(t =>
+//   new Set(t.match(/(?<=')node:.+?(?=')/g))
+//     .difference(new Set(require('module').builtinModules.map(x => `node:${x}`))))
+/** @internal */
+export const exclusivelyPrefixedNodeCoreModules = new Set([
+    "node:sea",
+    "node:sqlite",
+    "node:test",
+    "node:test/reporters",
+]);
+
+/** @internal */
+export const nodeCoreModules = new Set([
+    ...unprefixedNodeCoreModulesList,
+    ...unprefixedNodeCoreModulesList.map(name => `node:${name}`),
+    ...exclusivelyPrefixedNodeCoreModules,
+]);
diff --git a/src/jsTyping/jsTyping.ts b/src/jsTyping/jsTyping.ts
@@ -19,6 +19,7 @@ import {
     hasJSFileExtension,
     mapDefined,
     MapLike,
+    nodeCoreModules,
     normalizePath,
     Path,
     readConfigFile,
@@ -61,64 +62,6 @@ export function isTypingUpToDate(cachedTyping: CachedTyping, availableTypingVers
     return availableVersion.compareTo(cachedTyping.version) <= 0;
 }
 
-const unprefixedNodeCoreModuleList = [
-    "assert",
-    "assert/strict",
-    "async_hooks",
-    "buffer",
-    "child_process",
-    "cluster",
-    "console",
-    "constants",
-    "crypto",
-    "dgram",
-    "diagnostics_channel",
-    "dns",
-    "dns/promises",
-    "domain",
-    "events",
-    "fs",
-    "fs/promises",
-    "http",
-    "https",
-    "http2",
-    "inspector",
-    "module",
-    "net",
-    "os",
-    "path",
-    "perf_hooks",
-    "process",
-    "punycode",
-    "querystring",
-    "readline",
-    "repl",
-    "stream",
-    "stream/promises",
-    "string_decoder",
-    "timers",
-    "timers/promises",
-    "tls",
-    "trace_events",
-    "tty",
-    "url",
-    "util",
-    "util/types",
-    "v8",
-    "vm",
-    "wasi",
-    "worker_threads",
-    "zlib",
-];
-
-const prefixedNodeCoreModuleList = unprefixedNodeCoreModuleList.map(name => `node:${name}`);
-
-/** @internal */
-export const nodeCoreModuleList: readonly string[] = [...unprefixedNodeCoreModuleList, ...prefixedNodeCoreModuleList];
-
-/** @internal */
-export const nodeCoreModules = new Set(nodeCoreModuleList);
-
 /** @internal */
 export function nonRelativeModuleNameForTypingCache(moduleName: string) {
     return nodeCoreModules.has(moduleName) ? "node" : moduleName;

diff --git a/src/services/codefixes/fixCannotFindModule.ts b/src/services/codefixes/fixCannotFindModule.ts
@@ -11,8 +11,8 @@ import {
     InstallPackageAction,
     isExternalModuleNameRelative,
     isStringLiteral,
-    JsTyping,
     LanguageServiceHost,
+    nodeCoreModules,
     parsePackageName,
     SourceFile,
     tryCast,
@@ -71,6 +71,6 @@ function tryGetImportedPackageName(sourceFile: SourceFile, pos: number): string
 
 function getTypesPackageNameToInstall(packageName: string, host: LanguageServiceHost, diagCode: number): string | undefined {
     return diagCode === errorCodeCannotFindModule
-        ? (JsTyping.nodeCoreModules.has(packageName) ? "@types/node" : undefined)
+        ? (nodeCoreModules.has(packageName) ? "@types/node" : undefined)
         : (host.isKnownTypesPackageName?.(packageName) ? getTypesPackageName(packageName) : undefined);
 }
diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts
@@ -35,7 +35,6 @@ import {
     ExportKind,
     ExportMapInfoKey,
     factory,
-    fileContainsPackageImport,
     findAncestor,
     first,
     firstDefined,
@@ -84,7 +83,7 @@ import {
     isExternalModuleReference,
     isFullSourceFile,
     isIdentifier,
-    isImportableFile,
+    isImportable,
     isImportDeclaration,
     isImportEqualsDeclaration,
     isIntrinsicJsxName,
@@ -1542,10 +1541,7 @@ function getExportInfos(
     });
     function addSymbol(moduleSymbol: Symbol, toFile: SourceFile | undefined, exportedSymbol: Symbol, exportKind: ExportKind, program: Program, isFromPackageJson: boolean): void {
         const moduleSpecifierResolutionHost = getModuleSpecifierResolutionHost(isFromPackageJson);
-        if (
-            toFile && isImportableFile(program, fromFile, toFile, preferences, packageJsonFilter, moduleSpecifierResolutionHost, moduleSpecifierCache) ||
-            (!toFile && packageJsonFilter.allowsImportingAmbientModule(moduleSymbol, moduleSpecifierResolutionHost) || fileContainsPackageImport(fromFile, stripQuotes(moduleSymbol.name)))
-        ) {
+        if (isImportable(program, fromFile, toFile, moduleSymbol, preferences, packageJsonFilter, moduleSpecifierResolutionHost, moduleSpecifierCache)) {
             const checker = program.getTypeChecker();
             originalSymbolToExportInfos.add(getUniqueSymbolId(exportedSymbol, checker).toString(), { symbol: exportedSymbol, moduleSymbol, moduleFileName: toFile?.fileName, exportKind, targetFlags: skipAlias(exportedSymbol, checker).flags, isFromPackageJson });
         }

diff --git a/src/services/completions.ts b/src/services/completions.ts
@@ -64,7 +64,6 @@ import {
     Expression,
     ExpressionWithTypeArguments,
     factory,
-    fileContainsPackageImport,
     filter,
     find,
     findAncestor,
@@ -175,7 +174,7 @@ import {
     isIdentifierPart,
     isIdentifierStart,
     isIdentifierText,
-    isImportableFile,
+    isImportable,
     isImportAttributes,
     isImportDeclaration,
     isImportEqualsDeclaration,
@@ -274,7 +273,6 @@ import {
     JSDocTypedefTag,
     JSDocTypeExpression,
     JSDocTypeTag,
-    JsTyping,
     JsxAttribute,
     JsxAttributes,
     JsxClosingElement,
@@ -344,7 +342,6 @@ import {
     SemanticMeaning,
     setEmitFlags,
     setSnippetElement,
-    shouldUseUriStyleNodeCoreModules,
     SignatureHelp,
     SignatureKind,
     singleElementArray,
@@ -4221,19 +4218,11 @@ function getCompletionData(
         );
 
         function isImportableExportInfo(info: SymbolExportInfo) {
-            const moduleFile = tryCast(info.moduleSymbol.valueDeclaration, isSourceFile);
-            if (!moduleFile) {
-                const moduleName = stripQuotes(info.moduleSymbol.name);
-                if (JsTyping.nodeCoreModules.has(moduleName) && startsWith(moduleName, "node:") !== shouldUseUriStyleNodeCoreModules(sourceFile, program)) {
-                    return false;
-                }
-                return (packageJsonFilter?.allowsImportingAmbientModule(info.moduleSymbol, getModuleSpecifierResolutionHost(info.isFromPackageJson)) ?? true)
-                    || fileContainsPackageImport(sourceFile, moduleName);
-            }
-            return isImportableFile(
+            return isImportable(
                 info.isFromPackageJson ? packageJsonAutoImportProvider! : program,
                 sourceFile,
-                moduleFile,
+                tryCast(info.moduleSymbol.valueDeclaration, isSourceFile),
+                info.moduleSymbol,
                 preferences,
                 packageJsonFilter,
                 getModuleSpecifierResolutionHost(info.isFromPackageJson),
@@ -4371,7 +4360,7 @@ function getCompletionData(
             // dprint-ignore
             switch (tokenKind) {
                 case SyntaxKind.CommaToken:
-                    switch (containingNodeKind) {    
+                    switch (containingNodeKind) {
                         case SyntaxKind.CallExpression:                                               // func( a, |
                         case SyntaxKind.NewExpression: {                                              // new C(a, |
                             const expression = (contextToken.parent as CallExpression | NewExpression).expression;
@@ -4454,7 +4443,7 @@ function getCompletionData(
                     }
 
                 case SyntaxKind.TemplateHead:
-                    return { 
+                    return {
                         defaultCommitCharacters: allCommitCharacters,
                         isNewIdentifierLocation: containingNodeKind === SyntaxKind.TemplateExpression // `aa ${|
                     };