From 449b6e6df3eaf5bdd29c13873ddaacb4c893ac14 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Mar 2023 13:16:21 -0700 Subject: [PATCH 1/8] Fix Go To Source Definition in --moduleResolution bundler --- src/compiler/checker.ts | 7 ++-- .../fourslash/server/goToSource15_bundler.ts | 34 +++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 tests/cases/fourslash/server/goToSource15_bundler.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 4cfc2ed482612..16a37f5a60b30 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4149,7 +4149,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 && !!(getAllowSyntheticDefaultImports(compilerOptions)); const targetSymbol = resolveESModuleSymbol(moduleSymbol, moduleSpecifier, /*dontResolveAlias*/ false, suppressInteropError); if (targetSymbol) { if (name.escapedText) { @@ -9204,7 +9204,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 && getAllowSyntheticDefaultImports(compilerOptions)) { // target refers to an `export=` symbol that was hoisted into a synthetic default - rename here to match verbatimTargetName = InternalSymbolName.Default; } @@ -34163,7 +34163,8 @@ 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) && (getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.Bundler || compilerOptions.noDtsResolution) && isCommonJsRequire(node)) { + // `bundler` doesn't support resolving `require`, but needs to in `noDtsResolution` to support Find Source Definition return resolveExternalModuleTypeByLiteral(node.arguments![0] as StringLiteral); } diff --git a/tests/cases/fourslash/server/goToSource15_bundler.ts b/tests/cases/fourslash/server/goToSource15_bundler.ts new file mode 100644 index 0000000000000..8a8433db4e926 --- /dev/null +++ b/tests/cases/fourslash/server/goToSource15_bundler.ts @@ -0,0 +1,34 @@ +/// + +// @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"); From 1fddb2afeced81dd603f14f99f6896590fa3d46b Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Mar 2023 13:18:55 -0700 Subject: [PATCH 2/8] Accept baseline --- .../goToSource15_bundler.baseline.jsonc | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 tests/baselines/reference/goToSource15_bundler.baseline.jsonc diff --git a/tests/baselines/reference/goToSource15_bundler.baseline.jsonc b/tests/baselines/reference/goToSource15_bundler.baseline.jsonc new file mode 100644 index 0000000000000..36768c8dcf886 --- /dev/null +++ b/tests/baselines/reference/goToSource15_bundler.baseline.jsonc @@ -0,0 +1,22 @@ +// === 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'); +// }|] + +// === /index.ts === +// import { /*GOTO SOURCE DEF*/useState } from 'react'; + + // === Details === + [ + { + "containerKind": "", + "containerName": "", + "kind": "", + "name": "" + } + ] \ No newline at end of file From 8c81daf15187d0e816cec7de0596fbd54b1c1391 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Mar 2023 13:28:52 -0700 Subject: [PATCH 3/8] Actually fix it --- src/compiler/binder.ts | 3 +- src/compiler/checker.ts | 4 +-- src/compiler/program.ts | 4 +-- src/compiler/utilities.ts | 6 ++++ .../goToSource15_bundler.baseline.jsonc | 28 +++++++++++++------ 5 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 1ca6187ac39a4..1723c954b5db9 100644 --- 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 index 16a37f5a60b30..94f9d84f78b02 100644 --- 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 index cf596bccbedaf..7186a72480cc6 100644 --- 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 index c43eb48832023..bf657d5a15dba 100644 --- 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 index 36768c8dcf886..dc233e8fa79f4 100644 --- a/tests/baselines/reference/goToSource15_bundler.baseline.jsonc +++ b/tests/baselines/reference/goToSource15_bundler.baseline.jsonc @@ -1,12 +1,16 @@ // === 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'; @@ -14,6 +18,14 @@ // === Details === [ { + "defId": 0, + "containerKind": "", + "containerName": "", + "kind": "", + "name": "" + }, + { + "defId": 1, "containerKind": "", "containerName": "", "kind": "", From e8f71208660dc30c98be6da781c3ee7a8b431c96 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Mar 2023 13:42:01 -0700 Subject: [PATCH 4/8] Remove unnecessary function calls --- src/compiler/checker.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 94f9d84f78b02..745ac41305eaa 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4007,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; @@ -4150,7 +4150,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { if (!isIdentifier(name)) { return undefined; } - const suppressInteropError = name.escapedText === InternalSymbolName.Default && !!(getAllowSyntheticDefaultImports(compilerOptions)); + const suppressInteropError = name.escapedText === InternalSymbolName.Default && allowSyntheticDefaultImports); const targetSymbol = resolveESModuleSymbol(moduleSymbol, moduleSpecifier, /*dontResolveAlias*/ false, suppressInteropError); if (targetSymbol) { if (name.escapedText) { @@ -9205,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 && getAllowSyntheticDefaultImports(compilerOptions)) { + 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; } From 47ae29a1405e8d86bb2c5570a21b1885693f4ded Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Mar 2023 13:43:56 -0700 Subject: [PATCH 5/8] Fix bad edits --- src/compiler/binder.ts | 2 +- src/compiler/checker.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 1723c954b5db9..689a01dfdb920 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -3526,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) && - shouldResolveJsRequire(compilerOptions) && + shouldResolveJsRequire(options) && isVariableDeclarationInitializedToBareOrAccessedRequire(possibleVariableDecl) && !getJSDocTypeTag(node) && !(getCombinedModifierFlags(node) & ModifierFlags.Export) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 745ac41305eaa..5b405a639e2ca 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4150,7 +4150,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { if (!isIdentifier(name)) { return undefined; } - const suppressInteropError = name.escapedText === InternalSymbolName.Default && allowSyntheticDefaultImports); + const suppressInteropError = name.escapedText === InternalSymbolName.Default && allowSyntheticDefaultImports; const targetSymbol = resolveESModuleSymbol(moduleSymbol, moduleSpecifier, /*dontResolveAlias*/ false, suppressInteropError); if (targetSymbol) { if (name.escapedText) { From 4dd865567d4268d97d10305309ea436e91e4a249 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Mar 2023 14:43:51 -0700 Subject: [PATCH 6/8] Fix lint --- src/compiler/binder.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 689a01dfdb920..7e9eb617cc1ac 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -85,7 +85,6 @@ import { getContainingClass, getEffectiveContainerForJSDocTemplateTag, getElementOrPropertyAccessName, - getEmitModuleResolutionKind, getEmitScriptTarget, getEnclosingBlockScopeContainer, getErrorSpanForNode, From 2d362be3d110868c657f2ad81ff4710fb696a197 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Mar 2023 14:46:42 -0700 Subject: [PATCH 7/8] Fix lint --- src/compiler/program.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 7186a72480cc6..8acd63a08d724 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -287,6 +287,7 @@ import { setParentRecursive, setResolvedModule, setResolvedTypeReferenceDirective, + shouldResolveJsRequire, skipTrivia, skipTypeChecking, some, @@ -329,7 +330,6 @@ import { WriteFileCallbackData, writeFileEnsuringDirectories, zipToModeAwareCache, - shouldResolveJsRequire, } from "./_namespaces/ts"; import * as performance from "./_namespaces/ts.performance"; From aa4e9120f4d468e9f80f2b6b3bf80e6a6846ddd6 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Mar 2023 14:52:24 -0700 Subject: [PATCH 8/8] Fix lint --- src/compiler/binder.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 7e9eb617cc1ac..48836b20b04c6 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -241,7 +241,6 @@ import { ModifierFlags, ModuleBlock, ModuleDeclaration, - ModuleResolutionKind, Mutable, NamespaceExportDeclaration, Node,