Handle null in TypeScript optional #10907
Conversation
| static ofUndefinable<Value>(value: Value | undefined): Optional<Value> { | ||
| if (value === undefined) { | ||
| return Optional.empty(); | ||
| static ofUndefinable<Value>(value: Value | undefined | null): Optional<Value> { |
There was a problem hiding this comment.
We should maybe change the name of the method, or add a new method ofNullable(..).
What do you think?
There was a problem hiding this comment.
I thank about ofEmptyable but I am not really convinced.
There was a problem hiding this comment.
I like keeping this one to avoid breaking change and there is no really good naming for this :)
There was a problem hiding this comment.
How about having a single method named ofNullable?
This would then be similar to the java Api that we're using as an aspiration.
I'm not sure we need to worry about breaking things here:
- we have control over jhlite's webapp and it's not used externally
- in the generated Optional.ts (that is not changed in this PR by the way), people having already applied it have no reason to re-apply it
There was a problem hiding this comment.
True (and we need to change the generated one :D)
There was a problem hiding this comment.
I rename to ofNullable. Not in scope, but I added NaN to nullable value.
NolwennD
force-pushed
the
10897-handle-null-in-optional
branch
from
5418648
to
20ff2dc
Compare
NolwennD
force-pushed
the
10897-handle-null-in-optional
branch
from
20ff2dc
to
5c33df3
Compare
|
I had |
NolwennD
force-pushed
the
10897-handle-null-in-optional
branch
from
6fd7581
to
8e01c70
Compare
|
Thanks @NolwennD |
|
Sorry, I've not seen this PR before, but I think it could be interesting to choose a better name than ofNullable like safeOf() or ofSafe(). Because in java, it's ok, there is not this problem like js with null, undefined or NaN. So that's why I think ofNullable is confusing. |
Fix #10897