Skip to content
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

Infer to partially homomorphic types (such as Pick<T, K>) #29787

Merged
merged 7 commits into from
Feb 11, 2019

Conversation

ahejlsberg
Copy link
Member

With this PR we improve type inference for partially homomorphic types such as Pick<T, K>. Now, when inferring to a mapped type { [P in K]: X }, where K is a type parameter K extends keyof T, we make the same inferences as we would for a homomorphic mapped type { [P in keyof T]: X }.

Fixes #29765.

// enables us to make meaningful inferences when the target is a Pick<T, K>). Otherwise, infer
// from a union of the property types in the source to the template type X.
const extendedConstraint = getConstraintOfType(constraintType);
if (extendedConstraint && extendedConstraint.flags & TypeFlags.Index) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably consider handling unions of indexes here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically for along the lines of:

declare function f20<A, B, K extends keyof A | keyof B>(obj: Pick<A & B, K>): T;

which instantiable of the original f20 could easily produce.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't do that for ordinary homomorphic types so I'm not sure why we'd do that here. Also, not clear how we would divide the inferences between A and B.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do do that for ordinary homomorphic mapped types. It's why constraintType is passed into inferFromMappedTypeConstraint.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like, in this inference function we break down keyof A | keyof B, and we break down (T extends keyof A) | (U extends keyof B), I don't see why we would not also break down T extends (keyof A | keyof B).

Copy link
Member

@weswigham weswigham Feb 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-                     if (extendedConstraint && extendedConstraint.flags & TypeFlags.Index) {
-                         inferToHomomorphicMappedType(source, target, <IndexType>extendedConstraint);
-                         return true;
+                     if (extendedConstraint) {
+                         return inferFromMappedTypeConstraint(source, target, extendedConstraint);

maybe? Probably shouldn't return, since making inferences directly to the constrained type parameter as well as the things referenced in its constraint should both be possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you're getting at. We really need to repeat the whole process for the constraint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants