Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Go To Source Definition in --moduleResolution bundler #53613

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ import {
getContainingClass,
getEffectiveContainerForJSDocTemplateTag,
getElementOrPropertyAccessName,
getEmitModuleResolutionKind,
getEmitScriptTarget,
getEnclosingBlockScopeContainer,
getErrorSpanForNode,
Expand Down Expand Up @@ -242,7 +241,6 @@ import {
ModifierFlags,
ModuleBlock,
ModuleDeclaration,
ModuleResolutionKind,
Mutable,
NamespaceExportDeclaration,
Node,
Expand Down Expand Up @@ -279,6 +277,7 @@ import {
setValueDeclaration,
ShorthandPropertyAssignment,
shouldPreserveConstEnums,
shouldResolveJsRequire,
SignatureDeclaration,
skipParentheses,
sliceAfter,
Expand Down Expand Up @@ -3525,7 +3524,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(options) &&
isVariableDeclarationInitializedToBareOrAccessedRequire(possibleVariableDecl) &&
!getJSDocTypeTag(node) &&
!(getCombinedModifierFlags(node) & ModifierFlags.Export)
Expand Down
9 changes: 5 additions & 4 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,7 @@ import {
ShorthandPropertyAssignment,
shouldAllowImportingTsExtension,
shouldPreserveConstEnums,
shouldResolveJsRequire,
Signature,
SignatureDeclaration,
SignatureFlags,
Expand Down Expand Up @@ -4006,7 +4007,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const hasDefaultOnly = isOnlyImportedAsDefault(specifier);
const hasSyntheticDefault = canHaveSyntheticDefault(file, moduleSymbol, dontResolveAlias, specifier);
if (!exportDefaultSymbol && !hasSyntheticDefault && !hasDefaultOnly) {
if (hasExportAssignmentSymbol(moduleSymbol) && !(getAllowSyntheticDefaultImports(compilerOptions) || getESModuleInterop(compilerOptions))) {
if (hasExportAssignmentSymbol(moduleSymbol) && !allowSyntheticDefaultImports) {
const compilerOptionName = moduleKind >= ModuleKind.ES2015 ? "allowSyntheticDefaultImports" : "esModuleInterop";
const exportEqualsSymbol = moduleSymbol.exports!.get(InternalSymbolName.ExportEquals);
const exportAssignment = exportEqualsSymbol!.valueDeclaration;
Expand Down Expand Up @@ -4149,7 +4150,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (!isIdentifier(name)) {
return undefined;
}
const suppressInteropError = name.escapedText === InternalSymbolName.Default && !!(compilerOptions.allowSyntheticDefaultImports || getESModuleInterop(compilerOptions));
const suppressInteropError = name.escapedText === InternalSymbolName.Default && allowSyntheticDefaultImports;
const targetSymbol = resolveESModuleSymbol(moduleSymbol, moduleSpecifier, /*dontResolveAlias*/ false, suppressInteropError);
if (targetSymbol) {
if (name.escapedText) {
Expand Down Expand Up @@ -9204,7 +9205,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// If `target` refers to a shorthand module symbol, the name we're trying to pull out isn;t recoverable from the target symbol
// In such a scenario, we must fall back to looking for an alias declaration on `symbol` and pulling the target name from that
let verbatimTargetName = isShorthandAmbientModuleSymbol(target) && getSomeTargetNameFromDeclarations(symbol.declarations) || unescapeLeadingUnderscores(target.escapedName);
if (verbatimTargetName === InternalSymbolName.ExportEquals && (getESModuleInterop(compilerOptions) || compilerOptions.allowSyntheticDefaultImports)) {
if (verbatimTargetName === InternalSymbolName.ExportEquals && allowSyntheticDefaultImports) {
// target refers to an `export=` symbol that was hoisted into a synthetic default - rename here to match
verbatimTargetName = InternalSymbolName.Default;
}
Expand Down Expand Up @@ -34163,7 +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 && isCommonJsRequire(node)) {
if (isInJSFile(node) && shouldResolveJsRequire(compilerOptions) && isCommonJsRequire(node)) {
return resolveExternalModuleTypeByLiteral(node.arguments![0] as StringLiteral);
}

Expand Down
4 changes: 2 additions & 2 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ import {
setParentRecursive,
setResolvedModule,
setResolvedTypeReferenceDirective,
shouldResolveJsRequire,
skipTrivia,
skipTypeChecking,
some,
Expand Down Expand Up @@ -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);
}
Expand Down
6 changes: 6 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
34 changes: 34 additions & 0 deletions tests/baselines/reference/goToSource15_bundler.baseline.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// === goToSourceDefinition ===
// === /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": "",
"name": ""
}
]
34 changes: 34 additions & 0 deletions tests/cases/fourslash/server/goToSource15_bundler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/// <reference path="../fourslash.ts" />

// @Filename: /tsconfig.json
//// { "compilerOptions": { "module": "esnext", "moduleResolution": "bundler" } }

// @Filename: /node_modules/react/package.json
//// { "name": "react", "version": "16.8.6", "main": "index.js" }

// @Filename: /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');
//// }

// @Filename: /node_modules/react/cjs/react.production.min.js
//// 'use strict';exports./*production*/useState=function(a){};exports.version='16.8.6';

// @Filename: /node_modules/react/cjs/react.development.js
//// 'use strict';
//// if (process.env.NODE_ENV !== 'production') {
//// (function() {
//// function useState(initialState) {}
//// exports./*development*/useState = useState;
//// exports.version = '16.8.6';
//// }());
//// }

// @Filename: /index.ts
//// import { [|/*start*/useState|] } from 'react';

verify.baselineGoToSourceDefinition("start");