-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Find Source Definition #48264
Merged
Merged
Find Source Definition #48264
Changes from 25 commits
Commits
Show all changes
49 commits
Select commit
Hold shift + click to select a range
81ec4fd
Prototype resolving to JS when go-to-def aliases all resolve to ambie…
andrewbranch 422d845
Add test infrastructure
andrewbranch 1b43352
Start fleshing out test coverage
andrewbranch 7aeb92f
Fix some go-to-def stuff
andrewbranch f2915e6
Finish lodash test case
andrewbranch 309c4bb
Make go-to-implementation never return ambient results
andrewbranch 381799d
Build new functionality into go-to-implementation
andrewbranch 0727702
Update baselines
andrewbranch ebeda0b
Two more test cases
andrewbranch 03ea413
Refine definition searches for unresolved imports
andrewbranch e5556b8
Revert "Build new functionality into go-to-implementation"
andrewbranch 6104a5c
Fix tests
andrewbranch bea3de0
Merge branch 'main' into go-to-js
andrewbranch 4e64659
Revert go-to-implementation changes
andrewbranch 1cb2ba6
Wow a bunch of code was unnecessary
andrewbranch 7e57890
Update baselines and go-to-def test
andrewbranch 43c01e2
Fix navigation on symbols that are not aliases but resolve through al…
andrewbranch 34c6cfd
Temporarily replace go-to-def with new command implementation
andrewbranch 7357593
Revert "Temporarily replace go-to-def with new command implementation"
andrewbranch d14e43d
Revert "Wow a bunch of code was unnecessary"
andrewbranch 4e1cf1c
Bring back some deleted code needed for a new test case
andrewbranch 52a79d5
Clean up a little
andrewbranch e011f2d
Rename more stuff
andrewbranch 05cfd27
Update test
andrewbranch b95f496
Update API baseline
andrewbranch b883898
Temporarily replace go-to-def with new command implementation
andrewbranch 373bca2
PR review fixes
andrewbranch c05adbd
Merge branch 'main' into go-to-js
andrewbranch 3a85dc8
Fix getTopMostDeclarationNamesInFile
andrewbranch 7dd9db8
Rename local
andrewbranch 0f3b29c
Use hash set
andrewbranch 234c139
Remove option from commandLineParser
andrewbranch 92ff999
Merge branch 'main' into go-to-js
andrewbranch 1698930
Keep noDtsResolution project around
andrewbranch 859ed79
Handle AuxiliaryProject kind in ScriptInfo getDefaultProject etc.
andrewbranch 7be215a
Do not run updateGraph in the background for AuxiliaryProject
andrewbranch 8f4e622
Don’t create auxiliary project outside of semantic mode
andrewbranch d633356
No-op on scheduled invalidation
andrewbranch b89b7cb
Add comments to unit test
andrewbranch a59024c
Sync compiler options to auxiliary project
andrewbranch 7f290bb
Fix case sensitivity
andrewbranch c5bddbe
Update extensionIsOk with new file extensions
andrewbranch 24c0d8a
PR feedback
andrewbranch f6bd9d8
Update API baseline
andrewbranch 2cb2048
Mark scheduleInvalidateResolutionsOfFailedLookupLocations internal
andrewbranch f9f4592
Use same heuristics on property accesses of loosely-resolvable aliase…
andrewbranch e302f56
Rename command, and no need to return the bound span
andrewbranch 3f15ed3
Update API baseline
andrewbranch f80c9ef
Merge branch 'main' into go-to-js
andrewbranch File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,7 +69,8 @@ namespace ts { | |
JavaScript, /** '.js' or '.jsx' */ | ||
Json, /** '.json' */ | ||
TSConfig, /** '.json' with `tsconfig` used instead of `index` */ | ||
DtsOnly /** Only '.d.ts' */ | ||
DtsOnly, /** Only '.d.ts' */ | ||
TsOnly, /** '.[cm]tsx?' but not .d.ts variants */ | ||
andrewbranch marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm sure you've already bike-shedded this, but I think I'd prefer "TypeScriptNoDts". |
||
} | ||
|
||
interface PathAndPackageId { | ||
|
@@ -1290,7 +1291,19 @@ namespace ts { | |
export function nodeModuleNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache?: ModuleResolutionCache, redirectedReference?: ResolvedProjectReference): ResolvedModuleWithFailedLookupLocations; | ||
/* @internal */ export function nodeModuleNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache?: ModuleResolutionCache, redirectedReference?: ResolvedProjectReference, lookupConfig?: boolean): ResolvedModuleWithFailedLookupLocations; // eslint-disable-line @typescript-eslint/unified-signatures | ||
export function nodeModuleNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache?: ModuleResolutionCache, redirectedReference?: ResolvedProjectReference, lookupConfig?: boolean): ResolvedModuleWithFailedLookupLocations { | ||
return nodeModuleNameResolverWorker(NodeResolutionFeatures.None, moduleName, getDirectoryPath(containingFile), compilerOptions, host, cache, lookupConfig ? tsconfigExtensions : (compilerOptions.resolveJsonModule ? tsPlusJsonExtensions : tsExtensions), redirectedReference); | ||
let extensions; | ||
if (lookupConfig) { | ||
extensions = tsconfigExtensions; | ||
} | ||
else if (compilerOptions.noDtsResolution) { | ||
extensions = [Extensions.TsOnly]; | ||
if (compilerOptions.allowJs) extensions.push(Extensions.JavaScript); | ||
if (compilerOptions.resolveJsonModule) extensions.push(Extensions.Json); | ||
} | ||
else { | ||
extensions = compilerOptions.resolveJsonModule ? tsPlusJsonExtensions : tsExtensions; | ||
} | ||
return nodeModuleNameResolverWorker(NodeResolutionFeatures.None, moduleName, getDirectoryPath(containingFile), compilerOptions, host, cache, extensions, redirectedReference); | ||
} | ||
|
||
function nodeModuleNameResolverWorker(features: NodeResolutionFeatures, moduleName: string, containingDirectory: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache: ModuleResolutionCache | undefined, extensions: Extensions[], redirectedReference: ResolvedProjectReference | undefined): ResolvedModuleWithFailedLookupLocations { | ||
|
@@ -1299,14 +1312,19 @@ namespace ts { | |
const failedLookupLocations: string[] = []; | ||
// conditions are only used by the node12/nodenext resolver - there's no priority order in the list, | ||
//it's essentially a set (priority is determined by object insertion order in the object we look at). | ||
const conditions = features & NodeResolutionFeatures.EsmMode ? ["node", "import", "types"] : ["node", "require", "types"]; | ||
if (compilerOptions.noDtsResolution) { | ||
conditions.pop(); | ||
} | ||
|
||
const state: ModuleResolutionState = { | ||
compilerOptions, | ||
host, | ||
traceEnabled, | ||
failedLookupLocations, | ||
packageJsonInfoCache: cache, | ||
features, | ||
conditions: features & NodeResolutionFeatures.EsmMode ? ["node", "import", "types"] : ["node", "require", "types"] | ||
conditions, | ||
}; | ||
|
||
const result = forEach(extensions, ext => tryResolve(ext)); | ||
|
@@ -1533,20 +1551,22 @@ namespace ts { | |
default: return tryExtension(Extension.Dts); | ||
} | ||
case Extensions.TypeScript: | ||
case Extensions.TsOnly: | ||
const useDts = extensions === Extensions.TypeScript; | ||
switch (originalExtension) { | ||
case Extension.Mjs: | ||
case Extension.Mts: | ||
case Extension.Dmts: | ||
return tryExtension(Extension.Mts) || tryExtension(Extension.Dmts); | ||
return tryExtension(Extension.Mts) || (useDts ? tryExtension(Extension.Dmts) : undefined); | ||
case Extension.Cjs: | ||
case Extension.Cts: | ||
case Extension.Dcts: | ||
return tryExtension(Extension.Cts) || tryExtension(Extension.Dcts); | ||
return tryExtension(Extension.Cts) || (useDts ? tryExtension(Extension.Dcts) : undefined); | ||
case Extension.Json: | ||
candidate += Extension.Json; | ||
return tryExtension(Extension.Dts); | ||
return useDts ? tryExtension(Extension.Dts) : undefined; | ||
default: | ||
return tryExtension(Extension.Ts) || tryExtension(Extension.Tsx) || tryExtension(Extension.Dts); | ||
return tryExtension(Extension.Ts) || tryExtension(Extension.Tsx) || (useDts ? tryExtension(Extension.Dts) : undefined); | ||
} | ||
case Extensions.JavaScript: | ||
switch (originalExtension) { | ||
|
@@ -1803,6 +1823,7 @@ namespace ts { | |
switch (extensions) { | ||
case Extensions.JavaScript: | ||
case Extensions.Json: | ||
case Extensions.TsOnly: | ||
packageFile = readPackageJsonMainField(jsonContent, candidate, state); | ||
break; | ||
case Extensions.TypeScript: | ||
|
@@ -1889,6 +1910,8 @@ namespace ts { | |
return extension === Extension.Json; | ||
case Extensions.TypeScript: | ||
return extension === Extension.Ts || extension === Extension.Tsx || extension === Extension.Dts; | ||
case Extensions.TsOnly: | ||
return extension === Extension.Ts || extension === Extension.Tsx; | ||
andrewbranch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
case Extensions.DtsOnly: | ||
return extension === Extension.Dts; | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you have a JS file like:
If one module is a subtype of the other, go-to-definition on an import/require of this file was previously going straight into the supertype module. By not eagerly doing subtype reduction here, the services are able to retain the information that the export branches off into these two separate modules. I traced this line of code all the way back to the beginning of JS support, long before
UnionReduction.Literal
existed. At the time it was introduced,getUnionType
did subtype reduction by default. Later, that default was flipped, and this call site opted into it as a mechanical migration of the default. Even later, the default becameUnionReduction.Literal
, but this call site was of course left unchanged since by that time it specified a non-default argument. No pre-existing tests were impacted by my changing it now, so my assumption is this does not actually have an intentional reason to do something other than the default.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea if you're correct, but I appreciate your thoroughness. 😄