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

Probable bug: Contextual inference does not work in "object-like" scenarios #44999

Closed
devanshj opened this issue Jul 13, 2021 · 5 comments
Closed
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@devanshj
Copy link

devanshj commented Jul 13, 2021

Bug Report

πŸ”Ž Search Terms

Contextual inference

πŸ•— Version & Regression Information

tested with 4.4.0-beta

⏯ Playground Link

Playground

πŸ’» Code

createMachine({
  context: { foo: 1 },
  entry: {
    type: "xstate.assign",
    exec: c => ({ foo: c.foo + 1 })
  }
})

createMachine({
  context: { foo: 1 },
  entry: assign(c => ({ foo: c.foo + 1 })) // c is unknown but should be { foo: number } instead
})

declare const createMachine: <C>(defintion: Machine<C>) => {}
type Machine<C> = {
  context: C,
  entry: {
    type: string,
    exec: (context: C) => unknown
  }
}

declare const assign: <C>(f: (context: C) => C) =>
  { type: "xstate.assign", exec: (context: C) => C }

πŸ™ Actual behavior

assign infers type parameter C as unknown

πŸ™‚ Expected behavior

assign should infer type parameter C as { foo: number }

Filing as Ryan said here. Fwiw I think a more minimal repro would be this.

@devanshj
Copy link
Author

devanshj commented Jul 17, 2021

@RyanCavanaugh I remember you said "I knew it'd be interesting (usually implies time-consuming, but not this time)" so is this a trivial fix? Because if so and if possible I'd love to have this in 4.4 or maybe the next milestone (this directly impacts the xstate types I'm working on at this very moment and the workaround I use is pretty nasty). Thanks!

@ahejlsberg
Copy link
Member

Yeah, this is a tricky one, and effectively a design limitation. The issue is captured in this comment in resolveCallExpression:

When a call to a generic function is an argument to an outer call to a generic function for which
inference is in process, we have a choice to make. If the inner call relies on inferences made from
its contextual type to its return type, deferring the inner call processing allows the best possible
contextual type to accumulate. But if the outer call relies on inferences made from the return type of
the inner call, the inner call should be processed early. There's no sure way to know which choice is
right (only a full unification algorithm can determine that), so we resort to the following heuristic:
If no type arguments are specified in the inner call and at least one call signature is generic and
returns a function type, we choose to defer processing. This narrowly permits function composition
operators to flow inferences through return types, but otherwise processes calls right away. We
use the resolvingSignature singleton to indicate that we deferred processing. This result will be
propagated out and eventually turned into nonInferrableType (a type that is assignable to anything and
from which we never make inferences).

The comment also explains why your workaround works--adding a call signature to the return type of assign causes us to defer processing of the inner call.

@ahejlsberg ahejlsberg added Design Limitation Constraints of the existing architecture prevent this from being fixed and removed Bug A bug in TypeScript labels Jul 20, 2021
@ahejlsberg ahejlsberg removed this from the Backlog milestone Jul 20, 2021
@devanshj
Copy link
Author

devanshj commented Jul 20, 2021

Thanks a lot for your answer @ahejlsberg! Yeah I totally understand TypeScript's problem and even agree that the heuristic it applies is a good one (xstate just happens to have some very corner case scenarios), heck that's what made me come up with the workaround because intuitively I knew what's going on haha.

Though if I were to make a critic, I think the foundational problem is TypeScript (to put it naively) doesn't leverage or understand that some functions are essentially "sugars", it can be very easily identified if I write assign as <F>(f: F) => { type: "xstate.assign", exec: F }. The fact that inlining assign compiles says that compiler doesn't really understand fully that assign is essentially just a sugar. I also lay this out in depth in #45035 (I would LOVE it you can share your thoughts on it, it's a little messy in the beginning because I couldn't point my finger on it but later in the comments it's clear).

It would be cool if TypeScript perhaps uses some heuristic to identity sugar-like functions, or if I were to propose a solution it would be that TypeScript provides a type that allows the user to mark an inference site to be of lesser priority so that we can type assign like <F>(f: LowInfer<F>) => { type: "xstate.assign", exec: F }. (It's important that it's LowInfer and not NoInfer I'll add a comment in #45035 (edit: here you go) to point out how it's different)

Thanks again for your time!

@Andarist
Copy link
Contributor

@ahejlsberg being able to inform TS, even "manually", with some kind of annotation which path do we want to take here would be super lovely for some libraries (like XState πŸ˜…). This would be a powerful tool for library authors.

I'm slightly unsure how to treat this issue now because it has been classified as a Design Limitation but it has not been closed. Is this something that is still under consideration but perhaps it has a super low priority? I just wonder if you might ever revisit this kind of thing.

@RyanCavanaugh
Copy link
Member

We haven't updated the automation to automatically close these. Anything classed this way is in an indefinite "hope we can figure it out eventually!" state; things are revisited naturally as people continue to encounter them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

4 participants