Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

isNull, isUndefined convenience functions #125

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jmagaram
Copy link
Contributor

@jmagaram jmagaram commented Mar 25, 2023

I was writing some code checking for null and undefined and found it awkward. Originally I tried %raw which worked but wasn't pretty. I tried a bunch of Nullable.toOption and then checked for None and that was a mess. Then I realized I could use the == operator, but that also wasn't pretty. There was the uncertainty of do I use == or ===. In JavaScript null==undefined I think. The worst is if you are checking if something is null or undefined...

Nullable.make(obj)==Nullable.null || Nullable.make(obj)==Nullable.undefined
Nullable.make(obj).isNullOrUndefined

So here are some convenience functions like Nullable.isNull and Nullable.isUndefined that are analogous to Option.isNone and Option.isSome that make the code more readable. Includes tests and docs.

@tsnobip
Copy link
Contributor

tsnobip commented Jul 25, 2023

this could be simplified with v11 thanks to the new representation of untagged variants that will allow Nullable.t to be defined like this:

module Nullable = {
  @unboxed type t<'a> = Present('a) | @as(null) Null | @as(undefined) Undefined
}

Then those utility functions could be replaced with a simple pattern matching.

@DZakh
Copy link
Contributor

DZakh commented Jul 25, 2023

It's not related to the post, but I've just thought of a possible problem that Nullable.t<Nullable.t<string>> like Present(Null) is matched as Null which might cause problems. Maybe not a big problem since it wasn't working before as well. But to make it work correctly and improve DX I think that @unboxed types should collapse like Nullable.t<Nullable.t<string>>->Nullable.t<string>

So maybe we should recover the @cristianoc's idea from some time ago? https://forum.rescript-lang.org/t/rfc-automatic-optional-chaining/3791/17?u=dzakh

@cristianoc
Copy link
Contributor

It's not related to the post, but I've just thought of a possible problem that Nullable.t<Nullable.t<string>> like Present(Null) is matched as Null which might cause problems. Maybe not a big problem since it wasn't working before as well. But to make it work correctly and improve DX I think that @unboxed types should collapse like Nullable.t<Nullable.t<string>>->Nullable.t<string>

So maybe we should recover the @cristianoc's idea from some time ago? https://forum.rescript-lang.org/t/rfc-automatic-optional-chaining/3791/17?u=dzakh

That idea amounts to accepting that nested nullable is the same as nullable.
One can add a convenience function to that effect.

@jmagaram
Copy link
Contributor Author

Even if pattern matching can be used, I still think having boolean-returning convenience functions as in this PR are useful because they are easily discoverable and can sometimes be used for more concise code than pattern matching. I don't have my code in front of me but know I have often used Option.isNone and Option.isSome even though I could use pattern matching for that.

@tsnobip
Copy link
Contributor

tsnobip commented Jul 25, 2023

yeah I agree, those utility functions can come handy from time to time (eg as a parameter of a filter function).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants