-
Notifications
You must be signed in to change notification settings - Fork 15
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
Isolated declarations enums #118
Isolated declarations enums #118
Conversation
8cc09a1
to
3403de2
Compare
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.
LGTM with some nits
@@ -122,13 +118,39 @@ export function createEmitDeclarationResolver(file: SourceFile): IsolatedEmitRes | |||
} | |||
return undefined; | |||
} | |||
|
|||
function resolveEntityName(location: Node, node: Expression, meaning: SymbolFlags): EmitDeclarationSymbol | undefined { |
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.
Shouldn't this be resolveEntityName(location: EnumDeclaration, ...
?
And on the same point, I feel using a specific name for the variable would make more sense (e.g. enumDecl).
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.
This will actually be moved in #120 and be used in the more generic sense of resolving any entity name in any location.
return undefined; | ||
} | ||
} | ||
function isTargetEnumDeclaration(target: Expression, location: EnumDeclaration) { |
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.
Nit: Technically you're checking whether the target expression is an enum member, not a declaration.
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.
isExpressionMemberOfEnum
is more descriptive I think
Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>
3403de2
to
da557b0
Compare
No description provided.