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

typescript bindings & strict null checks #83

Closed
emmanueltouzery opened this issue Dec 12, 2016 · 8 comments
Closed

typescript bindings & strict null checks #83

emmanueltouzery opened this issue Dec 12, 2016 · 8 comments
Milestone

Comments

@emmanueltouzery
Copy link
Collaborator

When using the strict null checks mode in typescript, null & undefined are separated from the other types.

In other words, the signature of fromNull is definitely wrong:

fromNull<V>(val: V): Maybe<V>;

it should be:

fromNull<V>(val: V|null): Maybe<V>;

and maybe we also want to allow undefined (mapping it to None):

fromNull<V>(val: V|null|undefined): Maybe<V>;

That also mean that orSome(null) will not compile. We could do:

orSome(val: T|null|undefined): T|null|undefined;

or we do:

orNull(): T|null

What do you think? Would you accept a PR regarding that (but then, implementing which variant?)

@ulfryk ulfryk added this to the 0.9 Release milestone Dec 12, 2016
@ulfryk
Copy link
Member

ulfryk commented Dec 12, 2016

Good point @emmanueltouzery , but I have to think about it for a while. Current implementation of Maybe was meant to work with JS/TS where special type null (and awkward type undefined) where treated as subtypes of every other type. And it does not work with strict mode. On the other hand this lib should work well with TS in any configuration…

This also means we'll have to add typings tests for both scenarios (with --strictNullChecks and without it).

@emmanueltouzery
Copy link
Collaborator Author

hmm maybe there is a misunderstanding. if we write for instance T|null we cover both strictNullChecks and default operation.

In the default TS mode, T or any will include already null & undefined. So in that mode, T|null|undefined is the same as T. No effect. It's like you had just written T.
If the strict null checks mode, you must explicitely list null & undefined if you want to allow them.

So the null & undefined annotations are required for proper strict null checks operation, but they don't have any effect on the default typescript operation.

So:

fromNull<V>(val: V|null): Maybe<V>;

helps in the strict null checks mode.. but has no effect on the default typescript operation.

@ulfryk
Copy link
Member

ulfryk commented Dec 12, 2016

Ok, looks great. We'll be pleased to see a PR with an upgrade you proposed :) 👍

I think that T | null | undefined variant would properly reflect current functionality. I'd also suggest creating type alias -> type Nullable<T> = T | null | undefined, WDYT ?

Of course orNull(): T | null does not need undefined as it'll never return it.

@emmanueltouzery
Copy link
Collaborator Author

There is one but. I think some of this requires typescript 2+ (strict nulls or not). But well, it's the future and the present.

@ulfryk
Copy link
Member

ulfryk commented Dec 12, 2016

Updating typescript to 2.1 is welcome too.

@emmanueltouzery
Copy link
Collaborator Author

This typescript strict null checks option achieves exactly what it should, make you think about how you deal with null & undefined. Just if you're interested... I was actually surprised that Maybe.fromNull(null) returns None and Maybe.of(5).map(x => null) throws, because in scala (and javaslang, a java library offering FP features), Some(null) is allowed, and is the result to both expressions (but it's not in java's builtin Optional. ).
You probably thought over your decision well, but if you're interested, there is an article on the subject of null, Some, None, and java's Optional.

I'll have the PR ready soon. Maybe no need to have Nullable type alias, since the name could be confusing ("nullable" but undefined is allowed too), and I think we would use it in only one spot for now (fromNullable).

emmanueltouzery added a commit to emmanueltouzery/monet.js that referenced this issue Dec 13, 2016
@ulfryk
Copy link
Member

ulfryk commented Dec 13, 2016

See discussion in #53 abut why mapping to null/undefined and Some(null/undefined) is forbidden in Monet.

I agree that Nullable type alias may be obsolete.

emmanueltouzery added a commit to emmanueltouzery/monet.js that referenced this issue Dec 13, 2016
emmanueltouzery added a commit to emmanueltouzery/monet.js that referenced this issue Dec 13, 2016
ulfryk added a commit that referenced this issue Dec 13, 2016
fixes #83 "typescript bindings & strict null checks"
@emmanueltouzery
Copy link
Collaborator Author

Closing as it was applied

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

No branches or pull requests

2 participants