We read every piece of feedback, and take your input very seriously.
To see all available qualifiers, see our documentation.
There was an error while loading. Please reload this page.
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
This PR implements polymorphic typing of this for classes and interfaces as inspired by #3694. The new features are:
this
This feature makes patterns such as as described here.
An example:
class A { foo() { return this; } } class B extends A { bar() { return this; } } var b: B; var x = b.foo().bar(); / Fluent pattern works, type of x is B
In a non-static member of a class or interface, this in a type position refers to the type of this. For example:
class Entity { clone(): this { / Code to clone Entity } equals(other: this): boolean { / Code to compare Entity with another Entity } }
Each subclass of the above Entity will have a clone method that returns an instance of the subclass, and an equals method that takes another instance of the subclass.
Entity
clone
equals
The this type is a subtype of and assignable to the instance type of the containing class or interface, but not vice-versa (because this might actually be a subclass). That is a breaking change, and certain code patterns that previously compiled may now need an extra type annotation:
class A { getInstance() { / Should be getInstance(): A return this; } } class B extends A { getInstance() { return new B(); } }
The example above now errors because the inferred return type of getInstance in A is this and the inferred type of getInstance in B is B, which is not assignable to this. The fix is to add a return type annotation for getInstance in A.
getInstance
A
B
The polymorphic this type is implemented by providing every class and interface with an implied type parameter that is constrained to the containing type itself (except when the compiler can can tell that this is never referenced within the class or interface). That in particular turns a lot of previously non-generic classes into generic equivalents, causing more symbols and types to be created due to generic instantiation. The observed cost in batch compile time ranges from 0% in code that uses no classes to 6-7% in very class-heavy code.
Note that this PR specifically doesn't aim to implement other parts of #3694 such as this type annotations for functions. Those will be covered by other PRs if we choose to implement them.
Sorry, something went wrong.
Polymorphic "this" type
9438a4b
Accepting new baselines
89ea067
Commenting out broken fourslash tests
06a250e
@jbondc Agreed, it might be good to show which class a particular this type belongs to in error messages and information produced by the language service.
👏
Merge branch 'master' into polymorphicThisType
285483d
Conflicts: src/compiler/diagnosticInformationMap.generated.ts
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
How does this handle the case where an interface extends a class that uses this ?
extends
class
If one of the base types is a class (i.e. not an interface) we simply return true (because every class has a this type). We only return false from this function if the interface itself doesn't reference this, if every base type is an interface, and if no base interface has a this type.
Need to un-comment the fourslash tests that were edited
@mhegazy is looking at fixes for the failing fourslash tests.
Ideally these should not fail right? But only on instantiation.
No, they should fail. The first declaration infers type this for v, the second declaration specifies type MyGenericTestClass<T, U> for v. Those are not identical types, so error.
v
MyGenericTestClass<T, U>
Adding comments and addressing CR feedback
7acb9dd
@danquirk about this, and I think there are some issues with the meaning of this in a type literal. Take the following:
interface Thing { getResource(): { methodA(): this; methodB(): this; } }
In the current implementation, this refers to the type assignable to Thing; however, this doesn't accurately reflect the type of this when methodA is invoked off of its returned object. This could be very confusing for users because the intuition is that the method literally returns the expression this.
Thing
methodA
For either one semantics of the this type, a workaround exists to get the alternative semantics by extracting the resource type into its own interface.
However, if this refers to the enclosing type literal, then an easier fix exists by adding a type parameter to the method:
interface Thing { getResource<OwnerT extends Thing>(): { a(): OwnerT; b(): this; } }
I think that we should go with the semantics that most-closely matches the runtime: in this case, the this type should refer to the type returned by getResource().
getResource()
@DanielRosenwasser You're incorrect about the current behavior, your example is actually an error:
interface Thing { getResource(): { methodA(): this; / Error methodB(): this; / Error } }
The error reported is 'this' type is available only in a non-static member of a class or interface and I think that is the behavior we want. If at some point we decide to make it possible to reference this of a function or method (perhaps because the function declared a this type parameter as speculated about in #3694) the above would become valid, but for now it should be an error.
'this' type is available only in a non-static member of a class or interface
In order to reference an outer this in an object type literal you have to introduce a generic type and capture this as an argument:
interface Resource<T> { methodA(): T; methodB(): T; } interface Thing { getResource(): Resource<this>; }
Note, however, that you can capture the outer this in an object literal:
class A { foo() { return { x: this, f: () => this }; } }
In order to write down the type inferred for the above, you'd have to resort to a temporary type:
interface Foo<T> { x: T; f(): T; } class A { foo(): Foo<this> { return { x: this, f: () => this }; } }
@jbondc I think the this type and mixins are largely orthogonal issues. I'd take the discussion up on one of the many mixin suggestion threads we already have going on and keep this one focused to the semantics and implementation of the this type.
your example is actually an error
I actually was asking others about the implementation and it was our understanding that it was allowed; sorry about that. I didn't get the chance to try the branch out yesterday.
Note, however, that you can capture the outer this in an object literal: class A { foo() { return { x: this, f: () => this }; } }
But the .d.ts emit for that is currently not valid:
declare class A { foo(): { x: this; f: () => this; }; }
If I try to reuse that .d.ts file, I get:
.d.ts
foo.d.ts(3,12): error TS2526: 'this' type is available only in a non-static member of a class or interface. foo.d.ts(4,18): error TS2526: 'this' type is available only in a non-static member of a class or interface.
@mhegazy The .d.ts issue is tricky. Personally I think this is one of those situations where we should issue an error if a .d.ts is requested with guidance that a type annotation is required.
Maybe clarify that this is not a built-in type keyword.
As I understand, polymorphic this type will greatly simplify the following code:
/ interface Cloneable<T extends Cloneable<T>> { interface Cloneable<T extends Cloneable<any> { clone(): T; }
turns it into
interface Cloneable { clone(): this; }
It's cool!
However It looks like that it solves only a half of a problem (ok, let it be 90% in common practices)
Am I right, that the system
/ interface Vertex<V extends Vertex<V,E>, E extends Edge<V,E>> { interface Vertex<V extends Vertex<any,any>, E extends Edge<any,any> { incoming: E[]; outgoing: E[]; } /interface Edge<V extends Vertex<V,E>, E extends Edge<V,E>> { interface Edge<V extends Vertex<any,any>, E extends Edge<any,any> { from: V; to: V; } class City extends Vertex<City, Road> { constructor(public name: string){ this.incoming = []; this.outgoing = []; } } class Road extends Edge<City, Road> { constructor(public distance: number, public from: City, public to: City) { this.from.outgoing.push(this); this.to.incoming.push(this); } } var LA = new City('Los Angeles'); var SF = new City('San Francisco'); var hyperloop = [new Road(558.68, LA, SF), new Road(558.68, SF, LA)] console.log(hyperloop[0].from.outgoing[0].to.name) / stronly typed
still could not benefit from this type without introducing high order types into TypeScript ?
I could imagine the following implementation
/ interface Vertex<E<T> extends Edge<T>> -or- interface Vertex<E extends Edge> { incomming: E<this>[]; outgoing: E<this>; } / interface Edge<V<T> extends Vertex<T>> -or- interface Edge<V extends Vertex> { from: V<this>; to: V<this>; } class City extends Vertex<Road> { ... } class Road extends Edge<City> { ... }
where for high order types the constraint <G1 extends G2> could be formulated as
<G1 extends G2>
G1<T> extends G2<T> recursively for every T
G1<T> extends G2<T>
Am I right? Or there is a simpler solution that I've missed?
I mean that this type alone solves only direct type recursion problem. Mutually recursive constraints are still not expressible. @ahejlsberg ?
@Artazor Correct, the polymorphic this helps in the simple (and common) case, but the general solution likely requires higher-order types of some form since the dependent types can't otherwise refer to each other's this.
🎉 👍
chore(typescript) update to latest. Mostly for microsoft/TypeScript#4910
8f752a6
should this also support this syntax?
interface A<T> { } interface B<T extends A<T>> { }
#2225
This seems to cause the following issue. In the example below in the getProperty() method, style is assigned to this. This used to be able to infer the type Style, but the latest code including this pull request does not, and the assignment to style at the end of the while loop throws an error, "Type 'Style' is not assignable to type 'this'.
getProperty()
Is this expected?
class Style { public property: number; public basedOn: string; /** if property is null, get from basedOn */ public getProperty(): number { var style = this; / used to infer type of Style while (style !== null) { if (style.property) return style.property; / get style referenced by basedOn style = style.basedOn === null ? null : getStyle(style.basedOn); } return null; } }
@mjohnsonengr this is the intended behavior. If there were a subclass of Style, the result of getStyle would be unassignable to a variable of type this (i.e. attempting to assign a Style to a SuperCoolStyle during a polymorphic invocation of getProperty).
Style
getStyle
SuperCoolStyle
getProperty
I'd recommend writing var style: Style = this; - that will solve the issue
var style: Style = this;
Nevermind, saw the referenced issue, #5056
Ahh, but thank your for your reply @RyanCavanaugh! :)
Deserialize
so what should I do to use this instead of state below
function NavigatableRecordCreator(defaultValues: { [key: string]: any; }, name?: string) { abstract class NavigatableRecord<P extends NavigatableRecord<P> extends Record(defaultValues, name) { SetValue<T>(fn: (x: NavigableObject<P>) => NavigableObject<T>, value: T, state: P) { return this.setIn(fn(new NavigableObject<P>(state).getPath(), value) / can I use this instead of state here } } return NavigatableRecord; }
wrote:
Each subclass of the above Entity will have a clone method that returns an instance of the subclass, and an equals method that takes another instance of the subclass. The this type is a subtype of and assignable to the instance type of the containing class or interface, but not vice-versa (because this might actually be a subclass).
The this type is a subtype of and assignable to the instance type of the containing class or interface, but not vice-versa (because this might actually be a subclass).
As #3694, how can polymorphic clone(): this be implemented if it is not legal to assign an instance of the subclass to this??? It seems instead the compiler will need to deduce that inheritance from Entity types clone() as an abstract method even though the base has an implementation, so as to insure that each subclass provides a unique implementation.
clone(): this
clone()
Successfully merging this pull request may close these issues.