Fixed expando functions with symbol-only properties #54726
Conversation
src/compiler/checker.ts
Outdated
| let indexInfos: IndexInfo[] | undefined; | ||
| if (symbol.exports) { |
There was a problem hiding this comment.
From what I understand symbol.exports only are only meant to keep properties that pass isTypeUsableAsPropertyName test and this symbol table is lazily created. Since symbols don't pass this test there was a chance for the symbol.exports table to not be created at all.
This patch essentially just grabs getExportsOfSymbol earlier.
There was a problem hiding this comment.
There is also still a question if this should be an error in JS files - since that PR doesn't change the behavior in JS. I'm not sure what's expected here though since both expando functions and JS files have their own unique rules
There was a problem hiding this comment.
The same code should work in JS. I'm guessing this change does that--can you add a test for it?
There was a problem hiding this comment.
I added the JS-oriented test case (with types declared in a TS file since I have no idea how to declare a computed property in JSDoc). I also have no idea what I had in mind here when posting that comment - this code here isn't targeting TS files (or non-JS files or anything like that) 😅
Andarist
force-pushed
the
fix/expando-functions-with-symbols
branch
from
cb60cc8 to
761d637
Compare
There was a problem hiding this comment.
Needs a test for JS. Also, I need to sit down with an editor and figure out if moving the getExportsOfSymbol call will hurt circularity prevention. At least, that's what I remember as the reason for let members = emptySymbols; maybe it was just a convenient initialiser.
src/compiler/checker.ts
Outdated
| let indexInfos: IndexInfo[] | undefined; | ||
| if (symbol.exports) { |
There was a problem hiding this comment.
The same code should work in JS. I'm guessing this change does that--can you add a test for it?
fixes #54220