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

preferinfer modifier for type parameters #52241

Closed
wants to merge 2 commits into from

Conversation

Andarist
Copy link
Contributor

This is not yet complete - it's an idea I have and one that I want to have feedback on since some way to allow partial inference is highly in demand by library author and type parameter modifiers seems to be a natural fit for this.

I think that in reality, people would most likely opt into partial inference for the whole signature - but I'm not sure if you'd like to invent a new syntax-level concept just for this purpose. One alternative, with a completely different syntax, that I can imagine would be something like this:

declare function fn<preferinfer: A, B>(a: A, b: B): [A, B]
fn<string>('foo', 100)

This kind of thing could serve as a modifier for the list of type parameters.

The implementation here builds heavily on the @weswigham's work from #26349 . The mechanism to opt into partial inference is different though. The other PR required opt-in from the caller - whereas this one here requires opt-in by the declaration author. I would consider this a major DX improvement. The downside is that this PR here only allows for inferring trailing type parameters (in the partial type parameters lists) - but I think it's a reasonable limitation/a fair tradeoff.

supersedes #26349
closes #26242
closes #10571.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jan 14, 2023
@typescript-bot
Copy link
Collaborator

The TypeScript team hasn't accepted the linked issue #26242. If you can get it accepted, this PR will have a better chance of being reviewed.

@Andarist Andarist force-pushed the preferinfer branch 3 times, most recently from c06ba17 to ea45dc2 Compare January 14, 2023 17:32
@devanshj
Copy link

The other PR required opt-in from the caller - whereas this one here requires opt-in by the declaration author. I would consider this a major DX improvement.

This is half of the story the other half is where this is worse than #26349. It's worse because now I have to rely on the callee to "allow me" to pass in partial type arguments, and why should the callee be allowed to control my ability to pass in partial type arguments or not, it's irrelevant to the callee. That is let's say there's a function g: <T, U>(...) => ... here I wish to pass T and infer U, with #26349 I can simply write g<T, _>(...) but with this PR I need to "ask" to the callee to mark U as preferinfer and "allow me" to pass partial type arguments as g<T>(...), why should one need to drag the callee here (imagine people opening PRs for this), the caller wanting to infer U has almost nothing to do with the callee.

But I also understand the other half where it's an improvement, eg if we have something like f: <T, InternalU>(...) => ..., the improvement here afaiu would be that we need not rely or educate callers of f about partial type arguments ie we need not tell them "Hey that InternalU is an internal thing that gets inferred anyway but to allow inferring it you'll have to pass in _ so your call would look like f<MyT, _>(...)", all this messaging is not needed with this PR because now the callee can simply mark InternalU as preferinfer and the caller just passes MyT ie writes f<MyT>(...) as usual without having to deal with anything extra.

But we can achieve what this PR achieves with #26349 also, simply by declaring f as f: <T, InternalU = _>(...) => ... this would be equivalent of writing f: <T, preferinfer InternalU>(...) => .... I'm not sure if #26349 implements this already or not but it can done.

So what I'm saying is that this PR doesn't just solve a problem, it also creates another one (ie having to "ask" the callee to "allow" partial type parameter inference). But if #26349 allows (if it already doesn't) _ as the default type parameter type then it solves the same problem this PR solves but doesn't create a new one.

I hope I've got things right, just my two cents.

@Andarist
Copy link
Contributor Author

But I also understand the other half where it's an improvement, eg if we have something like f: <T, InternalU>(...) => ..., the improvement here afaiu would be that we need not rely or educate callers of f about partial type arguments ie we need not tell them "Hey that InternalU is an internal thing that gets inferred anyway but to allow inferring it you'll have to pass in _ so your call would look like f<MyT, _>(...)", all this messaging is not needed with this PR because now the callee can simply mark InternalU as preferinfer and the caller just passes MyT ie writes f(...) as usual without having to deal with anything extra.

This is my primary motivation behind this PR.

But we can achieve what this PR achieves with #26349 also, simply by declaring f as f: <T, InternalU = _>(...) => ... this would be equivalent of writing f: <T, preferinfer InternalU>(...) => .... I'm not sure if #26349 implements this already or not but it can done.

This probably isn't as easy as it sounds here because InternalU might need a reasonable default in case it fails to be inferred.

imagine people opening PRs for this

I agree that I didn't think this part through.


The bottom line for me is that some mechanism for this is needed. This is one of the most requested TS features - especially desired by library authors. I don't care much about the actual mechanism that will allow this - I could even live with _. The PR for _ has been open since 2018 and didn't move forward - I'm not sure why that is. I'm just trying to revive the conversation about this pressuring need with this new take on the feature.

I also mention in the PR description that this isn't a 1-to-1 replacement for _. Both could co-exist. Since my use case is closer to InternalU, I prefer not to force _ on the caller - it's my callee that decides about this parameter being "internal". _ would certainly be a workable (but not ideal) solution for us though.

@devanshj
Copy link

devanshj commented Jan 23, 2023

This probably isn't as easy as it sounds here because InternalU might need a reasonable default in case it fails to be inferred.

I don't follow this, what would happen in case of an inference failure is what happens today, ie the type parameter gets resolved to the constraint or whatever. I'm not suggesting a new behavior, g<T>() would be just a sugar for writing g<T, _>() and we understand the behavior of latter.

In case you're asking if <preferinfer A extends B>(...) is equivalent to <A extends B = _>(...) then we what is the equivalent of <preferinfer A extends B = C>(...) then the answer if we can make up a way/syntax to express that, eg we can say it'd be <A extends B = _ | C>(...), ie here | doesn't actually mean a union it's just a hint for compiler.

But again the mechanism and behavior of what should happen when is already there in #26349, it's just about bringing a new syntactic aid ie = _, so it's definitely easy, or at least that's what I call easy.

I also mention in the PR description that this isn't a 1-to-1 replacement for _. Both could co-exist.

I couldn't find where you mention or indicate that. Moreover you also explicitly write...

supersedes #26349
closes #26242
closes #10571

...which indicates otherwise.

Overall I agree there's a need for this kind of feature, but as I said it's better to extend #26349 with = _ that'd be simple and well understood.

@Andarist
Copy link
Contributor Author

I don't follow this, what would happen in case of an inference failure is what happens today, ie the type parameter gets resolved to the constraint or whatever. I'm not suggesting a new behavior, g() would be just a sugar for writing g<T, _>() and we understand the behavior of latter.

The constraint is usually the default too - but that's not always the case. You can use different values for both (within the boundaries of the allowed subtype relationship). It's not necessarily something that I'd like to do but this proposal removes such a possibility (since you use _ as a default type argument).

In case you're asking if (...) is equivalent to (...) then we what is the equivalent of (...) then the answer if we can make up a way/syntax to express that, eg we can say it'd be <A extends B = _ | C>(...), ie here | doesn't actually mean a union it's just a hint for compiler.

Yeah, this is what I was getting at. I don't think that overloading the unions to add hints for the compiler would be a good syntax for this but perhaps something different could be used.

I couldn't find where you mention or indicate that. Moreover you also explicitly write...

Oh, sorry! I could swear that I included such a note somewhere in the description but apparently, I didn't. It is a given though - since this doesn't preclude merging that other PR as well. As you have observed - this PR mainly fixes the problem of "internal" type parameters, which is a slightly different problem than the one solved by that other PR. I mostly copied the issue references from the other PR as the overlap is, IMHO, sufficient enough. I've seen PRs addressing some of the issues by providing alternatives etc and not necessarily by implementing the requested feature/bug fix "literally". I don't have a problem with tweaking those links to smth like "relates to" though. That's not really that important to me here 😉

@devanshj
Copy link

I don't think that overloading the unions to add hints for the compiler would be a good syntax for this but perhaps something different could be used.

Yeah that's a secondary issue. Another option is instead of <A = _> we could use <A?> that actually looks much more clear and well understood. And then <A extends B = _ | C> simply becomes <(A extends B)? = C>. (? outside following that it's [(infer A extends B)?] and not [infer A? extends B])

Oh, sorry! I could swear that I included such a note somewhere in the description but apparently, I didn't.

Haha, no problems.

@sandersn sandersn marked this pull request as draft February 1, 2023 19:06
@sandersn
Copy link
Member

sandersn commented Feb 1, 2023

@Andarist I'm marked this as a draft since the design of partial inference was never finished.

@Andarist
Copy link
Contributor Author

Andarist commented Feb 1, 2023

@sandersn that’s understandable. Is there any chance that we could revive the design process of this feature anyhow? It’s highly requested and I would love to work on the actual implementation. Perhaps as a 5.1 goal? 😉

@sandersn
Copy link
Member

sandersn commented Feb 3, 2023

@Andarist the first step in the design process is to open a bug to discuss the design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
5 participants