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

Implement partial type argument inference using the _ sigil #26349

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

@weswigham
Copy link
Member

@weswigham weswigham commented Aug 10, 2018

In this PR, we allow the _ sigil to appear in type argument lists in expression positions as a placeholder for locations where you would like inference to occur:

const instance = new Foo<_, string>(0, "");
const result = foo<_, string>(0, "");
const tagged = tag<_, string>`tags ${12} ${""}`;
const jsx = <Component<_, string> x={12} y="" cb={props => void (props.x.toFixed() + props.y.toUpperCase())} />;

This allows users to override a variable in a list of defaulted ones without actually explicitly providing the rest or allow a type variable to be inferred from another provided one.

Implements #26242.
Supersedes #23696.

Fixes #20122.
Fixes #10571.

Technically, this prevents you from passing a type named _ as a type argument (we do not reserve _ in general and don't think we need to). Our suggested workaround is simply to rename or alias the type you wish to pass. Eg,

interface _ { (): UnderscoreStatic; }

foo<_>(); // bad - triggers partial type inference, instead:

type Underscore = _;
foo<Underscore>(); // good

we did a quick check over at big ts query, and didn't find any public projects which passed a type named _ as a type argument in an expression/inference position, so it seems like a relatively safe care-out to make.

Prior work for the _ sigil for partial inference includes flow and f#, so it should end up being pretty familiar.

@alfaproject
Copy link

@alfaproject alfaproject commented Aug 10, 2018

Why write infer explicitly? Could we do like destructuring? <, string> instead of <infer, string>

By default, it would infer.

@weswigham
Copy link
Member Author

@weswigham weswigham commented Aug 10, 2018

@alfaproject I wrote my rationale down in #26242

@jwbay
Copy link
Contributor

@jwbay jwbay commented Aug 14, 2018

Would this PR enable this scenario? I didn't see a test quite like it. Basically extracting an inferred type parameter from a specified type parameter.

type Box<T> = { value: T };

type HasBoxedNumber = Box<number>;

declare function foo<T extends Box<S>, S>(arg: T): S;

declare const hbn: HasBoxedNumber;

foo<HasBoxedNumber, infer>(hbn).toFixed();
weswigham added 3 commits Aug 17, 2018
… fails)
@weswigham
Copy link
Member Author

@weswigham weswigham commented Aug 17, 2018

Based on the design meeting feedback, this has been swapped to variant 2 from the proposal - using the * sigil as a placeholder for inference. We'll need updates to our tmlanguage to get syntax highlighting right (although we already parsed * in type positions for jsdoc, so we probably should have already).

sheetalkamat added a commit to microsoft/TypeScript-TmLanguage that referenced this pull request Aug 17, 2018
@weswigham
Copy link
Member Author

@weswigham weswigham commented Aug 17, 2018

Would this PR enable this scenario? I didn't see a test quite like it. Basically extracting an inferred type parameter from a specified type parameter.

As is, no. Other type parameters (supplied or no) are not currently inference sites for a type parameter. We could enable it here (just by performing some extra inferType calls between the supplied types and their parameters' constraints), probably, but... should we? @ahejlsberg you have an opinion here?

@ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Aug 17, 2018

@ahejlsberg you have an opinion here?

I don't think we want constraints to be inference sites, at least not without some explicit indication. At some point we might consider allowing infer declarations in type parameter lists just as we do in conditional types:

type Unbox<T extends Box<infer U>> = U;

Though you can get pretty much the same effect with conditional types:

type Unbox<T extends Box<any>> = T extends Box<infer U> ? U : never;
@weswigham
Copy link
Member Author

@weswigham weswigham commented Aug 17, 2018

Alright, I'll leave this as is then and just mention that it's available as a branch if we ever change our minds in the future.

@weswigham weswigham changed the title Implement partial type argument inference using the infer keyword Implement partial type argument inference using the * sigil Aug 17, 2018
@treybrisbane
Copy link

@treybrisbane treybrisbane commented Aug 18, 2018

@weswigham It seems inconsistent (and kinda strange) to use the * sigil for this when we already use the infer keyword to denote explicit type inference...

type Tagged<O extends object, T> = O & { __tag: T };

// "Infer a type, and make it available under the alias 'T'"
declare function getTag<O extends Tagged<any, any>>(object: O): O extends Tagged<any, infer T> ? T : never;

// "Infer a type, and make it available to 'getTag' under the alias at the first type position"
getTag<infer>({ foo: string, __tag: 'bar' })
// => 'bar'

This seems like an obvious syntactic duality to me... What was the reason you instead decided to go with *?

@RyanCavanaugh
Copy link
Member

@RyanCavanaugh RyanCavanaugh commented Aug 21, 2018

The existing infer T keyword produces a new binding for T; this wouldn't be available in argument positions (e.g. you can't write getFoo<infer T, T>()). Having the infer keyword have arity 1 in conditional types and arity 0 in type argument positions seems like a decrease in overall consistency rather than an increase.

@KyleDavidE
Copy link

@KyleDavidE KyleDavidE commented Aug 22, 2018

It would probably be nice to be able to declare infer on the functions, ex: function foo<A, B = infer>(b: B, c: SomeComplexType<A,B>): SomeOtherComplexType<A,B>

@treybrisbane
Copy link

@treybrisbane treybrisbane commented Aug 23, 2018

@RyanCavanaugh

Having the infer keyword have arity 1 in conditional types and arity 0 in type argument positions seems like a decrease in overall consistency rather than an increase.

Thanks for the response. :)

Fair enough, but I'd argue that this decrease in consistency is far less than that of introducing an entirely new sigil for this purpose. Is there really a benefit to users in using such a radically different syntax for something whose only difference to infer T is the arity?

@treybrisbane
Copy link

@treybrisbane treybrisbane commented Aug 23, 2018

Something else to consider is that TypeScript supports JSDoc, and * in JSDoc means any. I'm not sure it's a good idea to reuse a symbol that means any in one context for something that means "please infer this type for me" in another context.

If we're concerned about making operators/keywords context-sensitive, then again it seems like making infer context-sensitive is far less of an evil than doing the same for *.

@insidewhy
Copy link

@insidewhy insidewhy commented Aug 31, 2018

I don't mind * as it jives with flow. Users of typescript can just avoid * in jsdoc and always use any for the purpose easily enough?

I'd also like to see this:

const instance = new Blah<T, **>(1, 'b', false, new Date())

I have a class that bundles many string literal types and I have to enumerate them all at every callsite even when I'm using the code from this branch. Everytime I add a new string literal I have to update every single callsite which is a massive drag ;)

@insidewhy
Copy link

@insidewhy insidewhy commented Sep 1, 2018

Consider:

type LiteralMap<S1 extends string, S2 extends string, S3 extends string> = {
  item1: S1,
  item2: S2,
  item3: S3
}

With this feature at every definition using this type I have to use:

function user(map: LiteralMap<*, *, *>) {}

Now if I need to add a new literal to my map I have to update this to:

type LiteralMap<S1 extends string, S2 extends string, S3 extends string, S4 extends string> = {
  item1: S1,
  item2: S2,
  item3: S3,
  item4: S4,
}

which is no big deal, but now I also have to update every single use of this to:

function user(map: LiteralMap<*, *, *, *>) {}

With LiteralMap<**> I can just update the definition without affecting every area it is used.

@svieira svieira mentioned this pull request Sep 1, 2018
4 of 4 tasks complete
@xaviergonz
Copy link

@xaviergonz xaviergonz commented Sep 1, 2018

Or it could follow the tuple system

type LiteralMap<S1?, S2?, S3?> = {
  item1: S1,
  item2: S2,
  item3: S3
}

function user(map: LiteralMap) {} // infer, infer, infer
function user(map: LiteralMap<boolean>) {} // boolean, infer, infer
function user(map: LiteralMap<_, boolean>) {} // infer, boolean, infer

type LiteralMap<S1, S2, S3?> = {
  item1: S1,
  item2: S2,
  item3: S3
}

function user(map: LiteralMap) {} // not allowed, S1 and S2 missing
function user(map: LiteralMap<boolean>) {} // not allowed, S2 missing
function user(map: LiteralMap<_, boolean>) {} // infer, boolean, infer

alternatively it could use the default assignation (which I guess makes more sense, since if you want it to infer the default type makes no sense?)

type LiteralMap<S1 = _, S2 = _, S3 = _> = {
  item1: S1,
  item2: S2,
  item3: S3
}

function user(map: LiteralMap) {} // infer, infer, infer
function user(map: LiteralMap<boolean>) {} // boolean, infer, infer
function user(map: LiteralMap<*, boolean>) {} // infer, boolean, infer

type LiteralMap<S1, S2, S3 = _> = {
  item1: S1,
  item2: S2,
  item3: S3
}

function user(map: LiteralMap) {} // not allowed, S1 and S2 missing
function user(map: LiteralMap<boolean>) {} // not allowed, S2 missing
function user(map: LiteralMap<_, boolean>) {} // infer, boolean, infer
@MrLoh
Copy link

@MrLoh MrLoh commented Aug 2, 2020

It would be great to see this merged, no matter what syntax is used.

@jamiepine
Copy link

@jamiepine jamiepine commented Aug 6, 2020

Also bumping this, would love the keyword "infer". Seems clean and verbose.

@ivoiv
Copy link

@ivoiv ivoiv commented Oct 2, 2020

Yet another bump for this feature.

There are many situations where you need to specify some argument types, but infer others for certain concepts to work.

Returning a new function or using a wrapper to take the inferable parameters does work around the issue, but it's an odd design choice and I feel like most developers don't understand the need or reasoning for it.

@jamiepine
Copy link

@jamiepine jamiepine commented Oct 24, 2020

This is... interesting. If it's for compatibility concerns can't we just set a new compiler flag on this and get on with it?

A compiler flag would not be ideal, for those who are writing libs used by others and need generic inference part of Typescript natively.

@tannerlinsley
Copy link

@tannerlinsley tannerlinsley commented Feb 22, 2021

From someone who is relatively young in the TS world, I encountered this pretty early on while building libraries like React Query, React Table etc and can't express how necessary I think this feature is. Here are some of my recent thoughts on this topic: https://twitter.com/tannerlinsley/status/1363926547489456129

@Ericnr
Copy link

@Ericnr Ericnr commented Feb 22, 2021

@tannerlinsley I ran into this limitation pretty early on too when trying to implement a type safe api for defining graphql schemas. The current solutions either sacrifice their api ergonomics or relies on code generation to guarantee type safety

@yeegor
Copy link

@yeegor yeegor commented Mar 17, 2021

It's sad to see that the work on this seems to have come to a halt.
Implementing would-be-convenient functionality now requires insane workarounds.

Bumping this again, in hope to see this released ♥️

@btoo
Copy link

@btoo btoo commented Mar 18, 2021

This reminds me of #7481 (comment): if there was a way (besides casting) to ensure that an expression or identifier satisfies some type, we could have partial type inference without introducing a new sigil (although it would require introducing a new operator):

const instance implements Foo<any, string> = new Foo(0, "");
// typeof instance extends Foo<number, string>

A feature like this would allow partial type inference anywhere, not just with function type parameters:

const instance implements { a: any, b: string } = { a: 0, b: "", c: true }
// typeof instance extends { a: number, b: string, c: boolean }

apologies if I'm derailing the thread

@yeegor
Copy link

@yeegor yeegor commented Mar 23, 2021

@ahejlsberg @RyanCavanaugh @andy-ms @DanielRosenwasser could you please update us on status of this?

@Andarist
Copy link
Contributor

@Andarist Andarist commented Apr 11, 2021

This is not stalled for lack of ownership, rather, for lack of drive to accept the feature in the first place.

@weswigham is this the priority issue or is this blocked because the TS team thinks that this is not something that should be done?

Regarding inferring the trailing type parameters automatically - I've noticed a comment from @RyanCavanaugh in #39008 (comment) that:

When any type arguments are specified, the remaining arguments are set to their defaults rather than going through inference. IMHO this was probably a mistake, but it was written that way on purpose and would be too large of a breaking change to modify at this point

This is, fortunately, an orthogonal problem so it's not necessarily tied to this very PR. But given this feels like the most appropriate place to ask for this at the moment - could this be somehow quantified? I would expect that this is something that surprises people quite a bit and it's something that is rarely wanted. Maybe worth implementing a change for this using compiler flag at first?


I care about this very PR so much that I could try to help out with rebasing this and doing anything that would be required to get this merged in but, of course, I would have to get a green light from your team on that one first.

@jamesopstad
Copy link

@jamesopstad jamesopstad commented Apr 11, 2021

Regarding inferring the trailing type parameters automatically - I've noticed a comment from @RyanCavanaugh in #39008 (comment) that:

When any type arguments are specified, the remaining arguments are set to their defaults rather than going through inference. IMHO this was probably a mistake, but it was written that way on purpose and would be too large of a breaking change to modify at this point

This is, fortunately, an orthogonal problem so it's not necessarily tied to this very PR. But given this feels like the most appropriate place to ask for this at the moment - could this be somehow quantified? I would expect that this is something that surprises people quite a bit and it's something that is rarely wanted. Maybe worth implementing a change for this using compiler flag at first?

As mentioned in #10571 (comment), this could be resolved by allowing the sigil as a type parameter default. Allowing the new placeholder to be used both in type argument lists and default values gives the most flexibility. Personally, I prefer infer to _ but I don't really mind as long as this moves forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment