microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.4k stars 12.4k forks source link

Nested call not inferred correctly when a conditional type tries to route to the final expected type #46201

Open Andarist opened 2 years ago

Andarist commented 2 years ago

Bug Report

πŸ”Ž Search Terms

inference, type parameters, conditional type, signature resolving

πŸ•— Version & Regression Information

This is the behavior present in all 4.x versions available on the playground, including 4.5-beta.

⏯ Playground Link

Slimmed down repro case

The originally reported TS playground

πŸ’» Code

This is the code for the slimmed-down variant. Note that the createMachine2 is not a part of the repro case but it shows how putting the conditional type on a "different level" makes it work - which I find to be surprising because semantically both variants do the same thing

interface EventObject { type: string; }
interface TypegenDisabled { "@@xstate/typegen": false; }
interface TypegenEnabled { "@@xstate/typegen": true; }

type TypegenConstraint = TypegenEnabled | TypegenDisabled;

interface ActionObject<TEvent extends EventObject> {
  type: string;
  _TE?: TEvent;
}

declare function assign<TEvent extends EventObject>(
  assignment: (ev: TEvent) => void
): ActionObject<TEvent>;

declare function createMachine<
  TTypesMeta extends TypegenConstraint = TypegenDisabled
>(
  config: {
    types?: TTypesMeta;
  },
  action?: TTypesMeta extends TypegenEnabled
    ? { action: ActionObject<{ type: "WITH_TYPEGEN" }> }
    : { action: ActionObject<{ type: "WITHOUT_TYPEGEN" }> }
): void;

createMachine(
  {
    types: {} as TypegenEnabled,
  },
  {
    // `action` property is of a correct type - it expects `ActionObject<{ type: "WITH_TYPEGEN" }> }`
    action: assign((event) => {
      // but the type of this `assign` has been inferred to `ActionObject<{ type: "WITHOUT_TYPEGEN" } | { type: "WITH_TYPEGEN" }>`
      // so `event` here is of type `{ type: "WITHOUT_TYPEGEN" } | { type: "WITH_TYPEGEN" }`
      ((_accept: "WITH_TYPEGEN") => {})(event.type);
    }),
  }
);

// below we have the code that is not part of the repro case but which is semantically the same as the one above and yet it behaves differently

declare function createMachine2<
  TTypesMeta extends TypegenConstraint = TypegenDisabled
>(
  config: {
    types?: TTypesMeta;
  },
  action?: {
    // it works correctly if we check the `TTypesMeta` further down the road
    action: TTypesMeta extends TypegenEnabled
      ? ActionObject<{ type: "WITH_TYPEGEN" }>
      : ActionObject<{ type: "WITHOUT_TYPEGEN" }>;
  }
): void;

createMachine2(
  {
    types: {} as TypegenEnabled,
  },
  {
    action: assign((event) => {
      ((_accept: "WITH_TYPEGEN") => {})(event.type);
    }),
  }
);

The code below showcases more accurately what I'm trying to achieve.

Code from the originally reported playground ```ts type Cast = T extends TCastType ? T : TCastType; type Prop = K extends keyof T ? T[K] : never; type IndexByType = { [K in T["type"]]: Extract; }; interface EventObject { type: string; } interface TypegenDisabled { "@@xstate/typegen": false; } interface TypegenEnabled { "@@xstate/typegen": true; } type TypegenConstraint = TypegenEnabled | TypegenDisabled; type ResolveTypegenMeta< TTypesMeta extends TypegenConstraint, TEvent extends { type: string } > = TTypesMeta extends TypegenEnabled ? TTypesMeta & { indexedEvents: IndexByType; } : TypegenDisabled; interface ActionObject { type: string; _TE?: TEvent; } interface MachineOptions { actions?: { [name: string]: ActionObject; }; } type TypegenMachineOptions< TResolvedTypesMeta, TEventsCausingActions = Prop, TIndexedEvents = Prop > = { actions?: { [K in keyof TEventsCausingActions]?: ActionObject< Cast, EventObject> >; }; }; type MaybeTypegenMachineOptions< TEvent extends EventObject, TResolvedTypesMeta = TypegenDisabled > = TResolvedTypesMeta extends TypegenEnabled ? TypegenMachineOptions : MachineOptions; declare function assign( assignment: (ev: TEvent) => void ): ActionObject; declare function createMachine< TEvent extends { type: string }, TTypesMeta extends TypegenConstraint = TypegenDisabled, >( config: { schema?: { events?: TEvent; }; types?: TTypesMeta; }, options?: MaybeTypegenMachineOptions> ): void; interface TypesMeta extends TypegenEnabled { eventsCausingActions: { actionName: "BAR"; }; } createMachine( { types: {} as TypesMeta, schema: { events: {} as { type: "FOO" } | { type: "BAR"; value: string }, }, }, { actions: { // `event` here is not narrowed down to the BAR one // yet the `actionName`'s type (available when I hover over the property) is correct actionName: assign((event) => { ((_accept: "BAR") => {})(event.type); }), }, } ); ```

πŸ™ Actual behavior

event parameter here is not narrowed down to the WITH_TYPEGEN event, yet the actionName's type (available when I hover over the property) is "correct" (only WITH_TYPEGEN event there)

πŸ™‚ Expected behavior

I would expect this event parameter to be inferred correctly.


I've been debugging this for a bit and I think this is roughly what happens:

  1. we have nested chooseOverload+inferTypeArguments calls because assign happens "within" createMachine
  2. typeParameters for assign are assigned based on both branches of the conditional type - basically, a union of both is created for this position and since there is an overlap the reduced type gets assigned there.
  3. the "routing" conditional type gets instantiated only after we exit the outer inferTypeArguments and it happens within getSignatureApplicabilityError but the assign has been instantiated and cached a step back - within inferTypeArguments
  4. the inferred type for assign is never rechecked - so it stays as if it was supposed to handle both branches of the conditional type

Note that this might be highly incorrect as I'm not too familiar with the codebase

andrewbranch commented 2 years ago

I think a more minimal repro would help, if you can come up with one.

Andarist commented 2 years ago

@andrewbranch I've managed to slim down the repro further (added to the original post). Interestingly, I've also been able to create a semantically equivalent variant that works (in my real code it's way harder to apply the same technique though). The difference between both might be an outcome of some implementation decision but for me, a user, it's hard to assess that and it's also hard to know how I can work around issues like this when they pop up.

If you have any pointers as to why this happens, or how this possibly could be fixed in the TS compiler then I could try to work on that said fix (if it would not be too involved, ofc). I've spent my fair share of time in the debugger with tsc lately, while investigating this and some other issues - and I would love to continue my journey with the codebase πŸ˜‰

Andarist commented 2 years ago

@millsp has found out that wrapping this conditional type in NoOp type "fixes" the problem:

diff --git a/repro_case.ts b/repro_case.ts
index 37eb31f9e..c78f843e7 100644
--- a/repro_case.ts
+++ b/repro_case.ts
@@ -20,15 +20,19 @@ declare function assign<TEvent extends EventObject>(
   assignment: (ev: TEvent) => void
 ): ActionObject<TEvent>;

+type NoOp<T> = { [K in keyof T]: T[K] };
+
 declare function createMachine<
   TTypesMeta extends TypegenConstraint = TypegenDisabled
 >(
   config: {
     types?: TTypesMeta;
   },
-  action?: TTypesMeta extends TypegenEnabled
-    ? { action: ActionObject<{ type: 'WITH_TYPEGEN' }> }
-    : { action: ActionObject<{ type: 'WITHOUT_TYPEGEN' }> }
+  action?: NoOp<
+    TTypesMeta extends TypegenEnabled
+      ? { action: ActionObject<{ type: 'WITH_TYPEGEN' }> }
+      : { action: ActionObject<{ type: 'WITHOUT_TYPEGEN' }> }
+  >
 ): void;

 createMachine(

TS playground

Andarist commented 2 years ago

I've found yet another workaround for this particular issue (the slimmed-down repro) - it doesn't help me in my project though as it breaks some other stuff:

diff --git a/repro_case.ts b/repro_case.ts
index 0ee0107..1e4fd8b 100644
--- a/repro_case.ts
+++ b/repro_case.ts
@@ -9,11 +9,12 @@ type ActionFunction<TEvent extends { type: string }> = (event: TEvent) => void;

 // this is supposed to "route" the second argument based on the information provided within the first one
 declare function createMachine<
-  TTypesMeta extends TypegenEnabled | TypegenDisabled = TypegenDisabled
->(
-  config: {
+  TTypesMeta extends TypegenEnabled | TypegenDisabled = TypegenDisabled,
+  TConfig = {
     types?: TTypesMeta;
-  },
+  }
+>(
+  config: TConfig,
   implementations: TTypesMeta extends TypegenEnabled
     ? ActionFunction<{ type: "test" }>
     : ActionFunction<{ type: string }>

Basically what I would like is to make TS:

The problem is that I have some nested generic calls expressions in the second argument and if I apply this workaround then they infer based on both branches of this conditional type. I would like to completely prevent it - TS should only start looking into the second argument while having the rest already computed.

andrewbranch commented 2 years ago

assume that TTypesMeta is only inferrable from the first argument

I haven’t fully wrapped my head around this issue, but it sounds like you’re saying this would help? https://github.com/microsoft/TypeScript/issues/14829

Andarist commented 2 years ago

I think that this would have the potential to fix this if it would be baked into the language.

We are already using NoInfer trick at a couple of places as there is only one place in our complex structure that should act as a possible inference site but the same generic is used all over the place. So even manual annotation with :any at a random place deopts our desired inference.

I've tried to apply NoInfer trick here though (and even LowInfer) but none of that helped for this particular case. I think that the part of the problem is that we have additional generic call expressions nested in the second argument and TS starts inferring their type params before it settles on the value of this TTypesMeta. Or something like that πŸ˜…

I'm basically on the lookout here for a trick to make TS start processing the second argument only after it settles the first one but I've already thrown all the tricks I got at this and I can't make it work. I've briefly thought that forcing the resolution with infer would help me, like here, and it did... but it broke some other type-tests related to those nested call expressions. I think that I would have to make somehow TTypesMeta to be inferred to its default within the type params list or by forcing it somehow within the scope of the first argument - but I have no idea how to make it. This would hopefully, make the second argument to already see the resolved value and that potentially would just satisfy all of my requirements.