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

Type narrowing of generic types in control flow analysis #50652

Open
adrienharnay opened this issue Sep 6, 2022 · 10 comments
Open

Type narrowing of generic types in control flow analysis #50652

adrienharnay opened this issue Sep 6, 2022 · 10 comments
Labels
Needs Investigation This issue needs a team member to investigate its status.
Milestone

Comments

@adrienharnay
Copy link

Bug Report

Hello,

🔎 Search Terms

type narrowing, generic types, control flow analysis

🕗 Version & Regression Information

(see Playground) When trying to narrow generic types (unions), the control flow analysis is not aware of the narrowing. I also see that this PR: #43183 was supposed to address this.

Please keep and fill in the line that best applies:

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about type narrowing

⏯ Playground Link

Playground link with relevant code

💻 Code

const INITIAL_STATE = {
  1: 'test1',
  2: 'test2',
};
type State = typeof INITIAL_STATE;

const stateReducer = <Type extends keyof State>(
  state: State,
  action: { type: Type; value: string } | { type: 'clear'; value: number },
) => {
  if (action.type === 'clear') {
    // action.value is "string | number", but should be "number"
    return action.value;
  }

  return {
    ...state,
    [action.type]: action.value,
  };
};

stateReducer(INITIAL_STATE, { type: 'clear', value: 0 });

🙁 Actual behavior

action.value is "string | number"

🙂 Expected behavior

but should be "number"

Thanks in advance!

@whzx5byb
Copy link

whzx5byb commented Sep 6, 2022

{ type: Type; value: string } | { type: 'clear'; value: number } is not a discriminated union so it may not be narrowed as the way you expected. To make it work, you have to distribute over the generic type Type.

const stateReducer = <Type extends keyof State>(
  state: State,
-  action: { type: Type; value: string } | { type: 'clear'; value: number },
+  action: Type extends unknown ? { type: Type; value: string } | { type: 'clear'; value: number } : never,
) => {

@andrewbranch
Copy link
Member

It’s a fair question why type isn’t a discriminant property, though. It’s not that every constituent in the union needs a unit type, because this is discriminable:

-  action: { type: Type; value: string } | { type: 'clear'; value: number },
+  action: { type: keyof State; value: string } | { type: 'clear'; value: number },

The fact that Type is a type parameter, though constrained to a union of literal types, prevents us from recognizing the property as a discriminant. The conditional type distributes, yes, but it also kind of un-genericizes by filling in the constraint, which lets us recognize the property as a discriminant. I don’t immediately see a great reason why we couldn’t say that a type parameter whose constraint is a literal type is a valid discriminant... but these things have a way of having unexpected rippling effects.

@fatcerberus
Copy link

fatcerberus commented Sep 7, 2022

It’s not that every constituent in the union needs a unit type

It's funny you say this because this was also my understanding for a long time. It's only due to past comments by @RyanCavanaugh that I found out the rule is the discriminant can either be a unit type or a union of unit types (thus why keyof State works as a discriminant). So I certainly wouldn't blame anyone who didn't realize that.

It's interesting that that conditional type trick works. I would expect it to just defer since it's distributive over a type parameter.

@andrewbranch andrewbranch added the Needs Investigation This issue needs a team member to investigate its status. label Sep 9, 2022
@andrewbranch andrewbranch added this to the Backlog milestone Sep 9, 2022
@wbt
Copy link

wbt commented Nov 14, 2022

Here is another toy example demonstrating what I think is the same issue:

interface UpdateReport<B extends boolean = true> {
    newValue: string;
    oldValue: B extends true ? string : undefined;
}
export const processReport = function<B extends boolean>(
    report: UpdateReport<B>,
    hasOldValue: readonly [B],
) {
    if(hasOldValue[0]) {
        //Error TS(2345): Argument of type 'UpdateReport<B>'
        //is not assignable to parameter of type 'UpdateReport<true>'.
        //Type 'B' is not assignable to type 'true'.
        //Type 'boolean' is not assignable to type 'true'.
        //However, within this conditional check / type guard,
        //TS should be able to figure out that B is 'true'.
        let oldValue = getOldValueFromReport(report); //...
    }
}
const getOldValueFromReport = function(
    report: UpdateReport<true>
) {
    return report.oldValue;
};

Search terms: Type guard narrowing assignable to parameter of type 2345

@thetutlage
Copy link

Another one when using unions with generics.

type BcryptConfig = {
  rounds: number
  type: 'bcrypt'
}

type ArgonConfig = {
  variant: number
  type: 'argon'
}

function defineConfig<T extends { [key: string]: ArgonConfig | BcryptConfig }>(config: T): T {
  return config
}

defineConfig({
  passwords: {
    type: 'argon',
    variant: 1,
    rounds: 1,
  }
})

I expect the rounds: 1 parameter to be disallowed, since the property does not exists on the ArgonConfig.

@rayzr522
Copy link

rayzr522 commented Feb 13, 2023

any update on this? I find myself FREQUENTLY running into this issue, especially when dealing with things like events where there's multiple different types with different payloads. here's an example:

(ts playground)

type EventType = 'NEW_MESSAGE' | 'NEW_USER'

type AppEvent =
  | { type: 'NEW_MESSAGE'; message: string }
  | { type: 'NEW_USER'; user: string }

function fetchEvents<Type extends EventType>(
  eventType: Type
): Extract<AppEvent, { type: Type }> {
  if (eventType === 'NEW_MESSAGE') {
    // this errors, because typescript can't figure out that Type must be 'NEW_MESSAGE'
    return {
      type: 'NEW_MESSAGE',
      message: 'hello, world',
    }
  } else if (eventType === 'NEW_USER') {
    // this errors, because typescript can't figure out that Type must be 'NEW_USER'
    return {
      type: 'NEW_USER',
      user: 'rayzr',
    }
  } else {
    throw new Error(`Unknown event type: ${eventType}`)
  }
}

I do this kinda thing all the time when making abstractions to handle multiple types of input/output, and it's a pain to have to typecast the return types

@jcalz
Copy link
Contributor

jcalz commented Aug 4, 2023

This looks like the same thing except the generic is constrained to something other than a union... but it should still probably be discriminable:

type Foo<T extends {}> =
    { a: undefined, b: string } | { a: T, b: number }

function foo<T extends {}>(f: Foo<T>) {
    if (f.a == null) {
        f.b // should be string, is actually string | number
    }
}

Playground link

@reiss-d
Copy link

reiss-d commented Sep 8, 2023

I think this is a slightly different example of the control flow not narrowing the generic, only it's not a property access:

declare function takesString(value: string): void

const data = { a: 'foo', b: 1 }
type Data = typeof data

function getValue_generic<K extends keyof Data>(key: K) {
   if (key === 'a') {
      key // K extends "a" | "b"
      const value = data[key] // { a: string; b: number }[K]
      takesString(value) // Argument of type 'string | number' is not assignable to parameter of type 'string'.
   }
}

function getValue_non_generic(key: keyof Data) {
   if (key === 'a') {
      key // "a"
      const value = data[key] // string
      takesString(value) // ok
   }
}

Playground

@Lordfirespeed
Copy link

And another toy example (I believe):

const foo = {
  bar: ['hello', (value: 'hello' | 'bye') => {}],
  baz: [3, (value: 3 | 5) => {}],
} as const;

function doWithFoo<T extends keyof typeof foo>(key: T) {
  if (key === 'bar') {
    const [value, setValue] = foo[key];
    setValue('bye'); // error here
  }
}

https://stackblitz.com/edit/typescript-repro-eqknuz?file=src%2Fmain.ts

@gima
Copy link

gima commented Dec 1, 2024

"Type narrowing from control flow analysis doesn't backpropagate to another, type-dependent, conditional parameter's type".

The demonstration code below is identical* for both TypeScript and Flowtype.

  1. 🔗 TypeScript Playground link
    Result: TypeScript's type checker doesn't manage to narrow the type.

  2. 🔗 Flowtype Playground link
    Result: Flowtype's type checker manages to narrow the type correctly, as should be the case.
    *) (the code is otherwise identical, except for changing "String" to "string" and "Number" to "number").

The code in question:

type OneOfTwo = String | Number;
type ConditionalTybe<T, TRUETYBE, FALSETYBE> = T extends String ? TRUETYBE : FALSETYBE;

function funktion<T extends OneOfTwo> (arg1: T, arg2: ConditionalTybe<T, true, false> ) {
    if (typeof arg1 == "string") {
        if (arg2 === true) {}
        if (arg2 === false) {} // why is there no error here?
    }
}

funktion('str', true);
funktion('str', false); // correctly shows an error here.
funktion(1, true); // correctly shows an error here.
funktion(1, false);

If needed, I can provide a real and useful, and not terribly complex code example of this, where a correctly narrowed type would avoid a superfluous null check (to narrow the possibility of a null value out of a variable).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests