-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Proposal to improve static analysis for the thisArg of a function #1985
Comments
|
@Arnavion Thanks for identifying some of the issues with my proposal. Here are my solutions:
I hope this answers some of your questions. |
I would just like to say, for me at least, adding new syntax that further conflicts with React's JSX would be a bad idea. If the proposed syntax conflicts with the implementation of jsx-typescript over at https://github.com/fdecampredon/jsx-typescript/ then I think the syntax should change. (Look at facebook/react#759 for more info on TypeScript/JSX compatibly). |
If the proposed syntax conflicts with JSX (which it seems like it would, but this should be verified), here are some other possibilities:
The biggest advantages of the proposed syntax are:
For those reasons, I think 3 is the best replacement for the proposed syntax. I would also like to mention that I think this is a very important feature for the future of TypeScript, as many JS libraries rely on this-binding. The specific library where I found this problematic was @meteor-typescript. |
@jasonmeisel Your first advantage is incorrect, as angle brackets can be in a function signature. Angle brackets are used for TypeScript generics. Example: (view in TypeScript playground)
EDIT: A much better example of generic function signatures are the underscore / LoDash type definitions: https://github.com/borisyankov/DefinitelyTyped/blob/master/lodash/lodash.d.ts |
For more examples of generics, look at the declarations for Q.js // Some trivial example. Would never do this
var nums = Q.Promise<number[]>(resolve => {
resolve([1, 2, 3]);
});
nums.then(nums => {
nums // typescript knows this is a number[]
}); |
@jasonmeisel The guys over at React feel this wouldn't conflict with JSX anymore than TypeScript already does, and wouldn't break their current implementation. I propose we stick with the originally suggested syntax of |
@cspotcode @nathggns My mistake, I forgot about function generics :) If the proposed syntax doesn't break any existing systems, I am all for it! |
I do agree it should fail when calling bind. I just wanted to confirm that was your intent.
That is not what I asked. I asked about the assignability of a function type with a particular thisArg to another function type with a different thisArg based on the assignability of the type of the thisArgs.
Sorry, I misread your example. That said, by allowing them on the function operator you're also allowing them on interface and class methods. Consider: interface FooMethod {
<this: Foo>(x: string): number;
}
interface Bar {
func: FooMethod; // Equivalent to func<this: Foo>(x: string): number;
} Do you intend that func's thisArg type should revert to
Can you clarify what you mean by "first call" ? There are three lines in my example - the assignment, the direct call on the instance, and the indirect call through the class type. Which of these would compile with your proposal and which wouldn't? If by "first call" you mean the |
Sorry do you mean the following? class A {}
class B extends A {}
var x = function<this : number>(){};
var y = function<this : string>(){};
var z = function<this : A>(){};
var zz = function<this : B>(){};
x = y; // Error
y = x; // Error
zz = z; // Error
z = zz; // Okay
I think trying to set a property of an interface to a function interface that has a specified
Sorry I was being short-sited here. Yes, the assignment should error in order to stop changing Foo. class Foo {
someMethod() {
}
}
var foo = new Foo();
foo.someMethod = function<this : number>() {}; // Error
foo.someMethod(); // Wouldn't even reach this. |
Thanks, that answers my questions. You could put these points into your proposal post. |
@Arnavion I have done that now. Can I ask, do you support this proposal? While I know it would help me massively, and possibly the guys over at @DefinitelyTyped, I'm not sure how much other people need this feature. |
I have been subscribed to #229 for a long time, so yes I do support it :) this-typing is very helpful for callbacks, of course, but my own use for this feature would be to annotate interface methods with |
For what it's worth, I'm only an average TypeScript developer and constantly litter my code with |
Thank you. Would've been a bit rubbish to have written the proposal and for no one to really want it. Sent from my iPhone
|
If you want a quick idea for how the language should deal with a particular situation, you can rewrite the code so that For example, to test how this situation should behave:
Rewrite
|
Love this, elegant and non breaking. |
+1 |
Don't know if this is implied but I'd like the other arguments to
because (And perhaps As these would be breaking changes, perhaps they could be "switched on" by the presence of the |
@danielearwicker You mean this #212, right? :D |
It is a little strange this proposal addresses typing for Has anyone taken a stab at patching this proposal into the compiler? I can't imagine @danielearwicker's addition would be overly complex to add. |
Eagerly awaiting this feature! |
I've been thinking about this (and
And for me the pressing problem is catching the accidental creation and erroneous use of unbound methods, through isolated references to properties. I may have:
This is allowed in TS today, and happens to be correct. But what if
Now it is erroneous to grab a reference to
When they run it, it says "Hello undefined". With classes under the present TS type system, we are always a stone's throw away from this kind of error and the compiler ignores it. Also (whatever Crockford is saying this year!) there are realistic justifications for introducing the use of So I think as well as allowing explicit declaration of these unbound-method types, TS should assume an unbound-method type is being created whenever a function-type property is accessed without being immediately called (with "immediately" having the same meaning as it does for ES's behaviour with automatically binding Although it would cause backward incompatibility, it would most likely be welcomed. At any time in a large codebase a team member may want to introduce a use of Therefore, even if the coder never deliberately uses this proposal's syntax, I'm arguing that it would be valuable if this proposal had a backward-incompatible impact:
How could we mitigate this backward-incompatibility in situations where the coder is Crockford and wants to avoid
In the above proposal it is stated: "While specifying a thisArg type for function interfaces is allowed, trying to do the same for methods or properties is not." This makes perfect sense, because
So if this needed to be addressed, what actually appears to be needed is a way to state that a function property is not dependent on Straw-man syntax:
By prefixing with So in related issue #229 @RyanCavanaugh gave us some food for thought (using a different syntax from this proposal):
The first might be allowed because The second should only be allowed if The third isn't allowed if we assume Supposing the compiler is "smart" re: the
Then:
|
I think for the most part that that is an entirely separate proposal to this and should be submitted as so. I purposefully ignored automatically binding the thisArg on class methods as it would be a breaking change, whereas this proposal is not and could easily be introduced in a 1.x update. Sent from my iPhone
|
@nathggns Agreed. They are almost orthogonal in that you propose extending the syntax and not introducing incompatibility, whereas the core of what I'm suggesting is deliberate (focused) backward incompatibility, any new syntax being an optional extra. |
I think that my proposal (which could be introduced in 1.6?) would pave the way to make it possible to easily implement your proposal in 2. Sent from my iPhone
|
I described it above as: "By prefixing with function we declare that the method implementation cannot depend on contextual this." i.e. the rule is that function must not refer to My suggestion was probably off-topic (see subsequent comments above). The original issue is only about stating the type of I drifted onto a suggestion that TS should automatically assume function properties require a Then I added the suggestion that, to allow us to conveniently avoid the whole problem by not using classes and
You're right, I hadn't spotted there were two types involved. NB.
No, and I believe current opinion is not keen on changing this (there have been a few issues requesting that they be strongly typed). |
In fact, my |
Just noticed that in the proposal, it says:
In "use strict" mode, |
I see what you mean about the compiler enforcing that a function's implementation never uses
For that case, specifying If I understand correctly, we want two separate type-checks for What if we used
TypeScript can implicitly set
Oh ok. I wonder if it makes sense to add some useful typing to .bind without becoming restrictive. For example, we don't need to enforce the proper type of bound arguments. We can continue to allow
However, if
Does it make sense for the type of
|
@cspotcode - I'd prefer if the compiler pointed out to me that I'm passing a |
FYI: Flowtype has quite similar syntax (angle brackets and colon separator) for _bounded type parameters_, instead of thisArg. class BagOfBones<T: Bone> { ... }
function eat<T: Food>(meal: T): Indigestion<T> { ... } |
@teppeis The equivalent TS for that is |
@teppeis: Typescript already has that same syntax for generics, which are the same as bounded type parameters. Both are different than specifying the type of a variable, argument, or property ( Generics and bounded type parameters let you specify generic types constrained to other types. Contrast that with |
My vote is for the first/specially named parameter syntax. Anyone who has declared a function in the existing language with a first argument called 'this' deserves what they get really! E.g. setTimeout should be declared as Rather than just use "any" for callback as it does now. Because it's more likely I was thinking 'this' referred to the current class rather than the global Window object (which also has a 'name' property). |
@wizofaus Your syntax confuses me and makes me think the callback literally receives |
I would say "this" has enough semantics attached to it for any programmer that that's a pretty unlikely interpretation. I suppose an alternative is something like the extensions syntax used by C#: setTimeout(callback: (window: this Window, ...args: any[]) => void, timeout?: number, ...args: any[]): number; But it seems silly to have to name the parameter (and with Extension methods in C# you don't use "this" inside the function body). TBH if it were entirely up to me I'd actually simply change the meaning of 'this' to match that of C#, and introduce a different term for the cases where you need the existing semantics inside a function (though I'm not quite sure what I'd call it, and frankly I'd rather always use a named parameter). |
Yeah, the callback literally does "receive" Plus, TypeScript intends to add a type system onto vanilla ECMAScript, which means not covering up JS semantics. All TypeScript developers must understand how As a reminder, syntax is not at all preventing the Typescript team from implementing this feature. They still need a complete spec that addresses the specific semantic issues @RyanCavanaugh has mentioned. |
"TypeScript intends to add a type system onto vanilla ECMAScript, which means not covering up JS semantics" |
I'm just saying that, realistically, proposals that stray too far from the TS developers' goals are likely to be rejected. They want to align with ECMAScript 6, adding a type system and tooling on top of Javascript, allowing easy interop with existing Javascript libraries. They intentionally avoid overly-complex code transformations. It's fairly easy to migrate existing codebases to TypeScript because they've focused on a type system and tooling, not on changing language semantics. Even the current arrow functions and classes compile into ES6 arrow functions and classes. If the goal is merely a far better language, one might argue you should write C# and compile that into JS. |
For syntax I'd propose declare class MyClass {
someMethod(x: integer);
}
var x = function[MyClass](x: integer) {
// this: MyClass
this.someMethod(x);
} And for generics: declare class OtherClass<T> {
someMethod(arg: T);
}
var y = function<T>[OtherClass<T>](x: T) {
// this: OtherClass<T>
this.someMethod(x);
} About type system (roughly): |
Sorry for arriving late at this, as the biggest "item" is the ability to do essentially late binding of typing to allow chaining. I have seen a fair few machinations, but maybe we are missing something that would not impede compatibility with JavaScript/EcmaScript and would be essentially very much TypeScript without challenging generic usages. Admittedly, I am not at all familiar with the sort of hoops that this "late" typing would have on the compiler, but what about the following syntax: interface A {
foo(): typeof this;
}
class B implements A {
foo(): typeof this {
return this;
}
}
class C extends B {
bar(a: typeof this): typeof this {
return a.foo();
}
}
let c = new C();
c.foo().bar(c).foo(); /* should be totally valid */ So essentially, instead of a version of generics, or something else, whenever you need to type something of the current class (or validate that something is implemented an interface properly) you would use While, I haven't worked through all the emitting of the above, I think it could easily be determined and that the "reserved" type of |
@kitsonk I think that suggestion belongs in #285. My vote is for @nathggns: your proposal has assignability the wrong way around for class A {}
class B extends A {}
var z = function<this : A>(){};
var zz = function<this : B>(){};
z.call(new A()); // Okay
z.call(new B()); // Okay
zz = z // Okay
zz.call(new A()); // Error
zz.call(new B()); // Okay
z = zz; // Error |
FWIW, I (unaware of this issue) opened a similar one on facebook/flow#452, in my last comment proposing the same syntax that was proposed here (e.g. I think that would be the most intuitive syntax for this, and I use it in trine for annotating function types (I use function add (&a : number, b : number) { return a + b; }
function vec2add (&[x1, y1], [x2, y2]) { return [ x1 + x2, y1 + y2 ]; }
// bind syntax:
[1,2]::add([5,3]) // [6, 5] |
With respect to the new ES7 "::" bind operator I'd propose var abc = function MyClass::(x: integer) {
// this: MyClass
this.someMethod(x);
}
function MyClass::abc(x: integer) {
// this: MyClass
this.someMethod(x);
}
var abc = function<T> OtherClass<T>::(x: T) {
// this: OtherClass<T>
this.someMethod(x);
}
function<T> OtherClass<T>::abc(x: T) {
// this: OtherClass<T>
this.someMethod(x);
} Will it lead to the breaking changes? |
It will work also with function {count:number}::inc() {
this.count++
} The most difficult seems to parse the case where type should be enclosed into parentheses: function (()=>void)::inc() { ... }
function (A | B)::test() { ... } And even function<T> (T::(x: number)=>number)::bar(y: number, t: T): number {
return t::this(y) + y
}
var foo = function {factor:number}::(x: number) { return this.factor*x*x; }
console.log(foo::bar(5, {factor: 0.1})) // 7.5 However it seems that here we need not more than two or three look-ahead tokens. Need we? |
Is there any official progress/comments on this? |
this is now tracked by #3694 |
+1 |
This is an official proposal for one of the issues outlined in #229.
Firstly, I'd like to give a big thank you to @ahejlsberg for identifying the issue, and to @redexp, @ivogabe, @cspotcode, and others for their contributions to the original issue as they have formed most of the grunt work of this proposal. I've just decided I want the feature enough that I will take the time to write an official proposal.
I'd also like to note this is my first official proposal (for not just TypeScript, any language). I'm sorry if I've missed anything glaring out. Any advice on how to improve my proposals would be appreciated.
The Problem
When working with many libraries that take callbacks (including jQuery), TypeScript will give very little intelligence as to the type of the
thisArg
inside a callback's body �– intact, it will simply be typed asany
.A few examples follow
Because the
thisArg
is of typeany
, we get no type checking and no autocomplete or any other kind of intellisense. Also, when defining functions within TypeScript user land code that rely on thethisArg
being a certain type, there is nothing stopping consuming code calling it with whatever type they provide.Proposed Change
Ideally, a function definition would be able to define the type of its
thisArg
. This means that any consuming code must provide athisArg
of this type (usingcall
,apply
, orbind
). It also means that within the body of that function, we can assume that the type of itsthisArg
is of the provided type, rather than reverting to anany
type.Quick Note: None of this applies to functions defined using arrow functions. You cannot specify the type of the
thisArg
with arrow functions as their type is lexically bound. I believe if you try to use the syntax specified below with arrow functions it should be either a compiling error or a parsing error.There are many different places where you can define a function in TypeScript. However, the main syntax is as follows at the moment.
I propose adding to this syntax a way of specifying the type of the
thisArg
within the function body.There are a few suggestions for this syntax including some from the original issue and some I have thought of myself, but I think the best out of the suggested is the following syntax:
In the statement and expression examples, we can assume that the type of
thisArg
inside their bodies isSomeThisType
. In the last two examples, we can assume that type type ofthisArg
inside the passed callbacks is of typeSomeThisType
.However, these assumptions rely on enforcing that any TypeScript code calling these functions have to pass the correct
thisArgs
. Calling functions in JavaScript can get very complicated but here are the different ways that you can pass athisArg
, along with the semantics of this proposal.Taken the following code:
We would no longer be able to call this function simply by invoking it, as the
thisArg
would be set to the global object instead of an instance ofSomeType
. Trying to do so should throw a TypeError of some description (the exact error is intentionally left undecided at this point).Specifying the type of
thisArg
usingcall
,apply
, orbind
with an incompatible type will throw the same error. Withcall
andapply
the error is triggered at call time. Withbind
, it is triggered at "bind time" (to more easily find the source of an error).Only invoking with a compatible type of
SomeType
will be accepted by the type engine.Again, I would like to stress that this proposal has no affect on arrow functions. Even attempting to specify the
thisArg
for arrow functions should be an error.One notably exception to this is that you are able to pass arrow functions as arguments where a thisArg has been specified.
Example:
Note:
bind
does not change the specifiedthisArg
of a function. This can only be set at define-time.The Syntax
You will have noticed that we pass the
thisArg
within the type arguments of a function, where you would normally pass generic type information. This does not conflict however, as thethisArg
must be the last type argument, separated by a,
if any other arguments are passed, and must be prefixed withthis:
as follows.Note: This is a very contrived example, as very rarely will a function ever take a generic argument as well as specifying the
thisArg
as specifying thethisArg
is usually done when declaring function arguments and then passing anonymous functions like so:Why this syntax?
I chose this syntax as the main use case for this feature is for anonymous functions, in which you almost if not never have generic type arguments for. Alternatives were to add the this into the parameter list, which I disliked as it made for a messy-looking solution in the common use case.
Personally, I find the latter much harder to read.
How is this problem solved?
With this change, you can now specify the
thisArg
within function bodies. While this works for any functions, the main use case is for passing anonymous functions to library code. With this change, you can now specify in a declaration thethisArg
in the body of a function argument.This is a very basic example of how the on function could be written in a jQuery definition file. I've put the definition and usage in one file for convenience.
Note how in this example, the whole semantics of enforcing the type of
thisArg
when calling a function is irrelevant as we're simply telling TypeScript how existing JavaScript works. This is actually the common use case for this code.Void thisArg
In some cases, the value of
thisArg
can be undefined (directly invoking a function in strict mode). It can be helpful to disallow consuming code from being able to use the value of this even if it isn't undefined, by making TypeScript assume it is undefined.You can do this like so:
Classes
Currently, explicitly setting the type of
thisArg
in class methods is unsupported (triggering a syntax error) as doing this comes with its own set of problems, and can be added on at a later date once the basic semantics have been sorted.This also means you cannot dynamically change a method on a object to a function that has a specified
thisArg
. Also, to maintain currently functionality, TypeScript will continue to assume that this is of the type in which the method is defined within the method body, but will allow you to trigger it with anythisArg
.Automatically setting the thisArg within class methods
In the original issue, @cspotcode suggested that the
thisArg
automatically be set for class methods as such that the following is an error.I have intentionally left this out of this proposal as it causes a lot of its own backwards incompatibility issues, hence why there is no syntax addressed for setting the thisArg for class methods. However, should this proposal be accepted, then it might be worth extending it to address this. Should you choose to do this, this comment should be of some help to you.
Interfaces
While specifying a
thisArg
type for function interfaces is allowed, trying to do the same for methods or properties is not. This is something that can be revisited if/when class method support forthisArg
type specifications is added.Assignability
If
B
is assignable toA
,function<this:B>
is assignable tofunction<this:A>
, but not vice versa.function<this:any>
is assignable tofunction<this:A> and
functionthis:B.
functionthis:Aand
functionthis:Bis assignable to
functionthis:any`.Updating lib.d.ts
@cspotcode also showed that updating lib.d.ts to properly set the thisArg could cause a lot of breakage. Again, to simplify this proposal, I have left this out. Again, this is something I think should be addressed after choosing to merge this proposal, and this feature is still useful without updating
lib.d.ts
.Emitted JavaScript
Regardless of the syntax used to specify the
thisArg
, it must be stripped from the outputted JavaScript. This is simply a syntactical addition to the type engine, which is always stripped from emitted javascript.Incompatibilities
I won't claim to know of every feature currently proposed for ES6 and ES7 / current TypeScript proposals, but I don't believe that the proposed syntax conflicts any proposed feature.
Breaking Changes
There are no breaking changes by simply adding this feature (as functions must manually add an annotation for the type of their
thisArg
). Breaking changes would only be introduced should this be added tolib.d.ts
, or if classes begin to automatically set theirthisArg
.The text was updated successfully, but these errors were encountered: