Skip to content

Fix combineTypeMappers to use instantiateType #2824

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

Merged
merged 2 commits into from
Apr 20, 2015
Merged

Conversation

JsonFreeman
Copy link
Contributor

Fixes #1424.

When you have a chain of instantiated generics, like this:

interface Type<T1> {
    x: T1;
}
interface Type2<T2> {
    y: Type<T2>; // Replace T1 with T2 in Type
}
var v: Type2<string>; // Replace T2 with string in Type2

Eventually T1 has to be replaced with string in Type1. So to do that we combine / compose the two type mappers that we already have. The result is a type mapper that replaces T1 with T2, and then immediately replaces the resulting T2 with string.

This worked when the output of the first mapper was a type parameter (in the above example T2). However, the first mapper can output an arbitrary type, and it might not be appropriate to immediately apply the second mapper (this was the problem in #1424). In general, it is necessary to treat the application of the second mapper as an instantiation, and not simply apply the second mapper.

Another way of saying this is that a mapper must take a TypeParameter as a parameter, even though it can output any Type.

@CyrusNajmabadi
Copy link
Contributor

👍

JsonFreeman added a commit that referenced this pull request Apr 20, 2015
Fix combineTypeMappers to use instantiateType
@JsonFreeman JsonFreeman merged commit 02a480d into master Apr 20, 2015
@JsonFreeman JsonFreeman deleted the combineTypeMappers branch April 20, 2015 19:53
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 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.

Inheriting constructor through extends clause with a wrapped type argument does not work
3 participants