Type guards don’t always narrow generic types

[ArkaneMoose]

Bug Report

🔎 Search Terms

generics, narrowing, type guards

🕗 Version & Regression Information

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

⏯ Playground Link

This is nearly exactly how I discovered this bug.

💻 Code

/** Maps event names to an optional array of event listeners. */
type EventListeners<EventMap> = {
  [EventType in keyof EventMap]?: Array<(event: EventMap[EventType]) => void>;
};

/**
 * Given an EventListeners mapping, dispatches the given payload to event
 * listeners of the given type.
 */
function emit<EventType extends keyof EventMap, EventMap>(
  listenersByType: EventListeners<EventMap>,
  type: EventType,
  payload: EventMap[EventType],
) {
  const listeners = listenersByType[type];
  if (listeners !== undefined) {
    // This works...
    listeners.forEach((listener) => listener(payload));

    // But this doesn't?!
    for (const listener of listeners) {
      listener(payload);
    }
  }
}

🙁 Actual behavior

There’s an error on the word listeners on the line:

for (const listener of listeners) {

The error differs between target versions.

Targeting ES2015 or higher:

Type 'EventListeners<EventMap>[EventType]' must have a '[Symbol.iterator]()' method that returns an iterator.

Targeting ES5 or lower:

Type 'EventListeners<EventMap>[EventType]' is not an array type or a string type.

In context, the generic type EventListeners<EventMap>[EventType] is equivalent to Array<(event: EventMap[EventType]) => void> | undefined.

Since undefined is not an array type, TypeScript is right to complain… except that I already checked that listeners !== undefined.

🙂 Expected behavior

TypeScript should narrow EventListeners<EventMap>[EventType] to Array<(event: EventMap[EventType]) => void>, as it does when I call listeners.forEach(…).

Other findings

After some investigation, I’ve discovered the following.

Type narrowing with intersection types

With a type guard with return type obj is GuardedType, type narrowing for generics works correctly because TypeScript narrows the generic type T to an intersection type T & GuardedType.

For instance:

function isArray(obj: any): obj is unknown[] { … }
// …
if (isArray(listeners)) {
  // Now, listeners has type EventListeners<EventMap>[EventType] & unknown[].
  for (const listener of listeners) {
    listener(payload);
  }
}

However, if I use a type guard with return type obj is undefined and negate the result, then I see the behavior described in the issue.

function isUndefined(obj: any): obj is undefined { … }
// …
if (!isUndefined(listeners)) {
  // Still broken.
  for (const listener of listeners) {
    listener(payload);
  }
}

So, I’m guessing this means that this kind of type narrowing only works for “positive” type guards, i.e. those where the narrowed type can be an intersection type.

Type narrowing by constraint position

But then, why does listeners.forEach(…) work in both cases? Digging into the TypeScript source code and using a debugger, I found that the type of listeners differs between the listeners.forEach(…) call and the for (const listener of listeners) loop according to the following function:

// Lines 24166–24178 of src/compiler/checker.ts at 8e1bf08
function getNarrowableTypeForReference(type: Type, reference: Node, checkMode?: CheckMode) {
    // When the type of a reference is or contains an instantiable type with a union type constraint, and
    // when the reference is in a constraint position (where it is known we'll obtain the apparent type) or
    // has a contextual type containing no top-level instantiables (meaning constraints will determine
    // assignability), we substitute constraints for all instantiables in the type of the reference to give
    // control flow analysis an opportunity to narrow it further. For example, for a reference of a type
    // parameter type 'T extends string | undefined' with a contextual type 'string', we substitute
    // 'string | undefined' to give control flow analysis the opportunity to narrow to type 'string'.
    const substituteConstraints = !(checkMode && checkMode & CheckMode.Inferential) &&
        someType(type, isGenericTypeWithUnionConstraint) &&
        (isConstraintPosition(reference) || hasNonBindingPatternContextualTypeWithNoGenericTypes(reference));
    return substituteConstraints ? mapType(type, t => t.flags & TypeFlags.Instantiable ? getBaseConstraintOrType(t) : t) : type;
}

Because isConstraintPosition(…) is defined as:

// Lines 24138–24147 of src/compiler/checker.ts at 8e1bf08
function isConstraintPosition(node: Node) {
    const parent = node.parent;
    // In an element access obj[x], we consider obj to be in a constraint position only when x is not
    // of a generic type. This is because when both obj and x are of generic types T and K, we want
    // the resulting type to be T[K].
    return parent.kind === SyntaxKind.PropertyAccessExpression ||
        parent.kind === SyntaxKind.CallExpression && (parent as CallExpression).expression === node ||
        parent.kind === SyntaxKind.ElementAccessExpression && (parent as ElementAccessExpression).expression === node &&
            !isGenericIndexType(getTypeOfExpression((parent as ElementAccessExpression).argumentExpression));
}

Therefore:

• When checking listeners in listeners.forEach(…):
  • parent.kind === SyntaxKind.PropertyAccessExpression
  • So isConstraintPosition(…) === true
  • So substituteConstraints === true
  • So getNarrowableTypeForReference(…) returns the type ((event: EventMap[EventType]) => void)[] | undefined
  • Which gets narrowed to ((event: EventMap[EventType]) => void)[] according to control flow
• When checking listeners in for (const listener of listeners):
  • parent.kind === SyntaxKind.ForOfStatement
  • So isConstraintPosition(…) === false
  • So substituteConstraints === false
  • So getNarrowableTypeForReference(…) returns the type EventListeners<EventMap>[EventType]
  • Which cannot be narrowed any further

Straightforward solution

I believe the straightforward solution is to add a check for parent.kind === SyntaxKind.ForOfStatement in the definition of isConstraintPosition(node: Node) so that it would be defined as follows:

function isConstraintPosition(node: Node) {
    const parent = node.parent;
    // In an element access obj[x], we consider obj to be in a constraint position only when x is not
    // of a generic type. This is because when both obj and x are of generic types T and K, we want
    // the resulting type to be T[K].
    return parent.kind === SyntaxKind.PropertyAccessExpression ||
        parent.kind === SyntaxKind.CallExpression && (parent as CallExpression).expression === node ||
        parent.kind === SyntaxKind.ElementAccessExpression && (parent as ElementAccessExpression).expression === node &&
            !isGenericIndexType(getTypeOfExpression((parent as ElementAccessExpression).argumentExpression)) ||
        parent.kind === SyntaxKind.ForOfStatement;
}

However, this isConstraintPosition(…) check feels like a special case for just a few specific contexts. I can think of several examples that would cause such a check to fail:

function f1<T extends string | undefined>(x: T) {
  if (x) {
    // This works because x is considered to be in a constraint position...
    x.length;

    // But adding parentheses breaks it.
    (x).length;

    // Assigning it to a new variable breaks it.
    const y = x;
    y.length;

    // Returning it from a function breaks it.
    const getX = () => x;
    getX().length;
  }
}

But patching it to include a check for SyntaxKind.ForOfStatement would be a quick and easy fix for my use case.

More general solution

I found that if I manually cast listeners to type Exclude<typeof listeners, undefined>, then this fixes my example:

if (listeners !== undefined) {
  const listeners_ = listeners as Exclude<typeof listeners, undefined>;

  // This works...
  listeners_.forEach((listener) => listener(payload));

  // And so does this.
  for (const listener of listeners_) {
    listener(payload);
  }
}

A similar kind of cast fixes the other example I presented showing failures of isConstraintPosition(…):

type Falsy = false | '' | 0 | 0n | null | undefined;

function f1<T extends string | undefined>(x: T) {
  if (x) {
    const x_ = x as Exclude<typeof x, Falsy>;

    // This worked without the cast...
    x_.length;

    // But these didn't, and now do.
    (x_).length;

    const y = x_;
    y.length;

    const getX = () => x_;
    getX().length;
  }
}

So, I feel like the general solution would be: when a type guard cannot narrow a generic type T to an intersection type, it should instead narrow the type T to Exclude<T, …>. (Is there a situation where this wouldn’t work?)

Let me know what you all think, and thanks for creating my favorite programming language!

[fatcerberus]

As you saw, type-guarding generics is done via intersections. Negating the result of an undefined guard would therefore require #4196.

[ArkaneMoose]

My proposal in the “More general solution” is to use Exclude<T, undefined> for such a scenario instead of an intersection. Would this work?

[G-Rath]

I'm thinking this is the same issue - if it's not, will create a new issue:

function isDefined(value: unknown): asserts value {
  throw new Error();
}

interface Tx {
  v1: string[];
}

interface Mx<TOptions extends readonly unknown[]> {
  settings: Tx;
  options: TOptions;
}

const getOptionValue = <TOption extends keyof Tx>(
  context: Mx<readonly [Partial<Pick<Tx, TOption>>]>,
  key: TOption,
): Tx[TOption] => {
  const optionValue = context.options[0][key];

  if (optionValue) {
    isDefined(optionValue);

    // TS2322: Type 'Tx[TOption] | undefined' is not assignable to type 'Tx[TOption]'.
    //  Type 'undefined' is not assignable to type 'Tx[TOption]'.
    //    Type 'undefined' is not assignable to type 'string[]'.
    return optionValue;
  }

  return context.settings[key];
};

I can make TS happy in a few ways:

• use !: return optionValue!
• add ?? undefined to the assignment: context.options[0][key] ?? undefined
• add ?? context.settings[key] to the assignment.

[ArkaneMoose]

Ah, it seems like this issue might be a duplicate of #22137, which will hopefully be fixed by #22348.

[cpiber]

Not sure if this is the same, but there is something similar:

class Gen<T extends string | undefined = string | undefined> {
    isUndefined(): this is Gen<undefined> { ... }
}
const g = new Gen();
if (g.isUndefined())
    throw new Error(); // correctly narrowed to Gen<undefined>
// but undefined isn't excluded here