Predicate value is object doesn't work in Array.prototype.filter()

[Peeja]

Bug Report

🔎 Search Terms

type narrowing, type predicate, value is object, filter(), lodash.isObject

🕗 Version & Regression Information

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

⏯ Playground Link

Playground link with relevant code

💻 Code

// isNumber and isObject are modeled after the functions by the same names in lodash.

function isNumber(value?: any): value is number {
  return typeof value === "number";
}

function isObject(value?: any): value is object {
  return typeof value === "object";
}

// Each element could be an object or a number, so we can't treat it as either.
const variousThings = [{ a: 1 }, 1, "abc"];
variousThings[0] + 1;
variousThings[0].a;

// Filtering for only numbers yields values we can add, but not get properties of.
const allNumbers = variousThings.filter(isNumber);
allNumbers[0] + 1;
allNumbers[0].a;

// But filtering for only object yields fails to remove number from the union type.
const allObjects = variousThings.filter(isObject);
allObjects[0] + 1;
allObjects[0].a;

// Yet isObject is able to narrow a single element in a simple conditional.
const value = variousThings[0];
if (isObject(value)) {
    value.a
}

🙁 Actual behavior

allObjects was typed as (string | number | { a: number })[], so both lines below const allObjects fail.

🙂 Expected behavior

allObjects should be typed as { a: number }[], so allObjects[0].a should pass the type checker. Notably, as seen above, isObject does manage to narrow the type of a single value with the union type to the only member of the union that's an object, it just can't narrow all of the elements using .filter() the way isNumber is able to.

I can't conceive of why they behave differently, but I also don't know the internals well. I think this difference is undesired, but I might be missing something deeper.

[Peeja]

Ah! I understand now. The difference is that number is one of the types in the union and also the type that isNumber checks, but the object type in the union is { a: number }, while isObject checks for object in general. If you change isObject to return value is { a: number }, it works. (Though obviously that's not a reasonable definition of isObject.)

I think I have a solution, though: this alternate definition of filter (written here as filter2) should work with a predicate that checks a broader type than any elements in the union, and returns the intersection of the two broad types (the union and the type checked by the predicate).

interface Array<T> {
  filter2<S>(
    callbackfn: (value: T | S, index: number, array: (T | S)[]) => value is S,
    thisArg?: any
  ): (T & S)[];
}

(Playground)

The biggest downside I see at the moment is that the elements come out typed as { a: number; } & object. Maybe worse, the elements of allNumbers are typed as number | ({ a: number; } & number), which is rather absurd. I'm not sure why that doesn't reduce to number automatically. It's perfectly functional, though; you can still treat it as a number.

Would a PR with this as the new definition for .filter(), or something similar, be welcomed?

[andrewbranch]

Your analysis is correct, and that definition for filter looks better in many ways. However, I’d be concerned that with the intersection, we’d be opening the doors for really explosive types (if your array element type is a union of 100 things, and your type predicate tells you that a type is a union of 100 other things, now you have a union of 100,000 things). I’m unsure whether there’s a good way forward with changing the lib typings. Semi-related is #36554, though that one actually doesn’t have any filter examples at the moment.

[johan]

I've had the same problem with when filtering unknown[] with type guards, which current ts just ignores. Building on Peeja's solution, I arrived at a solution that also addresses an edge case that broke code reliant on inferred context, changing the src/lib/es5.d.ts override definition:

filter<S extends T>(predicate: (value: T, index: number, array: T[]) => value is S, thisArg?: any): T[]

…into two overrides that both produce (T & S)[] instead of T[]:

// make array.filter(isFoo).filter(isBar) produce Array<Foo & Bar>:

interface ReadonlyArray<T> {
    filter<S extends T>(predicate: (value: T, index: number, array: readonly T[]) => value is S, thisArg?: any): (T & S)[];
    filter<S>(predicate: (value: T | S, index: number, array: readonly (T | S)[]) => value is S, thisArg?: any): (T & S)[];
}
interface Array<T> {
    filter<S extends T>(predicate: (value: T, index: number, array: T[]) => value is S, thisArg?: any): (T & S)[];
    filter<S>(predicate: (value: T | S, index: number, array: (T | S)[]) => value is S, thisArg?: any): (T & S)[];
}

Updated version of the PR's example, re-calibrated for current typescript 5.2.2, and with another test case that broke with Peeja's first pitch: playgrounds

In other words: this problem can be addressed by this change, plus whatever test suite updates are needed.