reasonml.org icon indicating copy to clipboard operation
reasonml.org copied to clipboard

Documenting Js.Undefined

Open johnridesabike opened this issue 5 years ago • 4 comments

There are some official Belt functions that use Js.Undefined, but it's not documented on the Reason or BuckleScript docs. The page on option mentions how to use Js.Nullable for interoperability, but not Undefined.

Some questions I have about Js.Undefined.t:

  • Is it obsolete now that option is unboxed?
  • Can you nest it?
  • Can you annotate externals with it as polymorphic instead of a concrete type? (Js.Undefined.t('a))

I had long assumed it was deprecated, but it’s been referenced in a recent BuckleScript blog post. I think that as long as it’s still being officially used on the BuckleScript site, it should be explained in the docs.

I know its API has basic documentation but there’s no explanation as to how it works or what its purpose is.

I would submit a PR myself, but I don't understand the type enough to adequately write about it.

johnridesabike avatar Apr 13 '20 16:04 johnridesabike

There are some official Belt functions that use Js.Undefined

The reason is that nested Undefined is flattened which is contrary to option so that the generated code is slightly better than the option.

Is it obsolete now that option is unboxed?

It's less used when you don't care about perf or code size or in non-polymorphic code

Can you nest it?

Undefined is flattened contrary to option

Can you annotate externals with it as polymorphic instead of a concrete type? (Js.Undefined.t('a))

I am not sure I fully get the question, why it is problematic here?

bobzhang avatar May 04 '20 08:05 bobzhang

Can you annotate externals with it as polymorphic instead of a concrete type? (Js.Undefined.t('a))

I am not sure I fully get the question, why it is problematic here?

I think that such a binding can be unsound precisely because of flattening. Or, just with "surprising behaviour".

cristianoc avatar May 04 '20 08:05 cristianoc

Can you annotate externals with it as polymorphic instead of a concrete type? (Js.Undefined.t('a))

I am not sure I fully get the question, why it is problematic here?

I’m thinking of the caveats listed on the Reason docs for option:

  • Never, EVER, pass a nested option value (e.g. Some(Some(Some(5)))) into the JS side.
  • Never, EVER, annotate a value coming from JS as option('a). Always give the concrete, non-polymorphic type.

The second one in particular. Do these apply to Js.undefined the same way?

johnridesabike avatar May 04 '20 12:05 johnridesabike

On Mon, May 4, 2020 at 8:59 PM John Jackson [email protected] wrote:

Can you annotate externals with it as polymorphic instead of a concrete type? (Js.Undefined.t('a))

I am not sure I fully get the question, why it is problematic here?

I’m thinking of the caveats listed on the Reason docs for option https://reasonml.github.io/docs/en/null-undefined-option#caveat-1:

  • Never, EVER, pass a nested option value (e.g. Some(Some(Some(5)))) into the JS side.

That's not exact. Some (Some (Some (5)))) is just 5 which is fine. It is problematic when you pass Some None or (Some (Some (Some (None)))))

  • Never, EVER, annotate a value coming from JS as option('a). Always give the concrete, non-polymorphic type.

The second one in particular. Do these apply to Js.undefined the same way?

Monomorphic is always preferred to polymorphic -- you get better performance and more safety check. If you really want bind to polymorphic optional, 'a Js.undefined is safer here since it is a JS thing, while 'a option is not

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/reason-association/reasonml.org/issues/165#issuecomment-623448355, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFWMK2VM2K2U357CGPQB2LRP23ZZANCNFSM4MHCWBAA .

-- Regards -- Hongbo Zhang

bobzhang avatar May 06 '20 08:05 bobzhang