Repository navigation
Annotation of this in function declarations #452
Description
Activity
Flow is (mostly) smart about this already without annotations. I'm not clear what this new bind syntax is (do you have a link to a spec?), but I imagine flow will support it the same way that flow currently supports
bind,call, andapply./* @flow */ function head(count) { return this.slice(0, count); } head.bind([]) // OK head.bind(1) // bad
I'm not clear what this new bind syntax is (do you have a link to a spec?)
Ah, sorry, forgot the link, fixed.
Flow is (mostly) smart about this already without annotations
Yes, but it would be nice to be able to annotate this instead, for documentational purposes (easier to understand at a glance and also allows for better error messages, as well as documentation generation), but also for standalone definitions.
Having thought about this a bit more, a simpler syntax would actually be to allow specifying
thisas a parameter, as so:function add (this : number, b : number) : number { return this + b; } function add (/*: this : number, */ b /*: number */) /*: number */ { return this + b; } /*: (this : number, b: number) => number */ function add (b) { return this + b; }
Agree that we should support
thisto be annotated as a parameter. This requires some parser work too, btw, sincethisis not a valid identifier everywhere.We need this whenever methods are exported, by the way. It's true Flow is good in inferring
thistypes, but it sucks that this information is lost across modules.Related: constructor functions can be annotated as of 618186a
Reacted by Joe Grund, Nicolas Jakob, Roma P., chocolateboy, Faris Masad, M. Walker Wells, Moritz Mahringer and andretshurotshkaHi guys! what about the following examples: microsoft/TypeScript#1985 (comment)
Are they too complex/ugly?Reacted by Michael Bøcker-Larsen and andretshurotshka+1
Pulling in some material from #968: an example, and a link to how Google Closure does this.
There's an error (marked with
<---) in this snippet which Flow could potentially catch:class RemoteFile { url: string; ... promiseXHR(xhr: XMLHttpRequest): Q.Promise<[any, Event]> { var url = this.url; var deferred = Q.defer(); xhr.addEventListener('load', function(e) { if (this.status >= 400) { deferred.reject(this.status + ' ' + this.statusText); } else { deferred.resolve([this.response, e]); } }); xhr.addEventListener('error', function(e) { deferred.reject(`Request for ${this.url} failed: ${this.status}`); // <--- }); this.numNetworkRequests++; xhr.send(); return deferred.promise; } }
The problem is that
thisinside theaddEventListenercallback refers to theXMLHttpRequestobject (I believe) and not theRemoteFileclass. The latter has aurlproperty but not astatusproperty, whereas the former hasstatusbut noturl. Clearly this code isn't right!Flow doesn't complain, though. It thinks
thishas typeanyin the event listener.Where possible, the DOM declarations should indicate the type of
thisin callbacks to prevent this sort of mistake.For example, Google's Closure Compiler provides an
@thisannotation. You can see an example of this in action in the declaration forArray.prototype.find.In case you land here before this +1 functionality is added, and you need types just for the fn body, just do something like this:
type Ctx = { foo: string, bar: number } myMethod() { const myCtx: Ctx = this; // do stuff with myCtx instead of this }
Reacted by Louis Roman and andretshurotshkaReacted by Vlad SydorenkoJust for reference, TS 2.0 landed with this feature last month. It special-cases the first parameter if its name is
this. I like that, reminds me of Python.function f(this: void) { // make sure `this` is unusable in this standalone function } interface UIElement { addClickListener(onclick: (this: void, e: Event) => void): void; }
An interesting note from the ticket when they were speccing it:
Yeah, the callback literally does "receive" this object as a parameter. That's how it works for all functions. It just happens that there are different rules for how you pass the value of this, it doesn't appear in the arguments array, and there's a different default value in non-strict mode. Otherwise it's basically just another argument.
Makes sense to me. If
babel-plugin-flow-strip-typesstripped the entire first parameter when its name isthis, it could work. Will likely require special-casing in ESLint etc.Reacted by Esa-Matti Suuronen, Thorsten Lünborg, Desmond Brand, Greg Thornton, Frizi, Katja Lutz, chocolateboy, ivan, Michael Bøcker-Larsen and Moritz MahringerThis already works in Flow:
function head (count: number) { (this: Array<number>); return this.slice(0, count); } head.call([1, 2, 3], 10); head.call(['123'], 20); // error
Do we really need new syntax for this?
However there are some bugs with the approach as this totally bugs out:
function head<T>(count: number): Array<T> { (this: Array<T>); return this.slice(0, count); } head.call([1, 2, 3], 10); head.call(['123'], 20);
@nmn Yes we do need.
It's for library declaration and use site completion. This is quite common for context injection.
function addListener<T: HTMLElement>(elem: T, func: (this: T) => void) { // implementation } addListener(htmlInputElement , function() { this.value // should have this completion })
Another usage is in library like Vue/backbone.
Vue.extend({ methods: { log() { this.instanceProperty // this is injected as Vue instance } } })
Without this annotation, it is impossible for library author to provide a good type declaration for automatic completion.
Reacted by kazuya kawaguchi, Eugene, Oscar N., Scott Bedard, Katja Lutz, Fernando H-T Goldáraz, chengchao, Nick Messing, tpines, Javier Chávarri and 9 moreI hope this feature is provided 🙏
Reacted by Scott Bedard, Herrington Darkholme, Katja Lutz, tpines, Masato Noguchi, Nick Messing, neotech-development, Sorin Iclanzan, Glebka Fokin, Loupasakis Kostas and 1 moreReacted by chocolateboy+1 for this feature
Reacted by Nick Messing, tpines, Andrei Glingeanu, Grzegorz Szeliga, neotech-development, sobolevn, Sorin Iclanzan, Loupasakis Kostas, Maxime Briziarelli, Louis Roman and 1 moreReacted by chocolateboyHas there been no progress on this yet?
@calebmer was looking for people to help implement it a while ago. I'm not sure if anyone started on it. I'd love to do it if I had more time.
Any updates on this? :)
@danvk It seems that you're using flow type version of
Q.Promise, is it? If so do you mind to share a link to flow typedQ.Pomiselibrary?@akoppela https://github.com/hammerlab/pileup.js/blob/0182b225205e52e1489727004d69e84442eafb88/lib/q.js but take note that these are ~3 years old and there may be better ones available elsewhere.
Reacted by Andrey Koppel and andretshurotshka- added a commit that references this issue
on Oct 13, 2018 Hey! I tried to implement it at #7807
Reacted by Desmond Brand and Sebastien GuillemotNote that
thisconstraints in classes can be emulated via$Calldeclare class O<T> { m(): $Call<(string => string), T>; m(): $Call<(number => number), T>; } declare var o1: O<number> const a1: number = o1.m() // ok const b1: string = o1.m() // error declare var o2: O<string> const a2: number = o2.m() // error const b2: string = o2.m() // ok declare var o3: O<{}> const a3 = o3.m() // error const b3 = o3.m() // error
^ flow try
class M<+State> { +s: State constructor(s: State) { this.s = s } to2: $Call<(1 => () => M<2>) & () => {...}, State> = () => { return new M(2) // ok } to3: $Call<(1 => () => M<3>) & () => {...}, State> = () => { return new M(2) // error } } const m1 = new M(1) const m2 = m1.to2() // ok const m3 = m2.to2() // error
^ flow try
declare class O<+T> { constructor(T): void; m(): $Call<<U>(Array<U>) => U, T> } const oarr = new O([1,2,3]) const n: number = oarr.m() // ok const s: string = oarr.m() // error declare class Ap<+T> { ap<A, B>(Ap<A>): $Call<(A => B) => Ap<B>, T> } const a = new Ap<number => string> const m: Ap<number> = a.ap(new Ap<3>) // error const d: Ap<string> = a.ap(new Ap<3>) // ok
^ flow try
declare class A { m(): $Call<B => 1, this>; n(): $Call<<U>(C<U>) => U, this>; } declare class B extends A {} declare class C<T> extends A {} const a = new A a.m() // error (not in this place tho) const b = new B b.m() // ok const c = new C<string>() const n0: string = c.n() // ok const n1: number = c.n() // error
^ flow try
Reacted by Sebastien Guillemot, soulofmischief and Michael Bøcker-LarsenWow, this is gold! Thank you @Bannerets!
Fixing
thiswas mentioned in the latest roadmap.Reacted by Ross AllenThe
thisannotation was released in 0.139.To use, add this to flowconfig:
[options] experimental.this_annot`= trueTo use:
function foo(this: {toString: (number) => string}, a: number): string { return this.toString(a); }
It is also usable at https://flow.org/try.
Reacted by HE Shi-Jun, Rentaro Matsukata and Kim Mu-hun
Currently it seems that you can only annotate the
thismagic reference by putting the function as a method in a class declaration.However, in the light of the new bind syntax proposal, it would make sense to be able to annotate functions that take in
thisas data, e.g.could be annotated something like: