Allow global Symbol computed names during pseudochecker object literal serialization - #3297
Conversation
| export const symbolNamed = { | ||
| [Symbol.toStringTag]: "demo", | ||
| [Symbol.iterator]() { | ||
| return [1, 2, 3][Symbol.iterator](); |
There was a problem hiding this comment.
In Strada, this reports a slightly different error (TS playground):
Method must have an explicit return type annotation with --isolatedDeclarations.
| expression := e.Name().Expression() | ||
| if !ast.IsPrimitiveLiteralValue(expression, false) { | ||
| if !ast.IsPrimitiveLiteralValue(expression, false) && | ||
| !ch.isDefinitelyReferenceToGlobalSymbolObject(expression) { |
There was a problem hiding this comment.
I'm not super fond of coupling the pseudochecker with a checker's method like this but this is kinda the earliest moment at which this thing can be checked without doubling the work somewhere else.
I tried putting this in the nodebuilder but that had its own issues and was allowing all computed properties in the pseudochecker, just so the nodebuilder could reject them
There was a problem hiding this comment.
There was a problem hiding this comment.
I'd go so far as to say it's a bad idea that goes against the premise of the ID pseudochecker. The functionality needs to be implementable without a full check to be consistent with ID's goals.
There was a problem hiding this comment.
Fortunately, it seems like isDefinitelyReferenceToGlobalSymbolObject seems like it's not too bad to implement an equivalent outside the checker? You just need the binder's reference resolver instead.
There was a problem hiding this comment.
You just need the binder's reference resolver instead.
I actually had this on #4902 but deleted it; I should have remembered this thread, oops
8534285 to
611d271
Compare
Ryan Cavanaugh (RyanCavanaugh)
added
the
No linked issue
Jake Bailey (jakebailey)
added
the
Unmigrated PR
|
Thank you for contributing to the TypeScript native port! Development has moved from this repository back to the main microsoft/TypeScript repository. GitHub does not have PR transfer functionality, so we're closing this PR here. If this change is still relevant, please reopen it as a new pull request in See microsoft/typescript-go#4918 for more information about the move. |
4 participants
This change matches Strada's behavior more closely