Actually fix it

typescript-bot · Mar 31, 2023 · 8c81daf · 8c81daf
1 parent 1fddb2a
commit 8c81daf
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 13 deletions.
diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts
@@ -279,6 +279,7 @@ import {
     setValueDeclaration,
     ShorthandPropertyAssignment,
     shouldPreserveConstEnums,
+    shouldResolveJsRequire,
     SignatureDeclaration,
     skipParentheses,
     sliceAfter,
@@ -3525,7 +3526,7 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
         if (!isBindingPattern(node.name)) {
             const possibleVariableDecl = node.kind === SyntaxKind.VariableDeclaration ? node : node.parent.parent;
             if (isInJSFile(node) &&
-                getEmitModuleResolutionKind(options) !== ModuleResolutionKind.Bundler &&
+                shouldResolveJsRequire(compilerOptions) &&
                 isVariableDeclarationInitializedToBareOrAccessedRequire(possibleVariableDecl) &&
                 !getJSDocTypeTag(node) &&
                 !(getCombinedModifierFlags(node) & ModifierFlags.Export)

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -940,6 +940,7 @@ import {
     ShorthandPropertyAssignment,
     shouldAllowImportingTsExtension,
     shouldPreserveConstEnums,
+    shouldResolveJsRequire,
     Signature,
     SignatureDeclaration,
     SignatureFlags,
@@ -34163,8 +34164,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         }
 
         // In JavaScript files, calls to any identifier 'require' are treated as external module imports
-        if (isInJSFile(node) && (getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.Bundler || compilerOptions.noDtsResolution) && isCommonJsRequire(node)) {
-            // `bundler` doesn't support resolving `require`, but needs to in `noDtsResolution` to support Find Source Definition
+        if (isInJSFile(node) && shouldResolveJsRequire(compilerOptions) && isCommonJsRequire(node)) {
             return resolveExternalModuleTypeByLiteral(node.arguments![0] as StringLiteral);
         }
 

diff --git a/src/compiler/program.ts b/src/compiler/program.ts
@@ -329,6 +329,7 @@ import {
     WriteFileCallbackData,
     writeFileEnsuringDirectories,
     zipToModeAwareCache,
+    shouldResolveJsRequire,
 } from "./_namespaces/ts";
 import * as performance from "./_namespaces/ts.performance";
 
@@ -3211,8 +3212,7 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
             collectModuleReferences(node, /*inAmbientModule*/ false);
         }
 
-        // `require` has no special meaning in `--moduleResolution bundler`
-        const shouldProcessRequires = isJavaScriptFile && getEmitModuleResolutionKind(options) !== ModuleResolutionKind.Bundler;
+        const shouldProcessRequires = isJavaScriptFile && shouldResolveJsRequire(options);
         if ((file.flags & NodeFlags.PossiblyContainsDynamicImport) || shouldProcessRequires) {
             collectDynamicImportOrRequireCalls(file);
         }

diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -8438,6 +8438,12 @@ export function moduleResolutionSupportsPackageJsonExportsAndImports(moduleResol
         || moduleResolution === ModuleResolutionKind.Bundler;
 }
 
+/** @internal */
+export function shouldResolveJsRequire(compilerOptions: CompilerOptions): boolean {
+    // `bundler` doesn't support resolving `require`, but needs to in `noDtsResolution` to support Find Source Definition
+    return !!compilerOptions.noDtsResolution || getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.Bundler;
+}
+
 /** @internal */
 export function getResolvePackageJsonExports(compilerOptions: CompilerOptions) {
     const moduleResolution = getEmitModuleResolutionKind(compilerOptions);

diff --git a/tests/baselines/reference/goToSource15_bundler.baseline.jsonc b/tests/baselines/reference/goToSource15_bundler.baseline.jsonc
@@ -1,19 +1,31 @@
 // === goToSourceDefinition ===
-// === /node_modules/react/index.js ===
-// [|'use strict';
-// 
-// if (process.env.NODE_ENV === 'production') {
-//   module.exports = require('./cjs/react.production.min.js');
-// } else {
-//   module.exports = require('./cjs/react.development.js');
-// }|]
+// === /node_modules/react/cjs/react.production.min.js ===
+// 'use strict';exports.[|{| defId: 0 |}useState|]=function(a){};exports.version='16.8.6';
+
+// === /node_modules/react/cjs/react.development.js ===
+// 'use strict';
+// if (process.env.NODE_ENV !== 'production') {
+//   (function() {
+//     function useState(initialState) {}
+//     exports.[|{| defId: 1 |}useState|] = useState;
+//     exports.version = '16.8.6';
+//   }());
+// }
 
 // === /index.ts ===
 // import { /*GOTO SOURCE DEF*/useState } from 'react';
 
   // === Details ===
   [
    {
+    "defId": 0,
+    "containerKind": "",
+    "containerName": "",
+    "kind": "",
+    "name": ""
+   },
+   {
+    "defId": 1,
     "containerKind": "",
     "containerName": "",
     "kind": "",