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

question: how can i create and return a generic function in TypeScript? #9949

Closed
zpdDG4gta8XKpMCd opened this issue Jul 26, 2016 · 22 comments · Fixed by #30215
Closed

question: how can i create and return a generic function in TypeScript? #9949

zpdDG4gta8XKpMCd opened this issue Jul 26, 2016 · 22 comments · Fixed by #30215
Labels
Duplicate An existing issue was already created Fixed A PR has been merged for this issue

Comments

@zpdDG4gta8XKpMCd
Copy link

function flip<a, b, c>(fn: (one: a, two: b) => c): (one: b, two: a) => c {
    return function flipped(one: b, two: a): c {
        return fn(two, one);
    };
}
function of2<a, b>(one: a, two: b): [a, b] {
    return [one, two];
}
const flipped = flip(of2); // expected: <a, b>(one: b, two: a) => [a, b], actual: (one: {}, two: {}) => [{}, {}]
@zpdDG4gta8XKpMCd zpdDG4gta8XKpMCd changed the title how do i make a flip function that works in TypeScript? question: how can i create and return a generic function in TypeScript? Jul 26, 2016
@jesseschalken
Copy link
Contributor

When TS sees flip(of2), it has to unify (one: a1, two: b1) => c1 (from flip) with <a2, b2>(one: a2, two: b2): [a2, b2] (from of2). My guess is at this step it sees that of2 has type parameters but no information for them, so just fills them in with {}.

What you would need is for TS to lift the type parameters up a level rather than just defaulting them. That is, if any type parameters needed to be defaulted when inferring the type of an expression (eg flip(of2)), the resulting type should itself have those parameters. (More than one set of type parameters may have needed to be filled in, so their order may be arbitrary.)

I'm not sure what the broad implications of that would be, though.

@yuit yuit added the Question An issue which isn't directly actionable in code label Jul 27, 2016
@mhegazy mhegazy added Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. and removed Question An issue which isn't directly actionable in code labels Sep 20, 2016
@gcnew
Copy link
Contributor

gcnew commented Dec 16, 2016

I took a stab at this and made a hacky implementation. It's by no means complete, but it can be used to play around with.

Current progress:

function flip<A, B, R>(f: (a: A, b: B) => R): (b: B, a: A) => R {
    return (b, a) => f(a, b);
}

function id<T>(x: T): T {
    return x;
}

function fconst<X, Y>(x: X, y: Y): X {
    return x;
}

function addStr(x: number, y: string) {
    return x + y;
}

function tagged<T extends string, Q>(tag: T, value: Q): { tag: T, value: Q } {
    return { tag, value };
}

// it was working before
const f1 = flip(addStr); // (b: string, a: number) => string
const v1 = f1("hello", 3);
const v2 = id(id)(3); // `3`

// working now
const f2 = flip(id);     // <T>(b: {}, a: T): T
const f3 = flip(fconst); // <Y, X>(b: Y, a: X) => X

const v3 = f3(1, "qw") // `"qw"`
const v4 = f3([], {})  // `{}`

const f4 = flip(tagged);   // <Q, T extends string>(b: Q, a: T) => { tag: T, value: Q }
const v5 = f4(5, "hello"); // { tag: "hello", value: number }
const v6 = f4(5, 5);       // Error as expected

declare function compose<A, B, C>(f: (b: B) => C, g: (a: A) => B): (a: A) => C;

declare const f: <T>(x:number) => T;
declare const g: (x:boolean) => number;
const f5 = compose(f, g)      // OUCH! this gets type `<T>(a: boolean) => T`

declare const g_2: <T>(x: T) => boolean;
declare const f_2: (x: boolean) => number;
const f6 = compose(f_2, g_2)  // <T> (a: T) => number
const f7 = compose(id, x => String(x)) // (a: {}) => string

// Was working, now not:
declare function h<R>(f: (x: number) => R): R;
var z: number = h(id);  // number ~ T, R ~ T
                        // backpropagation is now broken

const arr = [1, 2, 3].map(id); // T[]     Snap :(

See https://github.com/gcnew/TypeScript/tree/polyFuncUnification

Edit: updated branch link and the examples

@gcnew
Copy link
Contributor

gcnew commented Dec 16, 2016

@mhegazy What should the proposal look like?

@mhegazy
Copy link
Contributor

mhegazy commented Dec 16, 2016

mainly how it would be implemented. implications on different parts of the system, e.g. assignablity (isTypeRelatedTo), overload resolution, and generic argument inference.

@jesseschalken
Copy link
Contributor

It works in Flow.

@KiaraGrouwstra
Copy link
Contributor

Man, this is already an awesome improvement as-is, hope this could make it in.

@gcnew: what would you be expecting for f5 here? I'd be inclined to think the compiler is getting the best inference it can using the info available...

That Type argument candidate 'T' is not a valid type argument because it is not a supertype of candidate 'string'. in f7 is pretty weird though.

@gcnew
Copy link
Contributor

gcnew commented Dec 20, 2016

@tycho01 Thank you!

Yes, you are right about f5. My remark is that it's wrong from a logical perspective. f7 is correct as well. {} says any type could work here (because of String(x: any) => string) and id just picks it up. It can be generalized to a type param but it won't change anything.

PS: I've pushed a fix for what you are observing on f7. Check [1, 2, 3].map(id) for the new road block.

@KiaraGrouwstra
Copy link
Contributor

Hah, T[] for [1, 2, 3].map(id) huh. Don't think I'd seen that before, a generic without any binding...

@gcnew
Copy link
Contributor

gcnew commented Jan 11, 2017

I just pushed a greatly improved unification version. It works 80% of the time (with some corner cases left out). I'm still going through the failed test cases, but I think other people can give it a try as well.

@shutej
Copy link

shutej commented Apr 9, 2017

Any ideas when/if this will land? Trying to determine if I should redesign my code (which assumed unbound types would be carried forward) or just wait for this to land...

@gcnew
Copy link
Contributor

gcnew commented Apr 9, 2017

I wouldn't depend on it. Currently it's not on the roadmap and the core TS team doesn't look very interested in this feature. To support it, the compiler will need some fairly substantial changes, or better said a rearchitecture of the inferencer. I've layed out the major hurdles in this comment.

@arichiardi
Copy link

Just wanted to up this feature, because it requires very odd additions when working with higher-order functions (like in typed-typings/npm-ramda#175).

@mhegazy
Copy link
Contributor

mhegazy commented May 18, 2017

looks like a duplicate of #9366

@mhegazy mhegazy added Duplicate An existing issue was already created and removed Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript labels May 18, 2017
@KiaraGrouwstra
Copy link
Contributor

KiaraGrouwstra commented May 27, 2017

Fixed with #16072.

@gcnew
Copy link
Contributor

gcnew commented May 27, 2017

Fixed is a bit of an overstatement, but #16072 helps.

@KiaraGrouwstra
Copy link
Contributor

@gcnew I've yet to really test; would you have examples still failing?

@gcnew
Copy link
Contributor

gcnew commented May 27, 2017

This PR is a precursor for inferring higher order function types when no inferences can be made for one or more type parameters.

const wrapper = compose(x => [x], y => ({ p: y }));  // (x: {}) => { p: {}[] }

We currently infer (x: {}) => { p: {}[] }, but ideally we'd infer (x: A) => { p: A[] }. That's next on the list.

All polymorphic higher order uses still continue to fail as noted in the PR. For example, all flip tests that I used are still failing. What has changed is that if you provide a type annotation it will be used during inference, e.g.:

declare function flip<A, B, R>(f: (a: A, b: B) => R): (b: B, a: A) => R;
declare function fconst<X, Y>(x: X, y: Y): X;

const f1 = flip(fconst); // (b: {}, a: {}) => {}

const f2: (b: string, a: number) => number = flip(fconst); // OK, now it's working
const f3: (b: string, a: number) => string = flip(fconst); // Error correctly caught

// assignment works but all for the wrong reasons
// flip's A, B and R are inferred as `any` :/
const f4: <A, B>(a: A, b: B) => B = flip(fconst);

// uncaught error and implicit any not reported :/
const f5: <A, B>(a: A, b: B) => A = flip(fconst);

#16072 makes f2 type check while previously it didn't. All the rest are pretty much the same. f4 and f5 are now inferred with any instead of {}, however this makes no actual difference because of #5616.

@gcnew
Copy link
Contributor

gcnew commented Jun 6, 2017

I made a quick implementation of my suggestion from #16114 (comment). This work (attempt4) builds on #16104 and shows promising results, however there are still some important issues left. I do believe that this approach is working and the issues will be resolved with more effort put into them.

Issues:

  • when the invocation of a function yields a polymorphic function (e.g. flip(fconst)), the prototype hacks the resolved return type to contain the type parameters on itself. Thats incorrect and creates many problems.
  • mapped type inference doesn't always work
  • type paramaters sometimes leak out when they should be inferred as {}. This is very apparent when creating a class instance without providing the required type parameters. An alternative approach would be to implement skolems/rank 2 types but they don't seem to be the best fit.
  • contextually typed functions are not always fully checked
  • deferred constraints are not currently solved
  • top level literal types are not always handled as before

The test cases that I've used are here (gist). With a few exceptions, most of them yield what's expected.

PS: The code is not well written. My primary aim was to verify that the proposed logic could be made to work. If all goes well, I'll refactor it into something maintainable.

PS2: This version of the compiler cannot build itself. However, it works fine if built externally. I've modified the jakefile so that jake local builds the compiler with the last known good one.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 17, 2017

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@mhegazy mhegazy closed this as completed Aug 17, 2017
@zpdDG4gta8XKpMCd
Copy link
Author

@mhegazy problem still there, please reopen

@mhegazy
Copy link
Contributor

mhegazy commented Sep 21, 2017

It is a duplicate/same underlying issue as #9366.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
@ahejlsberg ahejlsberg added the Fixed A PR has been merged for this issue label Mar 8, 2019
@ahejlsberg
Copy link
Member

Fixed in #30215.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants