Skip to content
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

extract rest params #17898

Closed

Conversation

KiaraGrouwstra
Copy link
Contributor

@KiaraGrouwstra KiaraGrouwstra commented Aug 18, 2017

This stub to extracting rest params into tuple types is the second part of my attempt to tackle #5453.

It allows specifying array-like types including tuple types and array-bound generics as rest constraints: <T extends number[]>(...args: T) / (...args: [number, string]). The former of these examples aims to extract the types of passed parameters on function application, while the latter is primarily useful to dynamically generate functions, e.g. to type bind or curry.

One may extract the types passed into generics from rest parameters as follows:

declare function f<T extends number[]>(...args: T): T;
const x = f(1,2);
// type: [1, 2]

Fixes a third of #5453, alongside other issues that ran into A rest parameter must be of an array type, like #1024, #2328, #5331, and #16931.

@KiaraGrouwstra
Copy link
Contributor Author

Update: managed to make it error both if passing too many as well as if passing too few arguments now. That only leaves the hard part now...

@eggers
Copy link

eggers commented Aug 24, 2017

@tycho01 Here's another use case to check:

interface Promise<T> {
    spread<U>(fulfilledHandler: (...values: T & any[]) => U | Promise<U>): Promise<U>;
    spread<U>(fulfilledHandler: (...values: [T]) => U | Promise<U>): Promise<U>;
}

@KiaraGrouwstra
Copy link
Contributor Author

@eggers: out of curiosity, what's up with the T & any[]?

@eggers
Copy link

eggers commented Aug 24, 2017

@tycho01 I was looking for a way to ensure that T is an array to activate that signature. There's probably a better way.

@KiaraGrouwstra
Copy link
Contributor Author

@eggers: Makes sense then. This wasn't a standard library function though is it?

@eggers
Copy link

eggers commented Aug 25, 2017

@tycho01 It's not in the native promise library, but it's in most polyfills. There are different ways of declaring spread now:

// Q
spread<U>(onFulfill: (...args: any[]) => IWhenable<U>, onReject?: (reason: any) => IWhenable<U>): Promise<U>;

// bluebird
spread<U, W>(fulfilledHandler: (...values: W[]) => U | PromiseLike<U>): Bluebird<U>;
spread<U>(fulfilledHandler: Function): Bluebird<U>;

// When
spread<T>(onFulfilled: _.Fn0<Promise<T> | T>): Promise<T>;
spread<A1, T>(onFulfilled: _.Fn1<A1, Promise<T> | T>): Promise<T>;
spread<A1, A2, T>(onFulfilled: _.Fn2<A1, A2, Promise<T> | T>): Promise<T>;
spread<A1, A2, A3, T>(onFulfilled: _.Fn3<A1, A2, A3, Promise<T> | T>): Promise<T>;
spread<A1, A2, A3, A4, T>(onFulfilled: _.Fn4<A1, A2, A3, A4, Promise<T> | T>): Promise<T>;
spread<A1, A2, A3, A4, A5, T>(onFulfilled: _.Fn5<A1, A2, A3, A4, A5, Promise<T> | T>): Promise<T>;

None of which will give an error if the fulfilled handler arguments are incompatible with the promise type. e.g.:

declare var promise: Promise<number>;
promise.spread((x: string) => x); // No error

@KiaraGrouwstra
Copy link
Contributor Author

KiaraGrouwstra commented Aug 25, 2017

@eggers: Thanks for the background, at least I know where to check for run-time behavior now.
I guess your second overload is already covered by your first one as well?

Your case here is actually interesting as you don't even require capturing the info into a generic for this -- with that, it appears to already work with my current progress :), just added a test for it.

@eggers
Copy link

eggers commented Aug 25, 2017

Awesome. I made some explicit use cases below. The second overload was to cover how I thought bluebird treated promises that aren't arrays (see case 1), but I retested, and it actually throws an error (TypeError: expecting an array or an iterable object but got [object Null]), so that overload isn't actually necessary to define spread properly.

declare var promiseNumber: Promise<number>;
promiseNumber.spread((x: string) => x); // Case 0: Error
promiseNumber.spread((x: number) => x); // Case 1: No Error

declare var promiseNumberArray: Promise<number[]>;
promiseNumberArray.spread((x: string) => x); // Case 2: Error
promiseNumberArray.spread((x: number) => x); // Case 3: No Error

declare var promiseTuple: Promise<[number, string]>;
promiseTuple.spread((x: string, y: string) => x); // Case 4: Error
promiseTuple.spread((x: number, y: string) => x); // Case 5: No Error
promiseTuple.spread((x: number) => x); // Case 6: Probably Error, though not critical
promiseTuple.spread((x: number, ...rest: any[]) => x); // Case 7: No Error
promiseTuple.spread((...start: any[], x: string) => x); // Case 8: No Error

@KiaraGrouwstra KiaraGrouwstra force-pushed the 5453-extract-rest-params branch from 9b6de0c to 6d38c6b Compare August 28, 2017 18:51
@KiaraGrouwstra KiaraGrouwstra changed the title WIP: extract rest params extract rest params Aug 28, 2017
@KiaraGrouwstra
Copy link
Contributor Author

I got this working now, would be happy to get feedback.

@KiaraGrouwstra
Copy link
Contributor Author

Update: I fixed the test Travis complained about. Apparently the getTypeOfSymbol accessor indirectly mutated the symbol adding an id, which somehow turned out breaking in that case.

@Eyas
Copy link
Contributor

Eyas commented Sep 17, 2017

I ran into this recently with code like:

function flatten<Args extends {}[], Return>(
    fn: (args: Args) => Return
): (...args: Args) => Return
{
    return null as any;
}

So looking forward to seeing this merged.

@microsoft microsoft deleted a comment from msftclas Sep 27, 2017
@MikkelSnitker
Copy link

Hi.

Would this change allow me pass a callback function where i change the callback function's return type but keeping the parameters names and types?

e.g. something like this:

type PacthedReturnType<ReturnType> = {
 prop1:number;
result: ReturnType;
}

function patchFunction<Args extends {}[], Return>(fn: (args: Args) => Return): (...args: Args) => PatchedReturnType<Return>
{
    return null as PatchedReturnType<Return>;
}

function greet(name:string, email:string) {
 return `Hi ${name} (${email})`
}

var myPatchedFunction = patchFunction(greet);

would the signature of myPatchedFunction then be:

(name:string,email:string)=>({
prop1:number;
result: string;
});

/Mikkel

@KiaraGrouwstra
Copy link
Contributor Author

KiaraGrouwstra commented Oct 16, 2017

@MikkelSnitker: this PR does half of that.

That (args: Args) => Return would turn into (...args: Args) => Return -- this PR allows capturing those arguments into that Args this way.

The second part, the ...args: Args you already had in your version, would become possible with #18004.

An alternative possible today would be to type patchFunction using overloads (one for each function arity).

@MikkelSnitker
Copy link

@tycho01 Thanks for the clarification :) I don't see how I could overload the patchFunction, and keep the parameter names (it's meant to take an arbitrary function as input). In my use case the patchFunction would take a pure function and wrap it into a redux action, but still keep the parameter names/types of the original function.

Are there any timeline for when we could expect this (and #18004 ) to be included in typescript?

/Mikkel

@KiaraGrouwstra
Copy link
Contributor Author

In my use case the patchFunction would take a pure function and wrap it into a redux action, but still keep the parameter names/types of the original function.

Yeah, state management libs like Redux are definitely one of the main use cases of type programming :), that was what drove me to wanting to see more progress on this front as well.

Are there any timeline for when we could expect this (and #18004 ) to be included in typescript?

I haven't received feedback from the team on either here yet, seems they're quite busy.

and keep the parameter names (it's meant to take an arbitrary function as input)

I would love this as well. My current PRs don't cover param names though -- I don't know what would be the nicest API for this. I'd be pretty interested in ideas on that front.

@MikkelSnitker
Copy link

I'm not a language designer, but a could a simple solution be to make a generic function type? like Function<TParams,TReturn>
it could then be used like


type Wrapped<T> = {
item: T
}

function patchFunction<TParams, TReturn>(func: Function<TParams,TReturn>){
return null as Function<TParams,Wrapped<TReturn>>
}

@KiaraGrouwstra
Copy link
Contributor Author

Note that for testing types in TS you can write declare function patchFunction<TParams, TReturn>(func: Function<TParams,TReturn>): Function<TParams,Wrapped<TReturn>> to omit details like functions' run-time implementation. :)

The idea of a generic function type was discussed at gcanti/typelevel-ts#8, with an attempt at implementation on my part here.

Note that using that in an attempt to capture the function is a bit problematic though -- I don't think you can currently use the type params in Function<TParams,TReturn> to capture it. (Nor do I know how that could be effectively tackled, even in theory, for that matter.)

In fact, afaik we currently don't have a way to extract parameter types from function types at all, other than using overload matching (kills generics, verbose, doesn't scale well performance-wise once you get multiple variables that explode the number of combinations). I'd made a PoC to work around this for curry / bind at #5453 (comment).

@Xenya0815
Copy link

What's the status of that cool feature?

@typescript-bot
Copy link
Collaborator

Thanks for your contribution. This PR has not been updated in a while and cannot be automatically merged at the time being. For housekeeping purposes we are closing stale PRs. If you'd still like to continue working on this PR, please leave a message and one of the maintainers can reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants