Rescript-compiler: Proposal: consolidate null/undefined checking under Js.Null

Created on 17 Aug 2017  路  6Comments  路  Source: rescript-lang/rescript-compiler

Aside from boolean conversion, the other bug is forgetting to check for either undefined or null when using Js.Null and Js.Undefined respectively. I think we should reduce the API learning surface from Js.Null, Js.Null_undefined and Js.Undefined to just Js.Null_undefined.

Perf concern: there should be no perf degradation; and the power-user-ish minimal perf increase in not checking for both isn't worth the bugs caused by it. For example: knowing that the field is only nullable, but forgetting that the field can be absent. Btw I think most engine would optimize the foo == null check which checks for both undefined and null.

Ergonomy: from the above point, compiling to foo == null is also a popular JS pattern for checking against null and undefined at the same time, and the only == linters don't warn against. And it's fewer overlapping modules to recommend to newcomers too. Wanna prevent nullables? See Js.Null_undefined.

Upgrade path: I think we can take the occasion to call the module Js.Nullable. This way we don't obstruct any existing code. Easy codemod too, if needed. The name's intuitive too, because when explaining null vs option we always call the JS ones "js nullables" already.

If we agree with this change, I'll submit the PR. Should be trivial.

Most helpful comment

You do need both from bs->js. Some JS callsites expect a strict null, not undefined, vice-versa.

All 6 comments

what's the semantics of foo == null, any link? so your proposal would be renaming Null_undefined to a shorter name 锛坅nd also removing Null and Undefined module)?

== does casting: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Comparison_Operators#Equality

http://blog.boyet.com/blog/javascriptlessons/javascript-an-acceptable-use-of-double-equals-just/

But that was mostly a small suggestion; some folks might not even like such check, and since it's in also concerns just JS artifact, we can disregard this. No special compiler hack needed if we don't use it.

Regarding Null_undefined: yes, basically. A single concise place to worry about js nullables.

A single place where to cover all concepts of js-level option types sounds like a great idea. Nullable seems a good name, unless there's a standard way to define null+undefined which is not a mouthful.

Also, keeping the distinction would mean embracing the complexity of multiple-page descriptions of the subtle differences between the two when used in certain arithmetic or other contexts.

js to Reason:
I guess that when going from js to Reason, 99% of the cases will convert null/undefined to None.
So I would only provide 1 conversion function in that direction.
Plus a way to test if the value is specifically null or undefined, to be used rarely.

Reason to js:
Either a default, probably "undefined", or two conversion functions.

You do need both from bs->js. Some JS callsites expect a strict null, not undefined, vice-versa.

@bobzhang The semantics of foo == null is very simple: it returns true if foo is null or undefined, in all other situations it returns false

[ECMAScript 2015 specification]

thanks for the input, finished in the master, btw are you aware of https://bucklescript.github.io/bucklescript/Manual.html#_return_value_checking_since_1_5_1
I also added nullable in place of null_undefined

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jordwalke picture jordwalke  路  4Comments

bobzhang picture bobzhang  路  4Comments

bobzhang picture bobzhang  路  5Comments

wyze picture wyze  路  3Comments

TheSpyder picture TheSpyder  路  5Comments