Skip to content

Nullable additions #67

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

Merged
merged 10 commits into from
Feb 25, 2023
Merged

Nullable additions #67

merged 10 commits into from
Feb 25, 2023

Conversation

glennsl
Copy link
Contributor

@glennsl glennsl commented Feb 23, 2023

Other monads like Option, Result and List has them, so I think it makes sense for Nullable to have them too.

@zth
Copy link
Collaborator

zth commented Feb 23, 2023

I like these, I think adding them makes sense. @cknitt thoughts?

@cknitt
Copy link
Member

cknitt commented Feb 23, 2023

I like these, too, but I think the signature for map and flatMap is incorrect. It is

let map: (t<'a>, 'a => 'b) => option<'b>
let flatMap: (t<'a>, 'a => option<'b>) => option<'b>

but should be

let map: (t<'a>, 'a => 'b) => t<'b>
let flatMap: (t<'a>, 'a => t<'b>) => t<'b>

@glennsl
Copy link
Contributor Author

glennsl commented Feb 23, 2023

Oops. Shows where I've copied them from! I'm also missing the interface file that was added a few days ago, so I need to add signatures there as well.

@glennsl glennsl force-pushed the feat/nullable/additions branch from c1523fc to c0e22ca Compare February 23, 2023 21:15
@cknitt
Copy link
Member

cknitt commented Feb 24, 2023

So map and flatMap transform null to undefined. I was wondering if they shouldn't leave null being null?

@glennsl
Copy link
Contributor Author

glennsl commented Feb 24, 2023

I kind of agree, but that requires using Obj.magic. See what you think of it in fdfb5a0

@cknitt
Copy link
Member

cknitt commented Feb 24, 2023

Looks good to me! 👍

Copy link
Collaborator

@zth zth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Looks good to you too @cknitt ?

@glennsl glennsl force-pushed the feat/nullable/additions branch from fdfb5a0 to 0ccfb1e Compare February 24, 2023 18:32
@glennsl glennsl force-pushed the feat/nullable/additions branch from 0ccfb1e to 3136da6 Compare February 24, 2023 18:34
@cknitt
Copy link
Member

cknitt commented Feb 25, 2023

Ready to merge from my side!

BTW, would be nice to have these additions for Null, too.

@zth zth merged commit 3a8de61 into rescript-lang:main Feb 25, 2023
@glennsl glennsl deleted the feat/nullable/additions branch February 25, 2023 11:28
@glennsl glennsl mentioned this pull request Feb 25, 2023
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