findAllReferences/rename: Search in all open projects #25648
Conversation
ghost
force-pushed
the
findAllRefs_upstream
branch
from
366568c to
0265fff
Compare
src/compiler/program.ts
Outdated
| @@ -738,6 +738,10 @@ namespace ts { | |||
| getDiagnosticsProducingTypeChecker, | |||
| getCommonSourceDirectory, | |||
| emit, | |||
| getDeclarationEmitPath: fileName => { | |||
| const file = getSourceFile(fileName); | |||
There was a problem hiding this comment.
why does this need to be on the program? getDeclarationEmitOutputFilePath is already available outside this closure?
There was a problem hiding this comment.
getEmitHost() isn't though. That function appears to need host.getCompilerOptions, host.getCurrentDirectory, host.getCommonSourceDirectory, and host.getCanonicalFileName. getCurrentDirectory and getCanonicalFileName can be answered without a Program, but I don't think the other two can.
There was a problem hiding this comment.
getCurrentDirectory and getCanonicalFileName should be there on project and can be answered from there.
There was a problem hiding this comment.
That still leaves getCompilerOptions and getCommonSourceDirectory unfortunately.
There was a problem hiding this comment.
getCompilerOptions is also present on project and getCommonSourceDirectory can be added as a private method to the project that redirects to the program ?
src/server/session.ts
Outdated
| combineProjectOutputWorker( | ||
| projects, | ||
| defaultProject, | ||
| { fileName: "<arbitrary>", position: 0 }, // Location is ignored by callback |
There was a problem hiding this comment.
Why cant you pass null instead ? If location is ignored in combineProjectOutputWorker Dont think you should have need to create default value just for passing sake.
| projectAndLocation: ProjectAndLocation, | ||
| function getDefinitionInProject(definition: sourcemaps.SourceMappableLocation | undefined, definingProject: Project, project: Project): sourcemaps.SourceMappableLocation | undefined { | ||
| if (!definition || project.containsFile(toNormalizedPath(definition.fileName))) return definition; | ||
| const mappedDefinition = definingProject.getLanguageService().getSourceMapper().tryGetGeneratedLocation(definition); |
There was a problem hiding this comment.
You also want to check if definingProject.languageServiceEnabled
There was a problem hiding this comment.
That comes from this.getDefaultProject, shouldn't it always be enabled?
ghost
mentioned this pull request
ghost
force-pushed
the
findAllRefs_upstream
branch
from
b8f9274 to
bd64c07
Compare
|
@sheetalkamat Please re-review |
|
@Andy-MS please port to release-3.0 as well |
* findAllReferences/rename: Search in all open projects * Avoid needing a dummy location when location is unused * Remove Program#getDeclarationEmitPath * Only iterate over enabled projects
ghost
mentioned this pull request
Previously if a project A depended on a project B, we would find references in both only if the reference request started at A. Now the request can start in B, and we will find references in A as long project A has been opened.