-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
bind(), call(), and apply() are untyped #212
Comments
It would be nice to type these more strongly but we will need a proposal for some options. We previously discussed this in some early design meetings on generics and weren't really able to come up with something suitable. At least using Function instead of 'any' where applicable might be a meaningful improvement. See Ander's response here for a similar example: https://typescript.codeplex.com/discussions/462819
|
With regards to 'bind' and 'call', what specifically do you need proposals for? Are there cases where the returned type could be ambiguous? Apologies if I'm missing something obvious. |
Just a brief exploration of some of the issues here. We would need to fill in all the interface fn {
(): string;
(n: number): void;
(n: number, m: number): Element;
(k: string): Date;
}
var x: fn;
var y = x(); // y: string
var args = [3]; // args: number[]
var c1 = x.apply(window, args); // c1: ?
args = []; // arg: still number[]
var c2 = x.apply(window, args); // c2: ??
var b = x.bind(undefined, null); // b: what type?
var j = b(); // j: ?
|
It seems like the
|
Here is another example I came across where this is an issue:
The The only solution I could think of is to force the type:
|
@RyanCavanaugh
I find I have a lot of trouble with this issue, as I frequently factor my functions so that they contain small helper functions. Here's a simplified example to show the structure I use:
Any helpers that reference this must be invoked via call(). I would greatly appreciate it if you could add support for type checking the arguments of a call(). EDITED: to add type helper |
@psnider, if you want to avoid introducing a lexical var _this = this;
function helper(opts: Options) {
_this.a = // ...
} or you could also use an arrow function. var helper = (opts: Options) => {
this.a = // ...
} |
awesome! Cleans things up nicely, solving two problems:
Thanks, I had overlooked using the fat-arrow syntax here, I can continue on with my refactoring addiction without fear! For reference, the modified code, for which the compiler correctly reports the type error is:
|
Ok how about this as a spec for bind: Binding with:
results in compiler error: 'Supplied parameters do not match any signature of call target.' e.g.
For an overloaded function where there is ambiguity, 'bind' should return type any. e.g.
For:
bind returns the expected typed value:
|
@psnider glad that solves your problem! Now that we have tuple types, my personal opinion is that instead of trying to add magic to the typing for these functions, you should actually use typed functions. var _apply = (f: any, args: any[], thisArg?: any) => f.apply(thisArg, args);
var apply$: <T, R>(f: (...values: T[]) => R, args: T[], thisArg?: any) => R = _apply
var apply2: <T1, T2, R>(f: (a1: T1, a2: T2) => R, args: [T1, T2], thisArg?: any) => R = _apply
var apply3: <T1, T2, T3, R>(f: (a1: T1, a2: T2, a3: T3) => R, args: [T1, T2, T3], thisArg?: any) => R = _apply;
var apply4: <T1, T2, T3, T4, R>(f: (a1: T1, a2: T2, a3: T3, a4: T4) => R, args: [T1, T2, T3, T4], thisArg?: any) => R = _apply;
var apply5: <T1, T2, T3, T4, T5, R>(f: (a1: T1, a2: T2, a3: T3, a4: T4, a5: T5) => R, args: [T1, T2, T3, T4, T5], thisArg?: any) => R = _apply;
// etc. Or you could use overloads if you don't like explicit arities: function apply<T1, T2, T3, T4, T5, R>(f: (a1: T1, a2: T2, a3: T3, a4: T4, a5: T5) => R, args: [T1, T2, T3, T4, T5], thisArg?: any): R;
function apply<T1, T2, T3, T4, R>(f: (a1: T1, a2: T2, a3: T3, a4: T4) => R, args: [T1, T2, T3, T4], thisArg?: any): R;
function apply<T1, T2, T3, R>(f: (a1: T1, a2: T2, a3: T3) => R, args: [T1, T2, T3], thisArg?: any): R;
function apply<T1, T2, R>(f: (a1: T1, a2: T2) => R, args: [T1, T2], thisArg?: any): R;
function apply<T, R>(f: (...values: T[]) => R, args: T[], thisArg?: any): R;
function apply(f: any, args: any[], thisArg?: any) {
return f.apply(thisArg, args);
} Instead of function curry2<T1, T2, R>(f: (a1: T1, a2: T2) => R) {
return (a1: T1) => (a2: T2) => f(a1, a2);
}
function curry3<T1, T2, T3, R>(f: (a1: T1, a2: T2, a3: T3) => R) {
return (a1: T1) => (a2: T2) => (a3: T3) => f(a1, a2, a3);
}
// etc.
function bind2<T1, T2, R>(f: (a1: T1, a2: T2) => R, thisArg: any) {
return (a1: T1) => (a2: T2): R => f.call(thisArg, a1, a2);
}
// etc.
|
@DanielRosenwasser Could you expand on why you are against my solution for bind? Do you believe your proposal will be better from a user standpoint? As a typescript user, I want to use bind in an identical way to javascript. i.e.
and have it typed as per my previous post. Your proposals would not support this. For example, using your solution 'bind2', the code would look like:
And even worse for functions with more applied args e.g.
Using bind as described in my proposal is not only more familiar to people who know javascript, but also allows easier transition from untyped javascript code into properly typed typescript. |
@jameslong we generally understand what the desired behavior of bind is. Your solution is stating the cases which would be errors and which would not be but that's not really the level of detail holding back this feature. We need a solution that models the type signatures and type flow in a way that allows the compiler to actually figure out whether or not to report an error in these cases. |
Thanks @danquirk, that effectively describes the main issue. The other is that I'm generally not a fan of having compiler-internal support of @jameslong, for your examples, why even bother with bind2(myFunction, undefined)(arg0) becomes something like (x: number) => myFunction(arg0, x) and bind5(myFunction, undefined)(arg0)(arg1)(arg2)(arg3) becomes something like (x: string) => myFunction(arg0, arg1, arg2, arg3, x) It's not perfect, but it's not nearly the worst workaround either. If you need to specify a |
@danquirk Thanks Dan. I appreciate there are complexities in the compiler implementation of this feature. However it seems there is not agreement on whether we would like this to be a compiler feature or not which is why I was making that point. |
I think in general we'd all agree that any signature that has |
interface Function {
apply<T, R, ...X>(
this: (this: T, ...args: X) => R,
thisArg: T,
args: X
): R;
call<T, R, ...X>(
this: (this: T, ...args: X) => R,
thisArg: T,
...args: X
): R;
}
interface Function {
// bound functions
bind<T, R, ...X, ...Y>(
this: (this: T, ...initial: X, ...rest: Y) => R,
thisArg: T,
...initial: X
): (...rest: Y) => R;
// bound constructors
bind<C, ...X, ...Y>(
this: new (...initial: X, ...rest: Y) => C,
thisArg: any,
...initial: X
): new (...rest: Y) => C;
} Let's just say it requires lazy inference of the template each use, probably using a simplified structure along the same lines as Conway's Game of Life or Rule 110, something not currently done, and something not easy to program. let bar: T;
let baz: A;
let quux: B;
let spam: C;
let eggs: D;
declare function foo(this: T, a: A, b: B, c: C, d: D): E;
let res = foo.bind(bar, baz, quux);
res(spam, eggs) Expansion/substitution of last statement: // Initial
bind<T, R, ...X, ...Y>(this: (this: T, ...initial: X, ...rest: Y) => R, thisArg: T, ...initial: X): (this: any, ...rest: Y) => R;
// Steps to reduce
bind<T, R, ...X, ...Y>(this: (this: T, ...initial: X, ...rest: Y) => R, thisArg: T, ...initial: X): ( ...rest: Y) => R;
bind<T, E, ...X, ...Y>(this: (this: T, ...initial: X, ...rest: Y) => E, thisArg: T, ...initial: X): ( ...rest: Y) => E;
bind<T, E, A, ...X, ...Y>(this: (this: T, a: A, ...initial: X, ...rest: Y) => E, thisArg: T, a: A, ...initial: X): ( ...rest: Y) => E;
bind<T, E, A, B, ...X, ...Y>(this: (this: T, a: A, b: B, ...initial: X, ...rest: Y) => E, thisArg: T, a: A, b: B, ...initial: X): ( ...rest: Y) => E;
bind<T, E, A, B, ...Y>(this: (this: T, a: A, b: B, ...rest: Y) => E, thisArg: T, a: A, b: B ): ( ...rest: Y) => E;
bind<T, E, A, B, C, ...Y>(this: (this: T, a: A, b: B, c: C, ...rest: Y) => E, thisArg: T, a: A, b: B ): (c: C, ...rest: Y) => E;
bind<T, E, A, B, C, D, ...Y>(this: (this: T, a: A, b: B, c: C, d: D, ...rest: Y) => E, thisArg: T, a: A, b: B ): (c: C, d: D, ...rest: Y) => E;
bind<T, E, A, B, C, D >(this: (this: T, a: A, b: B, c: C, d: D ) => E, thisArg: T, a: A, b: B ): (c: C, d: D ) => E;
// Signature of what's finally type checked
bind<T, E, A, B, C, D>(this: (this: T, A, B, C, D) => E, T, A, B): (C, D) => E; Not simple. (Not even C++ can do this without major template hacks) Or, in highly technical CS jargon, checking the above type correctly may mean controlling the variadic expansion in each occurence via a simple Class 1 cellular automaton, reducing it until the variadic types have been eliminated. There is the possibility that a template expands infinitely, especially if the function is recursive (singly or cooperatively), but a hard limit could prevent a lock-up for the compiler. |
related to #3694 |
For those of you who care, I've create the following little TypeScript-snippet that outputs an overloaded function declaration of 'bind': Output (you can scale this up if you want):
Snippet:
|
Can this be approached in an incremental way? What I'm interested in the use of interface AngularScope
{
$watch<T>(event:string, callback: (newValue:T, oldValue:T)=>void);
}
class Example
{
constructor($scope: AngularScope){
$scope.$watch('change', this.theChangeCallback.bind(this));
}
theChangeCallback(new: string, old:string) {
// Do work
}
} In this circumstance, I am not using $scope.$watch('change', (new:string, old:string)=>this.theChangeCallback(new, old)); In this situation, the type should be What would need to be added to the compiler, just to achieve this? I feel like if |
@bryanerayner you should use an arrow function to define the member in the class from the get go. This is covered here : https://basarat.gitbooks.io/typescript/content/docs/tips/bind.html |
There was a simple
For a more general solution, I tried a interface Function {
bind<
F extends (this: T, ...args: ArgsAsked) => R,
ArgsAsked extends any[],
R extends any,
T,
Args extends any[], // tie to ArgsAsked
Left extends any[] = DifferenceTuples<ArgsAsked, Args>,
EnsureArgsMatchAsked extends 0 = ((v: Args) => 0)(TupleFrom<ArgsAsked, TupleLength<Args>>)
// ^ workaround to ensure we can tie `Args` to both the actual input params as well as to the desired params. it'd throw if the condition is not met.
>(
this: F,
thisObject: T,
...args: Args
): (this: any, ...rest: Left) => R;
// ^ `R` alt. to calc return type based on input (needs #6606): `F(this: T, ...[...Args, ...Left])`
}
|
@mhegazy Any updates this. Need this for typescript to support currying, partial applications etc. in functional programming. |
I wanted these to be strongly typed and found out that it's relatively easy to type, just time consuming. As a result, I made a generator to do it for me. People might have done this previously but I couldn't find anything. Was a fun side project. Generated types (supports up to 10 arguments): https://gist.github.com/Yuudaari/f4c21f8e5e3dad36e4c7f61cbddb4f22 Replace native methods with ones that support these methods on class constructors: https://gist.github.com/Yuudaari/68662657485524c8f1c65250e7c2d31f I really wish there was syntactic sugar for |
@Yuudaari looks like a good candidate for |
@RyanCavanaugh Any interest in merging in massively overloaded typings for |
@bcherny There are caveats to it; using the methods on varargs functions falls back to the provided function overloads, currently, whereas if there were just my overloads, it would not work. This also means that it can't be used for type checking, really, because the original is vague so it'll accept anything that doesn't work w/ my overloads. My overloads are only really nice for a little bit of type hinting. I'm not sure they're production ready. Also, there might be a non-negligible performance impact of adding a lot of overloads, but I haven't compared with and without. It's also not that hard to just stick them in your projects if you want them. |
@yuudaari Not sure I totally understand. Could you comment with a few examples of what works, and a few of what doesn’t? |
In case folks aren't aware, the tuple rest/spread work in #24897 slated for TS 3.0 will probably end up fixing this issue in the not-too-distant future.
|
@jcalz to qualify that, #24897: does:
does not:
That said, it's a great step forward. |
@tycho01 very interesting. Do we know of any plans or PRs that enable us to have a fully strong-typed Also, @ahejlsberg said that "we need to investigate the repercussions". Do we have a timeline on when this will happen, or an issue that keeps track of this progress? |
Now implemented in #27028. |
for the original question, I think there is another solution: just derive your class from this one class Bindable<T> {
bind<K extends keyof T>( name: K ) : ( ...args: EventIn<T,K> ) => void {
return ( ...args ) => {
this[name as string]( ...args );
}
}
}
// just a demo class implementation
class MyClass extends Bindable<MyClass> {
v: number;
constructor( value: number ) {
super( );
this.v = value;
}
// the demo method we will bind
test( x: number ) {
return this.v + x;
}
}
let obj = new MyClass( 5 );
let fn = obj.bind( 'test' ); // here editor shows 'test' ( and 'v' and 'bind' we can remove using more elaborated code)
fn( 6 ); // here editor is waiting for a number because arguments types were propagated
fn( 'hello' ); // will fail due to bad argument type |
bind() returns type 'any' and does not type check it's arguments. e.g.
compiles without error and 'addString' has type any.
bind is a default way to achieve partial application so it would be great to have it typechecked properly.
Similarly for apply, the code
compiles without error and listA is still incorrectly marked as type 'number[]'.
call() example:
Again without a compiler error.
The text was updated successfully, but these errors were encountered: