-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Make exportSymbol public #17457
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
Make exportSymbol public #17457
Conversation
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 don't think we want to make exportSymbol
public as we generally pass it through getMergedSymbol
before using it ourselves (see getExportSymbolOfValueSymbolIfExported
). I'd prefer we added a checker.getExportSymbolOfSymbol
that is just a pointer to getExportSymbolOfValueSymbolIfExported
.
src/compiler/checker.ts
Outdated
@@ -138,6 +138,7 @@ namespace ts { | |||
node = getParseTreeNode(node, isExportSpecifier); | |||
return node ? getExportSpecifierLocalTargetSymbol(node) : undefined; | |||
}, | |||
getExportSymbolOfSymbol: getExportSymbolOfValueSymbolIfExported, |
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.
My apologies. getExportSymbolOfValueSymbolIfExported
will only get the export symbol if the symbol is a Value symbol, not a Type symbol.
Perhaps this might be more reliable:
function getExportSymbolOfSymbol(symbol: Symbol) {
return getMergedSymbol(symbol.exportSymbol || symbol);
}
Otherwise, the example in your comment in types.ts isn't strictly valid.
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.
It would be nice to have unit tests for this, though we don't currently have much in the way of unit tests for checker's public API surface.
#16742 changed the behavior of
getSymbolsInScope
to return local symbols and not exported symbols.This means that in:
The
getSymbolAtLocation
atT
inN.T
will return the exported symbol forT
.However,
getSymbolsInScope
at the same location returns the local symbol forT
.Tslint used to compare
getSymbolAtLocation
withgetSymbolsInScope
to determine when a namespace qualifier was unnecessary; this was broken whengetSymbolAtLocation
started returning local symbols. This was "fixed" by palantir/tslint#3052 which checks every declaration for equality, but a better solution would be to test for.exportSymbol
of the local symbol.We could instead add a
checker.getExportedSymbolOfLocalSymbol
method if you think that would make for a better API.