microsoft / TypeScript

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

TypeScript fails to narrow out `undefined` via `typeof` check on generic indexed access #50548

Open wbt opened 2 years ago

wbt commented 2 years ago

Bug Report

πŸ”Ž Search Terms

undefined narrowed 2322

πŸ•— Version & Regression Information

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

type StarStats = {
    mass: number;
    surfaceTemperature: number;
    planets: number;
}
type PlanetStats = {
    mass: number;
    moons: number;
    orbitalPeriod: number;
    orbitalInclination: number;
}
interface PlanetaryBodiesMap {
    'Planet' : PlanetStats;
    'Star' : StarStats;
}
const getStatFromSet = function<
    TN extends keyof PlanetaryBodiesMap,
>(
    statSet : Partial<PlanetaryBodiesMap[TN]>,
    statName : keyof typeof statSet,
) : number {
    const potentialResult = statSet[statName];
    //Adding redundant `&& potentialResult !== undefined` to the conditional,
    //the error message changes between v4.7.4 and v4.8.0-beta
    if(typeof potentialResult !== 'undefined') {
        potentialResult;
        //Hover above to see: Partial<PlanetaryBodiesMap[TN]>[keyof PlanetaryBodiesMap[TN]]
        //Error ts(2322) below:  Type 'PlanetaryBodiesMap[TN][keyof PlanetaryBodiesMap[TN]] | undefined' is not assignable to type 'number'.
        //Where does that extra `| undefined` come from after having narrowed the type of the `const`?
        return potentialResult;
    } else {
        return 0;
    }
}

πŸ™ Actual behavior

TypeScript complains that a constant value can be undefined within a conditional block where that is already checked for and excluded.

πŸ™‚ Expected behavior

  1. Inside a conditional which can only be entered if a constant type is not undefined, TypeScript excludes | undefined from the possible types of the constant.
  2. When showing the type of a constant as X in an error message about how that can't be assigned to Y, the type X should be the same as the type shown when hovering over the type. In this example, they differ by | undefined, which matters. In the unsimplified version where the type is more complex, the type of the constant shown on hover is very linguistically different from the type shown in the error, making this issue harder to debug.
jcalz commented 2 years ago

This doesn't seem to have much to do with undefined per se; the same underlying problem happens with

const getStatFromSet = function <K extends keyof PlanetaryBodiesMap>(
    statSet: PlanetaryBodiesMap[K],
    statName: keyof typeof statSet,
): number {
    return statSet[statName]; // error
}

This feels like #32365 to me.

wbt commented 2 years ago

>3-year-old #32365 looks related and may have something to do with an underlying reason; it's possible that fixing that issue could also fix this one. However, I think it's quite possible this issue could also be fixed without fixing that one by focusing on the elimination of 'undefined' from the type of any constant that is inside a conditional eliminating undefined from its type.

jcalz commented 2 years ago

Not seeing how, given that the following still fails:

const getStatFromSet = function <K extends keyof PlanetaryBodiesMap>(
    statSet: Partial<PlanetaryBodiesMap[K]>,
    statName: keyof typeof statSet,
): number | undefined {
    return statSet[statName]; // error!
}

If statSet[statName] cannot be seen as assignable to number | undefined, then eliminating undefined wouldn't help it being seen as assignable to number. I really am not processing how undefined is the issue here. Can you articulate how that would work? Or maybe you have some more motivating code example handy?

wbt commented 2 years ago

The core expected behavior I'm trying to call attention to in this bug report is:

  const x = //any process at all by which x gets a value and type, even if that's a buggy process
  if(typeof x !== 'undefined') {
        //**Regardless of how x was defined,** 
        //whether using the assignment example from the OP here or any other,
        //any attempt to use it here should NOT result in an error that "x could be `undefined`". 
  } 

If fixing #32365 would allow statSet[statName] to be assignable to type number | undefined in the example context given, that seems like it would be a step forward, but it's not clear to me that this would necessarily eliminate the core unexpected behavior (which could potentially also stem from other causes) and it's not clear that ensuring the core expected behavior would necessarily require fixing #32365. (It might be that fixing my problem requires fixing both, but that doesn't mean this one should be closed as a duplicate of #32365.)

andrewbranch commented 2 years ago

Yeah, I think there is something weird with typeof narrowing here. Slightly simplified to extract from #32365:

Playground

interface Foo {
    a: { b: string }
}

function f<K extends keyof Foo>(f: Partial<Foo[K]>, k: keyof Foo[K]) {
    const x = f[k];
    if (typeof x !== "undefined") {
        let _: {} = x; // Should error because of possible `null`, but errors because of `undefined`
    }

    const y = f[k];
    if (y != undefined) {
        let _: {} = y;
    }

    x; // This is weird too
    y;
}
wbt commented 1 year ago

Here is what I suspect is another example of the same issue, though it can be broken out to a separate issue if others think it's not a duplicate:

//In the motivating example, these types are way more complex than simple constant strings.
interface AnimalSounds { Cat: 'meow'; Dog: 'woof'; Duck: 'quack';}
type AnimalSound = AnimalSounds[keyof AnimalSounds]; //"meow" | "woof" | "quack"
export const callerFn = function<A extends keyof AnimalSounds> (
    animalTypeName: A,
    sonicEnvironment: Partial<AnimalSounds> //cannot be restricted to require A here
) {
    const sound = sonicEnvironment[animalTypeName];
    if (typeof sound === 'undefined') {
        throw new Error('Could not find sound in environment.');
    }
    //At/after this line, 'sound' should be narrowed to EXCLUDE the 'undefined' type possibility.
    //i.e. sound should be Exclude<Partial<AnimalSounds>[A], undefined>, but the exclusion isn't working.
    //You can move the error and DRY up casting by using a narrowing const, but it's still an error:
    const soundNarrowed: Exclude<Partial<AnimalSounds>[A], undefined> = sound; //Type 'undefined' not assignable
    //Error in first parameter of next line:
    //Argument of type 'Partial<AnimalSounds>[A]' is not assignable to parameter of type 'AnimalSounds[A]'.
    //Type '"meow" | undefined' is not assignable to type '"meow"'.
    //Type 'undefined' is not assignable to type '"meow"'.ts(2345)
    calledFn<AnimalSounds[A]>(sound, toUpperCaseTyped(sound));
    //In the line below but NOT above, the 'undefined' possibility is correctly narrowed out:
    calledFnNoGenericOneParam(sound);
}
const calledFn = function<S extends AnimalSound>(sound: S, loudSound: Uppercase<S>) {/*...*/};
//Dropping the generic doesn't work in context,
//because multiple types in the function are derived from the generic type parameter.
const calledFnNoGenericOneParam = function(sound: AnimalSound) {/*...*/};
//Bonus issue(#44268): the cast in the return statement of the next line should be automatic & unnecessary:
const toUpperCaseTyped = function<S extends string>(strIn: S) {return strIn.toUpperCase() as Uppercase<S>;};

With this example, the behaviour changed between 4.2.3 and 4.3.5, so that the parameter to calledFnNoGenericOneParam is correctly constrained to exclude undefined; this fix unfortunately did not address the narrowing for the call to calledFn but the PR for it might be informative.