Array.prototype.filter doesn't require callback to return

[fourlastor]

I dig a bit and found out in #5850 and subsequently in #7779 the signature of filter has been changed not to return a boolean, so that people could return a truthy value in the implementation of the filter.

While I undertand the convenience in terms of ease of use, this as far as I observed puts developers at risk of simple bugs like the following:

const even = [1,2,3].filter(x => {
    x % 2 == 0
})

Notice the missing return statement means the function returns undefined, which translates to false, which filters all the items out. This applies also in the case of noImplicitReturns set to true, since the function return type is any, thus it could be a void function for what the compiler knows.

I believe TypeScript's job should be to put this kind of cases in check actually, so that things like truthy values (which sometimes aren't as obvious as we might think and introduce bugs we don't really understand) can be better taken care of.

One solution to make both parties happy would be to optionally support truthy values (maybe an extra compiler flag? Another truthy type to allow for more relaxed checking?) and reintroduce the original signature of filter.

[RyanCavanaugh]

#5850 (comment)

[gcnew]

Although I'm a fan of strictness the loose signature is justified in this case, IMHO.

PS: I feel a little uneasy adding 👎 on the proposals as it's ambiguous whether it's a vote cast on AMF or not.

[aj-r]

As I proposed in #20033:

Maybe a new type (BooleanLike? TruthyFalsy?) would do the trick? It would indicate that the value is meant to be a boolean, but would allow other truthy/falsy values based on the rules that @andy-ms described:

• It would behave like any except it would force people to return a value (so really it behaves more like {} | null | undefined.
• In --strictNullChecks we could also check that the value is actually potentially truthy/falsy, so string and Node | undefined would work but not just Node.
  • I'm not sure about this one though. Maybe we should still allow explicit boolean values, e.g. return true or return false.

A new type like TruthyFalsy would also help on the other side of things. For example, if I'm writing a function that accepts a callback:

function doStuff(callback: (arg: string) => TruthyFalsy): string[] {
    let arr: string[] = ["item 1", "item 2", "item 3"];
    let result: string[] = [];
    for (let item of arr) {
        // With TruthyFalsy, typescript can catch the bug on the next line:
        if (callback(item) === true) { // Error: cannot compare type TruthyFalsy to type boolean
            result.push(item);
        }
    }
    return result;
}

Basically, we don't allow people to compare TruthyFalsy types to any other type. We would only allow them to use TruthyFalsy values in a couple of ways

• if (truthyFalyValue)
• !truthyFalsyValue

[aj-r]

At a minimum, we should use {} | null | undefined instead of any - this forces callbacks to return a value. But if we do that, we should modify similar callbacks (in some, every, filter, for example) to be the same - right now all of these callbacks have a boolean return type. We should be consistent.

[aj-r]

A TruthyFalsy type would also allow us to catch bugs in if statements - we could add a compiler option for strictIfConditions or something, which requires the condition of an if statement to be a boolean or TruthyFalsy type. For example:

function isReady(): boolean {
    return false;
}

if (isReady) { // compile error: type '() => boolean' is not allowed in an 'if' condition. Only types 'boolean' or 'TruthyFalsy' are allowed.
}

The bug here, of course, is that we should have done if (isReady()) instead of if (isReady). But currently typescript will not catch that bug, and the code will execute as if isReady is always true.

Same for && and || operators - we could require both operands to be TruthyFalsy.

However, this could end up being overly restrictive. I often like to do if (x) to check if x is not undefined, which would be prohibited. I also like to do const y = x || "default"; (using || as sort of a null-coalescing operator), which would also be prohibited.

As a workaround, we could allow null or undefined types in addition to TruthyFalsy and boolean types. But this seems like a strange/unexpected exception to the rule. Alternatively, we could allow any type except function types, just to prevent this specific bug - then this becomes independent of the TruthyFalsy type.

[denis-sokolov]

I understood the TruthyFalsy type to be any type that has at least one truthy and one falsy value. A { foo: number } | null obviously matches. A boolean, string or number also do because they inherently have falsy values. However, neither { foo: 3 }, nor () => {}, nor "string-literal" would match because they are always truthy. Neither undefined, nor null, because they are always falsy. We then need an exception to make literal true or false match TruthyFalsy. This will ensure that we can have “always true” or “always false” filters as well as use constant values in conditions: const DEBUG = false; if (DEBUG) { ... }.

@aj-r, you say it could be overly restrictive. I don’t understand. What am I missing in your examples?

if (x) to check if x is not undefined, which would be prohibited.

If x is { foo: number } | undefined, then it would be allowed, that matches TruthyFalsy, it would not be prohibited. If x is always truthy or always undefined, then the condition is meaningless, and it’s great that it’s prohibited, no?

I also like to do const y = x || "default"; (using || as sort of a null-coalescing operator), which would also be prohibited.

Again, if x can be truthy and falsy, this would not be prohibited. If it’s always truthy or always falsy, the code above is a programmer error, so it’s good it’s prohibited.

[aj-r]

@denis-sokolov That's an interesting idea that I hadn't considered - allow types to be implicitly cast to TruthyFalsy, as long as they could be either truthy or falsy. I like it. When I said that strictIfConditions would be overly restrictive, I was thinking that one would need to explicitly cast a value as TruthyFalsy in order to use it with if/||/&&. But your idea solves this problem quite nicely.

I still think we should still allow people to explicitly cast any value as TruthyFalsy using a type assertion (e.g. true as TruthyFalsy). Furthermore, a function that returns TruthyFalsy should accept any value as the return type, without requiring a type assertion. For example:

function callback(value: any, index: number): TruthyFalsy {
    if (typeof value === "string")
        return true; // Should be allowed
    if (typeof value === "object")
        return false; // Should be allowed
    return typeof value === "number" && value > 0;
}

It would be really cool to see this in a future version of TypeScript. My only concern is that this type has some special cases that other types don't have, so it might be a lot of work to implement.

[sgronblo]

I think it's really unfortunate that these library functions are defined by default to be sloppy instead of strict.

In one of the really old suggestions for making the change from strict to sloppy: #5850 there was a suggestion to add your own override to make the function more sloppy by allowing any.

However it seems using the same trick to add a new more strict override doesn't really work because the sloppy override still exists in lib.d.ts.

The only solution I could come up with quickly was to add my own filter wrapper that basically just calls Array.filter under the hood. Is there any better solution to this problem?

[mikedidomizio]

Our team ran into this today with Array.find and I don't quite understand the reasoning to allow it to be loose. There's probably a lot of incorrect code out there.

Some Array methods allow for using generics like Array.map, but others do not. There's inconsistency.