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

infer function generic signature from actual function's return type #49618

Closed
4 of 5 tasks
trusktr opened this issue Jun 21, 2022 · 16 comments
Closed
4 of 5 tasks

infer function generic signature from actual function's return type #49618

trusktr opened this issue Jun 21, 2022 · 16 comments
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@trusktr
Copy link
Contributor

trusktr commented Jun 21, 2022

Suggestion

function test<T>(fn: (prev: T) => T) { }
test((prev) => ({ a: 1 })); // T is inferred as "unknown"

🔍 Search Terms

Maybe an issue exists, but I wasn't sure what to search for. "Infer generic function type from return value" didn't really help.

✅ Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code. not sure
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

T should be inferred as the type of the returned object, {a: number}

📃 Motivating Example

https://www.typescriptlang.org/play?#code/GYVwdgxgLglg9mABFApgZygHgCoD4AUwYAXIvgA4BOKAbqdgJSIC8uijiA3ogL4BQqDPgrUaTVmW4BDUgEZeDBgG4+QA

💻 Use Cases

make life easier

@Retsam
Copy link

Retsam commented Jun 21, 2022

It's not that TS doesn't infer the generics, in general, but just that it's inferring it as unknown in this specific case.

test(() => ({ a: 1 })); // T is inferred as "{ a: number }" as you might expect

It looks like the issue here is the unannotated prev, since that's unknown, T ends up as unknown.

In this very simple case (where prev is unused), it's fairly easy to see what the right behavior would be, but I suspect that evaluating the return type of a function and using that to infer the type of its arguments (which usually affect the return type) is not a simple thing to do.

@RyanCavanaugh
Copy link
Member

Two cases to consider here

The first case is where the function expression uses the parameter but the return type of the function provably doesn't depend on the parameter type. In practice, it's very rare for these kinds of function expressions to exist - they're by definition impure, and given control flow effects it's very difficult to even construct such a function.

The second case is where the function expression doesn't use the parameter, as is the case here. They can be removed WLOG, and after doing so the parameter is inferred successfully as expected.

So on balance there's not really much gain to be had here - the inference is still sound, and detecting if a used parameter has no effect on the return type of a function is a very difficult calculation to always get right

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds labels Jun 21, 2022
@fatcerberus
Copy link

WLOG

I had to google this acronym. First time I've ever encountered it.

To me it feels like TS should be treating (prev) => 42 as equivalent to () => 42 for the purpose of generic inference--that is to say, prev is not a valid inference site for T since it has no type annotation and can only later be typed through contextual typing--which requires T to be known first. If it weren't for the generic, (prev) => 42 would be an implicit any error. Effectively, the presence of the generic creates this weird pseudo-circular situation where T is indirectly inferred from its own constraint via contextual typing, which feels... wrong.

@RyanCavanaugh
Copy link
Member

Yeah, I'll reopen #47599 since it's not fully covered

@RyanCavanaugh
Copy link
Member

That said, I'm not really sure what the endgoal is. Any "improvement" we make here is just going to sow a bunch of "TypeScript is inconsistent, therefore has bug" reports because people will wonder why

function test<T>(fn: (prev: T) => T) { }
test((prev) => {
  return 0;
});

works but not

function test<T>(fn: (prev: T) => T) { }
test((prev) => {
  return prev ? 0 : 1;
});

The current behavior is at least very explainable and easy to reason about; moving the needle into the grey zone just raises more questions than it does solve problems.

@fatcerberus
Copy link

fatcerberus commented Jun 21, 2022

Yeah, I understand. What bothers me is mostly theoretical - the fact that the current behavior basically amounts to:

  1. What is T?
  2. Ooh, prev is a T, we can infer T from the callback the caller passed in...
  3. ...except prev has no type annotation!
  4. Contextual typing says prev is a T, but we don't know what T is yet...
  5. Screw it, we'll say prev: unknown (i.e. the constraint of T).
  6. Therefore, T is unknown.

It feels like a bug, even if the "correct" behavior isn't really qualitatively "better" in practice.

@fatcerberus
Copy link

To be clear, my problem isn’t the prev: unknown part (that part makes sense), but the fact that T ultimately has its own constraint as its inference site. I guess there’s no mechanism to fix the latter without compromising the former, though.

@RyanCavanaugh
Copy link
Member

I believe what actually happens is that we collect candidates for T, find out there are none, so default it to its constraint, which is unknown, and then process the call as if you had written test<unknown>(.

It's weird since if you had written test<unknown>(, that definitely shouldn't be an implicit any error. Maybe we need to make a marker unknown to use in zero-candidate inference that isn't allowed to contextually type a parameter - worth experimenting with, probably.

@fatcerberus
Copy link

fatcerberus commented Jun 21, 2022

I think a big part of what makes this case so tricky is that T is found in both covariant and contravariant positions. If you infer only from the covariant (return) position, you’re likely to end up with a type that’s too narrow, as you show in the examples above. I think the only change that would make sense is for this to become an error, a la implicit-any, as in the absence of a type annotation on the callback parameter, the contravariant position can’t be inferred from without always "inferring" the constraint. I acknowledge turning this into an error is a potentially disruptive breaking change, though.

@RyanCavanaugh
Copy link
Member

@weswigham pointed out this example

type Box<T> = { contents: T };
function test<T>(fn: (prev: Box<T>) => T) { }
test((prev) => ({ a: 1 })); 

It's not clear how we'd turn this into an error -- somehow it's (speculatively) a "Box<implicit any>" which isn't really a thing - implicit any arises when a binding should have a contextual type but doesn't, but that's not what's happening here. And zero-candidate inference is not a manifest error either:

function test<T>(x?: T): T;
test();

So there seems to be some difficulty in establishing what rule exactly would turn the OP example into an error without either breaking something that shouldn't be broken (benign zero-candidate inference), failing to break something equally suspect (Box<), or both.

@blaumeise20
Copy link
Contributor

Does TypeScript have a Hindley-Milner type system? Because I'm pretty sure it would work with that kind of type inference.

TypeScript would see that the return type and first parameter have to be the same, so it could check for return type and see "oh it's an object literal with type { a: number }". And because it knows that prev must be of the same type, it can say that prev is of type { a: number } too.

It seems like it doesn't work like that now, right?

@fatcerberus
Copy link

fatcerberus commented Jun 22, 2022

No, TS's type system is not H-M. Type inference is done locally. See #30134.

Your example of inferring prev based on the return type was discussed above:

function test<T>(fn: (prev: T) => T) { }
test((prev) => {
  return prev ? 0 : 1;
});

would infer T = 0 | 1 under your proposed behavior, which isn't ideal (number would be more useful). In general this behavior would tend to infer types that are too narrow. It's a tricky case because you generally want to infer wider types for a parameter but narrower types for a return type, but here they're required to be the same type.

@trusktr
Copy link
Contributor Author

trusktr commented Aug 26, 2022

Interesting points above.

I can describe a real-world use case. Suppose we have a memo function that is reactive, and re-runs an expression any time dependencies used inside of the function body change. The memo tool can be called in two ways. The first way:

const fname = reactiveVar('John')
const lname = reactiveVar('Doe')

const fullname = memo((prev) => {
  return fname.get() + ' ' + lname.get()
})

fullname() // string, combination of fname + lname

In this first case, prev arguments can only ever be string | undefined, where on the very first run of the reactive expression the initial prev value is undefined. It can never be anything other than string | undefined. Note though that the return value is never string | undefined but just string.

Any time that fname.set(string) or lname.set(string) are called, it causes the function passed to memo to re-execute to evaluate the new fullname and trigger other reactive expressions elsewhere that depend on fullname.

The second way to use the memo API is like this:

const fname = reactiveVar('John')
const lname = reactiveVar('Doe')

const fullname = memo((prev) => {
  return fname.get() + ' ' + lname.get()
}, "Godzilla Kong") // <--------------------------- HERE, new movie idea

fullname() // string, combination of fname + lname

In this case, prev arguments will only ever be strings. They will never be anything else, because the initial value use for prev will be "Godzilla Kong", and the return value is always a string.

This works totally fine in plain JavaScript, but the main issue is that in TypeScript, it currently requires too much superfluous type annotation.

@RyanCavanaugh
Copy link
Member

@trusktr did you mean to use prev in these examples? Again we have the problem of "you can just omit the parameter".

@btakita
Copy link

btakita commented Jun 7, 2023

I have a real-world use case related to a PR in nanostores.

I'm adding a cx function into the computed function. The cx function takes a callback & causes the computed to auto-listen to dependencies by adding itself to a global stack while the callback executes. I'd like to have the return type of the cx function be the return type of the callback.

Here is a contrived example to keep it simple. The real use case involves async, so I'll demonstrate with a sleep.

let firstName = atom('')
let lastName = atom('')
let $userId = atom(0)
let fullNameUserId = computed(async cx => {
  await sleep(100)
  let userId = cx(() => $userId()) // Should infer as a number but infers as any
  return cx(() => `${firstName()}-${lastName()}-${userId}`) // Should infer as a string but infers as any
})
function sleep(ms: number) {
  return new Promise(res => {
    setTimeout(() => res(null), ms)
  })
}
declare function computed<Value extends any, Cx extends ComputedCx<any>>(cx: Cx): ReadableAtom<Value>
type ComputedCx<R> = (cb: () => R) => R
declare function atom<Value>(initialValue: Value): WritableAtom<Value = any>
interface ReadableAtom<Value = any> {
  // ...
}
interface WritableAtom<Value = any> extends ReadableAtom<Value> {
  // ...
}

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Too Complex" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

No branches or pull requests

7 participants