Skip to content

Commit 9052ef8

Browse files
authored
Fix Go To Source Definition in --moduleResolution bundler (#53613)
1 parent 2eab265 commit 9052ef8

File tree

6 files changed

+83
-9
lines changed

6 files changed

+83
-9
lines changed

src/compiler/binder.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ import {
8585
getContainingClass,
8686
getEffectiveContainerForJSDocTemplateTag,
8787
getElementOrPropertyAccessName,
88-
getEmitModuleResolutionKind,
8988
getEmitScriptTarget,
9089
getEnclosingBlockScopeContainer,
9190
getErrorSpanForNode,
@@ -242,7 +241,6 @@ import {
242241
ModifierFlags,
243242
ModuleBlock,
244243
ModuleDeclaration,
245-
ModuleResolutionKind,
246244
Mutable,
247245
NamespaceExportDeclaration,
248246
Node,
@@ -279,6 +277,7 @@ import {
279277
setValueDeclaration,
280278
ShorthandPropertyAssignment,
281279
shouldPreserveConstEnums,
280+
shouldResolveJsRequire,
282281
SignatureDeclaration,
283282
skipParentheses,
284283
sliceAfter,
@@ -3525,7 +3524,7 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
35253524
if (!isBindingPattern(node.name)) {
35263525
const possibleVariableDecl = node.kind === SyntaxKind.VariableDeclaration ? node : node.parent.parent;
35273526
if (isInJSFile(node) &&
3528-
getEmitModuleResolutionKind(options) !== ModuleResolutionKind.Bundler &&
3527+
shouldResolveJsRequire(options) &&
35293528
isVariableDeclarationInitializedToBareOrAccessedRequire(possibleVariableDecl) &&
35303529
!getJSDocTypeTag(node) &&
35313530
!(getCombinedModifierFlags(node) & ModifierFlags.Export)

src/compiler/checker.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -940,6 +940,7 @@ import {
940940
ShorthandPropertyAssignment,
941941
shouldAllowImportingTsExtension,
942942
shouldPreserveConstEnums,
943+
shouldResolveJsRequire,
943944
Signature,
944945
SignatureDeclaration,
945946
SignatureFlags,
@@ -4006,7 +4007,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
40064007
const hasDefaultOnly = isOnlyImportedAsDefault(specifier);
40074008
const hasSyntheticDefault = canHaveSyntheticDefault(file, moduleSymbol, dontResolveAlias, specifier);
40084009
if (!exportDefaultSymbol && !hasSyntheticDefault && !hasDefaultOnly) {
4009-
if (hasExportAssignmentSymbol(moduleSymbol) && !(getAllowSyntheticDefaultImports(compilerOptions) || getESModuleInterop(compilerOptions))) {
4010+
if (hasExportAssignmentSymbol(moduleSymbol) && !allowSyntheticDefaultImports) {
40104011
const compilerOptionName = moduleKind >= ModuleKind.ES2015 ? "allowSyntheticDefaultImports" : "esModuleInterop";
40114012
const exportEqualsSymbol = moduleSymbol.exports!.get(InternalSymbolName.ExportEquals);
40124013
const exportAssignment = exportEqualsSymbol!.valueDeclaration;
@@ -4149,7 +4150,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
41494150
if (!isIdentifier(name)) {
41504151
return undefined;
41514152
}
4152-
const suppressInteropError = name.escapedText === InternalSymbolName.Default && !!(compilerOptions.allowSyntheticDefaultImports || getESModuleInterop(compilerOptions));
4153+
const suppressInteropError = name.escapedText === InternalSymbolName.Default && allowSyntheticDefaultImports;
41534154
const targetSymbol = resolveESModuleSymbol(moduleSymbol, moduleSpecifier, /*dontResolveAlias*/ false, suppressInteropError);
41544155
if (targetSymbol) {
41554156
if (name.escapedText) {
@@ -9204,7 +9205,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
92049205
// If `target` refers to a shorthand module symbol, the name we're trying to pull out isn;t recoverable from the target symbol
92059206
// In such a scenario, we must fall back to looking for an alias declaration on `symbol` and pulling the target name from that
92069207
let verbatimTargetName = isShorthandAmbientModuleSymbol(target) && getSomeTargetNameFromDeclarations(symbol.declarations) || unescapeLeadingUnderscores(target.escapedName);
9207-
if (verbatimTargetName === InternalSymbolName.ExportEquals && (getESModuleInterop(compilerOptions) || compilerOptions.allowSyntheticDefaultImports)) {
9208+
if (verbatimTargetName === InternalSymbolName.ExportEquals && allowSyntheticDefaultImports) {
92089209
// target refers to an `export=` symbol that was hoisted into a synthetic default - rename here to match
92099210
verbatimTargetName = InternalSymbolName.Default;
92109211
}
@@ -34163,7 +34164,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
3416334164
}
3416434165

3416534166
// In JavaScript files, calls to any identifier 'require' are treated as external module imports
34166-
if (isInJSFile(node) && getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.Bundler && isCommonJsRequire(node)) {
34167+
if (isInJSFile(node) && shouldResolveJsRequire(compilerOptions) && isCommonJsRequire(node)) {
3416734168
return resolveExternalModuleTypeByLiteral(node.arguments![0] as StringLiteral);
3416834169
}
3416934170

src/compiler/program.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ import {
287287
setParentRecursive,
288288
setResolvedModule,
289289
setResolvedTypeReferenceDirective,
290+
shouldResolveJsRequire,
290291
skipTrivia,
291292
skipTypeChecking,
292293
some,
@@ -3211,8 +3212,7 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
32113212
collectModuleReferences(node, /*inAmbientModule*/ false);
32123213
}
32133214

3214-
// `require` has no special meaning in `--moduleResolution bundler`
3215-
const shouldProcessRequires = isJavaScriptFile && getEmitModuleResolutionKind(options) !== ModuleResolutionKind.Bundler;
3215+
const shouldProcessRequires = isJavaScriptFile && shouldResolveJsRequire(options);
32163216
if ((file.flags & NodeFlags.PossiblyContainsDynamicImport) || shouldProcessRequires) {
32173217
collectDynamicImportOrRequireCalls(file);
32183218
}

src/compiler/utilities.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8438,6 +8438,12 @@ export function moduleResolutionSupportsPackageJsonExportsAndImports(moduleResol
84388438
|| moduleResolution === ModuleResolutionKind.Bundler;
84398439
}
84408440

8441+
/** @internal */
8442+
export function shouldResolveJsRequire(compilerOptions: CompilerOptions): boolean {
8443+
// `bundler` doesn't support resolving `require`, but needs to in `noDtsResolution` to support Find Source Definition
8444+
return !!compilerOptions.noDtsResolution || getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.Bundler;
8445+
}
8446+
84418447
/** @internal */
84428448
export function getResolvePackageJsonExports(compilerOptions: CompilerOptions) {
84438449
const moduleResolution = getEmitModuleResolutionKind(compilerOptions);
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// === goToSourceDefinition ===
2+
// === /node_modules/react/cjs/react.production.min.js ===
3+
// 'use strict';exports.[|{| defId: 0 |}useState|]=function(a){};exports.version='16.8.6';
4+
5+
// === /node_modules/react/cjs/react.development.js ===
6+
// 'use strict';
7+
// if (process.env.NODE_ENV !== 'production') {
8+
// (function() {
9+
// function useState(initialState) {}
10+
// exports.[|{| defId: 1 |}useState|] = useState;
11+
// exports.version = '16.8.6';
12+
// }());
13+
// }
14+
15+
// === /index.ts ===
16+
// import { /*GOTO SOURCE DEF*/useState } from 'react';
17+
18+
// === Details ===
19+
[
20+
{
21+
"defId": 0,
22+
"containerKind": "",
23+
"containerName": "",
24+
"kind": "",
25+
"name": ""
26+
},
27+
{
28+
"defId": 1,
29+
"containerKind": "",
30+
"containerName": "",
31+
"kind": "",
32+
"name": ""
33+
}
34+
]
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/// <reference path="../fourslash.ts" />
2+
3+
// @Filename: /tsconfig.json
4+
//// { "compilerOptions": { "module": "esnext", "moduleResolution": "bundler" } }
5+
6+
// @Filename: /node_modules/react/package.json
7+
//// { "name": "react", "version": "16.8.6", "main": "index.js" }
8+
9+
// @Filename: /node_modules/react/index.js
10+
//// 'use strict';
11+
////
12+
//// if (process.env.NODE_ENV === 'production') {
13+
//// module.exports = require('./cjs/react.production.min.js');
14+
//// } else {
15+
//// module.exports = require('./cjs/react.development.js');
16+
//// }
17+
18+
// @Filename: /node_modules/react/cjs/react.production.min.js
19+
//// 'use strict';exports./*production*/useState=function(a){};exports.version='16.8.6';
20+
21+
// @Filename: /node_modules/react/cjs/react.development.js
22+
//// 'use strict';
23+
//// if (process.env.NODE_ENV !== 'production') {
24+
//// (function() {
25+
//// function useState(initialState) {}
26+
//// exports./*development*/useState = useState;
27+
//// exports.version = '16.8.6';
28+
//// }());
29+
//// }
30+
31+
// @Filename: /index.ts
32+
//// import { [|/*start*/useState|] } from 'react';
33+
34+
verify.baselineGoToSourceDefinition("start");

0 commit comments

Comments
 (0)