-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Add index signature for anonymous object literal type #37903
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
Add index signature for anonymous object literal type #37903
Conversation
ping @weswigham because I believe you have thought a lot about index signatures. |
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.
We should implement the new behavior within getPropertyTypeForIndexType
, rather than checkElementAccessExpression
- specifically if the accessFlags
are for Writing, and the originalObjectType
passes isObjectTypeWithInferableIndex
, and we failed to lookup a property or index signature type. This should mirror the checks we do in comparisons, I think.
@weswigham I'd appreciate that you'd let me know any test cases I should add, if any! |
443ad78
to
2ce92c4
Compare
dccfdbb
to
8ea849b
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.
Whew, sorry for the huge delay - guess I was waiting for a response from @RyanCavanaugh that never came. I lieu of that, this looks broadly good, but I still have two small comments before this seems good to merge.
tests/baselines/reference/augmentedTypeBracketAccessIndexSignature.errors.txt
Outdated
Show resolved
Hide resolved
26a35bc
to
6eb7900
Compare
50c9007
to
73acd03
Compare
73acd03
to
5ae218d
Compare
@weswigham Looking at your last review, and the subsequent commits, I think this is ready to merge once the 4.1 RC is out. Does that sound right to you? |
Yeah, this looks good now. @sandersn do you want to merge whenever you feel is appropriate? |
The PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
Fixes #14951
Related to #26710