prelude-ts icon indicating copy to clipboard operation
prelude-ts copied to clipboard

Can areEqual be undefined-safe too?

Open malcolmredheron opened this issue 2 years ago • 2 comments

As https://github.com/emmanueltouzery/prelude-ts/blob/736e40c9dcea6a3179daf1db52152159e0ab7236/src/Comparison.ts#L82, says, areEqual is null-safe. However, the signature accepts undefined, and then it crashes if the undefined is in obj (versus obj2).

is there a reason that undefined isn't handled here? Or would you take a PR that changes this?

malcolmredheron avatar Jul 28 '22 17:07 malcolmredheron

hm at the time i didn't realize any included also undefined...

actually i think the right fix would be in hasEquals:

export function hasEquals(v: WithEquality): v is HasEquals {
    // there is a reason why we check only for equals, not for hashCode.
    // we want to decide which codepath to take: === or equals/hashcode.
    // if there is a equals function then we don't want ===, regardless of
    // whether there is a hashCode method or not. If there is a equals
    // and not hashCode, we want to go on the equals/hashCode codepath,
    // which will blow a little later at runtime if the hashCode is missing.
-    return ((<HasEquals>v).equals !== undefined);
+    return "equals" in v;
}

I'd definitely merge such a PR (assuming the tests pass and so on), and release shortly afterwards.

PS: sorry about the delay, I missed the notification during my holidays.

emmanueltouzery avatar Sep 14 '22 19:09 emmanueltouzery

Hmm actually I'm not sure anymore whether in is undefined-safe. But bottom line yes, a PR would be merged.

emmanueltouzery avatar Sep 14 '22 20:09 emmanueltouzery