Support find-all-references for default exports #13643
Conversation
ghost
force-pushed
the
find_all_refs_default
branch
from
7049625 to
83b3df9
Compare
| // Compute the meaning from the location and the symbol it references | ||
| const searchMeaning = getIntersectingMeaningFromDeclarations(getMeaningFromLocation(node), declarations); | ||
|
|
||
| const result: ReferencedSymbol[] = []; | ||
| // Maps from a symbol ID to the ReferencedSymbol entry in 'result'. | ||
| const symbolToIndex: number[] = []; |
There was a problem hiding this comment.
Moved these out of getReferencesInNode to avoid recomputing.
ghost
force-pushed
the
find_all_refs_default
branch
from
83b3df9 to
3d3e302
Compare
ghost
force-pushed
the
find_all_refs_default
branch
from
3d3e302 to
0ca4cb2
Compare
|
Note: The old behavior was to rename default imports globally if they shared the same name as the default export, otherwise to rename locally. The new behavior is to always rename default imports locally. (You can globally rename by renaming the export.) |
| @@ -3,4 +3,6 @@ | |||
| ////import [|a|] from "module"; | |||
| ////export { [|a|] }; | |||
|
|
|||
| verify.rangesAreRenameLocations(); | |||
There was a problem hiding this comment.
change the test name then?
There was a problem hiding this comment.
Oh, didn't mean to check in that change.
| @@ -1,7 +1,7 @@ | |||
| /// <reference path='fourslash.ts' /> | |||
|
|
|||
| // @Filename: B.ts | |||
| ////export default class /*1*/C { | |||
| ////export default class /*1*/[|C|] { | |||
There was a problem hiding this comment.
also add a test for import * as m from "B"; m.default; renaming should do nothing in this case.
There was a problem hiding this comment.
I'll have to change getRenameInfo for that. Created #13663.
|
We have the same problems with renamed exports/imports today as we do with default imports. our basic assumptions here are just wrong. we tried to expand the global find references logic to include modules, and just use the identity of the original declaration as a comparison point but we are still looking for a single name. we need to have a comprehensive solution for imports/exports in general. let's chat. |
| if (scope) { | ||
| result = []; | ||
| getReferencesInNode(scope, symbol, declaredName, node, searchMeaning, findInStrings, findInComments, result, symbolToIndex, implementations, typeChecker, cancellationToken); | ||
| getRefs(scope, declaredName); |
There was a problem hiding this comment.
the additonal indirection does not seem to be worth it.
| if (shorthandModuleSymbol) { | ||
| searchSymbols.push(shorthandModuleSymbol); | ||
| } | ||
| function isSearchedFor(symbol: Symbol): boolean { |
There was a problem hiding this comment.
i would switch this to a helper instead of a parameter and pass in searchSymbols.
ghost
mentioned this pull request
Fixes #11551