rescript-association / reasonml.org

Deprecated in favor of rescript-lang.org
MIT License
125 stars 34 forks source link

Documenting Js.Undefined #165

Open johnridesabike opened 4 years ago

johnridesabike commented 4 years ago

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:

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.

bobzhang commented 4 years ago

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?

cristianoc commented 4 years ago

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".

johnridesabike commented 4 years ago

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?

bobzhang commented 4 years ago

On Mon, May 4, 2020 at 8:59 PM John Jackson notifications@github.com 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