Stop reassigning .valueDeclaration to avoid replacing earlier declarations with late ones#60857
Conversation
Andarist
changed the title
.valueDeclaration to avoid replacing earlier declarations with late ones
| symbol.declarations.push(member); | ||
| } | ||
| if (symbolFlags & SymbolFlags.Value) { | ||
| if (!symbol.valueDeclaration || symbol.valueDeclaration.kind !== member.kind) { |
There was a problem hiding this comment.
Was the original logic trying to preserve the somewhat-sketchy logic in the binder around which type of declaration "wins" out?
There was a problem hiding this comment.
It sure looks like it, but there's a separate utility function for that now: setValueDeclaration, which is more complicated*. Specifically, the kind check is only for namespaces there, which wouldn't apply to this property assignments.
I think the better fix to try is
if (symbolFlags & SymbolFlags.Value) {
setValueDeclaration(symbol, member)
}In other words, what the binder does in addDeclarationToSymbol.
*It also has a 3rd case for function property assignments in .d.ts files.
There was a problem hiding this comment.
I pushed out this change. Could you take another look?
| symbol.declarations.push(member); | ||
| } | ||
| if (symbolFlags & SymbolFlags.Value) { | ||
| if (!symbol.valueDeclaration || symbol.valueDeclaration.kind !== member.kind) { |
There was a problem hiding this comment.
It sure looks like it, but there's a separate utility function for that now: setValueDeclaration, which is more complicated*. Specifically, the kind check is only for namespaces there, which wouldn't apply to this property assignments.
I think the better fix to try is
if (symbolFlags & SymbolFlags.Value) {
setValueDeclaration(symbol, member)
}In other words, what the binder does in addDeclarationToSymbol.
*It also has a 3rd case for function property assignments in .d.ts files.
sandersn
left a comment
sandersn
left a comment
There was a problem hiding this comment.
I noticed this after ignoring it for far too long. Would you mind merging from main before this goes in?
github-project-automation
bot
moved this from Waiting on author
to Needs merge
in PR Backlog
|
@sandersn done |
5 participants
cc @sandersn
fixes #60590