microsoft / TypeScript

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

Boolean() cannot be used to perform a null check #16655

Open kujon opened 7 years ago

kujon commented 7 years ago

TypeScript Version: 2.4.0

Apologies for today's issue raising binge.

Code

// Compiles
const nullCheckOne = (value?: number) => {
    if (!!value) {
        return value.toFixed(0);
    }

    return '';
}

const nullCheckTwo = (value?: number) => {
    if (Boolean(value)) {
        // Object is possibly 'undefined'
        return value.toFixed(0);
    }

    return '';
}

Expected behavior: Both examples compile.

Actual behavior: The latter example fails w/ Object is possibly 'undefined'.

Explanation To my knowledge !!value and Boolean(value) are equivalent. I'm wondering what is the reason behind not supporting the second case. One reason I can think of would be an imported, non-typescript module, globally overriding it to something like: Boolean = (value) => !value.

RyanCavanaugh commented 7 years ago

I'm wondering what is the reason behind not supporting the second case

It's not supported because no one has done the work to support it yet.

One reason I can think of would be an imported, non-typescript module, globally overriding it to something

In general we don't account for this. Literally anything in the global scope could be mutated to be something different and it'd be insane to say Well you can't use substr because maybe someone messed with it.

I think we could replace the signature with

declare function Boolean<T>(x: T | false | 0 | "" | null | undefined): x is T;

which is almost correct (it fails to do the right then in the else case when x is 0 coming from a non-literal numeric type).

I've literally never seen someone use Boolean like this, though. Is there any difference between if (value) and if (Boolean(value)) ?

kujon commented 7 years ago

I don't think there's any, apart from implicit vs. explicit type cast. I personally use it, because it makes type conversion to primitives consistent (Boolean(1), String(1), Number('1'), etc.)

Jessidhia commented 7 years ago

One common pattern using the boolean constructor is .filter(Boolean) to remove falsy values from an array (including null and undefined).

kujon commented 7 years ago

@Kovensky - that is a good point. filter as it currently stands however, can't be used to implement the type guards either. Both of the examples below infer to (number | undefined)[]. Would also be really cool to address it.

const nonFalsy1 = [1, 2, undefined].filter(v => typeof v !== 'undefined');
const nonFalsy2 = [1, 2, undefined].filter(Boolean);
ForbesLindesay commented 5 years ago

@kujon You can make .filter work with the custom function by adding explicit types:

const nonFalsy1: number[] = [1, 2, undefined].filter(
  <T>(v: T): v is Exclude<T, undefined> => typeof v !== 'undefined',
);

It's a little bit verbose, but very flexible.

RyanCavanaugh commented 5 years ago

This was reverted due to it causing a breaking change in 3.5. The simple repro looks like this:


declare const Bullean: {
    new(value?: any): Boolean;
    <T>(value?: T): value is Exclude<T, false | null | undefined | '' | 0>;
};

declare const arr: any[];
// Was any[], now (3.5) unknown[]
const arr2 = arr.filter(Bullean);

There might be something wrong in generic inference, but it's not yet clear. We'll need this scenario to work before adding it back in.

ForbesLindesay commented 5 years ago

I'm not sure what more I can do here?

danielrentz commented 5 years ago

This was reverted due to it causing a breaking change in 3.5. The simple repro looks like this: There might be something wrong in generic inference, but it's not yet clear. We'll need this scenario to work before adding it back in.

Just for interest, what is the reason that Exclude narrows any to unknown?

lgraziani2712 commented 4 years ago

Here is a playground repro BTW https://www.typescriptlang.org/play/?ssl=1&ssc=1&pln=7&pc=2#code/GYVwdgxgLglg9mABAJwKZRMsBRAHgQ2gAoBKALkQG9EYATCgZymRjAHNEBfAbQF0qAUImGIICJonzJkiALyJu1OhQDkARhVcANFRr1EKgEybOOpfvUneAbgFCRaDFknT7IkQDoAtvgAORVAAbVC85AD5EIJCPOjlZeSNNAH5EMBBAwMQKKK8SN3dED2AYQKhUZADg0NkInMQAQnk0jJJbTiA

AllainPL commented 4 years ago
const arr = [1, 2, undefined];
const filtered: Array<number> = arr.filter(elem => elem != undefined);
const filtered2: Array<number> = arr.filter(Boolean);

// Type '(number | undefined)[]' is not assignable to type 'number[]'.
//   Type 'number | undefined' is not assignable to type 'number'.
//     Type 'undefined' is not assignable to type 'number'.

// Type '(number | undefined)[]' is not assignable to type 'number[]'.

Both these scenarios right now still break. I found a temporary workaround with

const workaround = arr.filter((elem): elem is number => elem !== undefined);

However it would be nice not to need the type guard. Is there any hope for that :)?

LorenzHenk commented 4 years ago
const workaround = arr.filter((elem): elem is number => elem !== undefined);

Another workaround (playground link):

const values: Array<number | undefined> = [1, 2, undefined, 4, undefined, 6]

function nonNullish<V>(v:V): v is NonNullable<V> {
    return v !== undefined && v !== null
}

values.filter(nonNullish).map(v => v.toFixed(5))
Etheryte commented 3 years ago

For anyone else running into the specific case of Array.filter(Boolean), adding an overload for Array.prototype.filter() works great. I've tested this on 4.1.2 and https://github.com/microsoft/TypeScript/issues/31551#issuecomment-495201128 doesn't seem to be present anymore. Patching Boolean itself should work too, but I haven't tried that.

type Falsy = false | 0 | "" | null | undefined;

interface Array<T> {
  filter<S extends T>(predicate: BooleanConstructor, thisArg?: any): Exclude<S, Falsy>[];
}
k-funk commented 3 years ago

Here's a solution that doesn't require importing/defining a function, nor overloading a prototype. Verbose as hell, but 🤷

const arr: (Thing | undefined)[] = list.map(...) // may return undefined, or may return a Thing
arr.filter((thing): thing is Thing => !!thing)
vctormb commented 2 years ago

Here's a solution that doesn't require importing/defining a function, nor overloading a prototype. Verbose as hell, but 🤷

const arr: (Thing | undefined)[] = list.map(...) // may return undefined, or may return a Thing
arr.filter((thing): thing is Thing => !!thing)

cool! btw, what's the difference between your example and this one?:

arr.filter(Boolean) as Thing[]
mrlubos commented 2 years ago

Nice one @Etheryte, thank you!

levibuzolic commented 2 years ago

For anybody using @Etheryte's great tip from https://github.com/microsoft/TypeScript/issues/16655#issuecomment-797044678 you may also want to extend the ReadonlyArray interface with the same fix:

type Falsy = false | 0 | '' | null | undefined;
interface Array<T> {
  filter<S extends T>(predicate: BooleanConstructor, thisArg?: any): Exclude<S, Falsy>[];
}
interface ReadonlyArray<T> {
  filter<S extends T>(predicate: BooleanConstructor, thisArg?: any): Exclude<S, Falsy>[];
}
thorn0 commented 2 years ago

Why is the <S extends T> needed though? Seems to work without it too (in Exclude<S, Falsy>, S needs to be replaced with T).

karlhorky commented 1 year ago

Another pattern is using Boolean() along with && - a common pattern for example in embedded JS expressions in JSX with React:

function CartMessage(props: { cartItems: number }) {
  return <div>Cart contains ${props.cartItems} items</div>;
}

export default function App() {
  const cartItems = 0 as number | null;

  return (
    <div className="App">
      <h3>Using number on LHS of &&</h3>
      <h4>(see 0 below)</h4>
      <div>{cartItems && <CartMessage cartItems={cartItems} />}</div>

      <h3>Using Boolean() on LHS of &&</h3>
      <h4>(no 0 below)</h4>
      <div>{Boolean(cartItems) && <CartMessage cartItems={cartItems} />}</div>
    </div>
  );
}

https://codesandbox.io/s/falling-https-zvg2xv?file=/src/App.tsx

In the first example, the 0 is rendered on the screen because it is falsey:

Screen Shot 2022-10-03 at 18 37 04

To avoid this, the Boolean() constructor is used to convert the number on the LHS of the && to a boolean, but this causes a problem with the types:

Type 'number | null' is not assignable to type 'number'.
  Type 'null' is not assignable to type 'number'.ts(2322)

If the Boolean() constructor behaved the same way that !! (double negation) did, this wouldn't be an issue.

Also affects strings:

let name = '' as string | null;

// Renders empty string, usually not a problem but can cause crashes with React Native
// https://github.com/facebook/react-native/issues/20764
{name && <Profile name={name} />}

// 💥 Type 'string | null' is not assignable to type 'string'.
{Boolean(name) && <Profile name={name} />}

Alternatives

There are alternatives to doing this:

// Double negation:
{!!cartItems && <CartMessage cartItems={cartItems} />}

// Greater than:
{cartItems > 0 && <CartMessage cartItems={cartItems} />}

But sometimes the Boolean() option may be desirable because of its expressiveness.

kkmuffme commented 1 year ago

I think the "Awaiting more feedback" label can be removed, as tons of people seem to have this issue.

Hebilicious commented 1 year ago

Another workaround that I haven't seen mentioned is to create another Boolean object (Tested with TS 5.1.6) :

type Falsy = false | 0 | "" | null | undefined
const _Boolean = <T extends any>(v: T): v is Exclude<typeof v, Falsy> => Boolean(v)
const filtered = [1, null, undefined, 2, 3].filter(_Boolean) // number[]
const orders = settled
                    .map((result) => (result.status === "fulfilled" ? result.value : false))
                    .filter(_Boolean) // array of fullfilled values

It would be great if that was the default behaviour of Boolean

SergeOlabin commented 9 months ago

did anyone found a way to make it work within a project so that Boolean() can act as a null check ?

upd: as has been posted adding following seems solving MOST of the cases.

interface BooleanConstructor {
  new (value?: any): boolean;
  <T>(value?: T): value is Exclude<T, false | null | undefined | '' | 0>;
  readonly prototype: boolean;
}

although, in my case next piece still errored byts:2345:

  const accountUserList = useAsync(async () => {
    if (Boolean(customer?.name)) return await getAccountUserList(customer?.name);
  }, [customer?.name, getAccountUserList]);

at the same time with no error

const accountUserList = useAsync(async () => {
    if (customer?.name) return await getAccountUserList(customer?.name);
  }, [customer?.name, getAccountUserList]);
JH1ller commented 5 months ago

@SergeOlabin sadly that custom BooleanConstructor doesn't work when you do Boolean(valueA && valueB)

Would really be great this get this fixed from TS side.

mmkal commented 3 months ago

This was reverted due to it causing a breaking change in 3.5. The simple repro looks like this:

declare const Bullean: {
    new(value?: any): Boolean;
    <T>(value?: T): value is Exclude<T, false | null | undefined | '' | 0>;
};

declare const arr: any[];
// Was any[], now (3.5) unknown[]
const arr2 = arr.filter(Bullean);

There might be something wrong in generic inference, but it's not yet clear. We'll need this scenario to work before adding it back in.

@RyanCavanaugh maybe this is known, but some time between when you wrote this in 2019 and today, this breaking change seems to have happened anyway. Playground link

I'm doing my yearly rabbit-hole dive of why-can't-we-have-.filter(Boolean) - and this comment seems to be the only one that this solution doesn't solve for - but based on it already existing in v5.4.5, maybe it should be considered unrelated, and no longer a blocker to updating lib/es5.d.ts along the lines of https://github.com/total-typescript/ts-reset/issues/168#issuecomment-2037805462?

Here's another playground with that proposed fix applied as a module augmentation, along with some type tests for the edge cases/prior bugs reported over the years from this issue, https://github.com/microsoft/TypeScript/issues/50387, https://github.com/total-typescript/ts-reset/issues/168, and https://github.com/microsoft/TypeScript/issues/31551. I put links to GitHub issue comments as inline comments in the code. Here's the augmentation to Array<T> required (same would be needed for ReadonlyArray<T>):

interface Array<T> {
  // this overload already exists in lib/es5.d.ts, re-adding here to make sure the overloads are checked in the correct order
  filter<S extends T>(predicate: (value: T, index: number, array: T[]) => value is S, thisArg?: any): S[];

  // this is the proposed change to lib/es5.d.ts
  filter<P extends (value: T, index: number, array: T[]) => unknown>(
    predicate: P,
    thisArg?: unknown,
  ): (P extends BooleanConstructor ? Exclude<T, null | undefined | 0 | false | ''> : T)[];
}

What do you think? Can we have .filter(Boolean) work mostly as you'd expect in 2024? The only objection I can find that still seems to be a problem is your Bullean example, and since that's already in a released version of TypeScript, why not opportunistically get in a fix for .filter(Boolean) after all these years??

CodingDive commented 3 months ago

The boolean constructor is so useful for maps, JSX and in general far more readable than doing the double negation.

@RyanCavanaugh @DanielRosenwasser @sheetalkamat could you please consider fixing this in one of the next versions? 🙏

epmatsw commented 3 months ago

This seems particularly strange with the new predicate inference in 5.5. If you do .filter(t => !!t) you get narrowing, but if you do .filter(Boolean), you don't:

https://www.typescriptlang.org/play/?ts=5.5.2#code/MYewdgzgLgBFAWBLMBzCMC8MDaBXMAJgKYBmyRBANDCQIYA2ER1UATrszAAzUCMAujFrpQkKAG4AUKOhx2CAJ69McJKggA6MvShFWACn1QAlJgB8MAISWTUmbDa5FAJhUJkaLYh179AIRAQeiJaMGMpGBggA