-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Revert "Always substitute indexed generic mapped type when getting constraint from indexed access" #57202
Conversation
…nstraint from indexed access (microsoft#53066)" This reverts commit abb4052.
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
@@ -14230,7 +14230,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
} | |||
|
|||
function getConstraintFromIndexedAccess(type: IndexedAccessType) { | |||
if (isMappedTypeGenericIndexedAccess(type) || isGenericMappedType(type.objectType)) { |
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.
while my comment here can be partially true - so far I couldn't find the situation in which this extra check would benefit anything (after adjustments) since getSimplifiedIndexedAccessType
already performs this substitution when it's safe to do so
@@ -57,5 +83,5 @@ type ObjectWithUnderscoredKeys<K extends string> = { | |||
}; | |||
|
|||
function genericTest<K extends string>(objectWithUnderscoredKeys: ObjectWithUnderscoredKeys<K>, key: K) { | |||
const shouldBeTrue: true = objectWithUnderscoredKeys[`_${key}`]; | |||
const shouldBeTrue: true = objectWithUnderscoredKeys[`_${key}`]; // assignability fails here, but ideally should not |
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.
Those kinds of cases could always be OK if we could determine that the template type doesn't depend on the original K
. I don't think the compiler is equipped to check this sort of thing right now though.
@DanielRosenwasser I don't think this revert needs to be cherry-picked or anything, right? Since we'll sync the release branch with |
Yeah, it'll be in the RC |
reverts my own #53066 , fixes #53066 (comment)
cc @ahejlsberg @weswigham