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

Mapped type and string index signature relations #17633

Merged
merged 3 commits into from
Aug 7, 2017

Conversation

ahejlsberg
Copy link
Member

With this PR a mapped type { [P in K]: T }, where K is generic, is related to a string index signature { [key: string]: U } if T is related to U. For example:

function f<K extends string>(x: { [key: string]: number }, y: Record<K, number>) {
    x = y;  // Ok
}

Fixes #14548. Also fixes an unintended effect of #17382 that would allow any generic mapped type to be assignable to a string index signature (because generic mapped types have no manifest properties they were being mistaken for empty object types).

@ahejlsberg
Copy link
Member Author

@rbuckton @sandersn I'd like to get this one into 2.5 RC so take a look.

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

One small nit about the test.

y = x; // Error
}

function f2<T, K extends string>(x: { [key: string]: T }, y: Record<string, T>) {
Copy link
Member

Choose a reason for hiding this comment

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

what's the point of this test? K isn't used.

Copy link
Member

Choose a reason for hiding this comment

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

I guess it's to assert that the key type of Record doesn't matter, only the mapped type's template type. Seems self-evident to me, but maybe it's worth a test.

Copy link
Member

Choose a reason for hiding this comment

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

To me it appears to be unused and unnecessary for the test. @ahejlsberg, is the type parameter K needed for this test?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, no need to have that type parameter. Will fix.

y = x; // Error
}

function f2<T, K extends string>(x: { [key: string]: T }, y: Record<string, T>) {
Copy link
Member

Choose a reason for hiding this comment

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

To me it appears to be unused and unnecessary for the test. @ahejlsberg, is the type parameter K needed for this test?

@ahejlsberg ahejlsberg merged commit aa0fc0b into master Aug 7, 2017
@ahejlsberg ahejlsberg deleted the indexSignatureMappedType branch August 7, 2017 21:17
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mapped types over keyof T should be subtypes of types with string index signatures
4 participants