rescript-compiler icon indicating copy to clipboard operation
rescript-compiler copied to clipboard

Add support to use `[@bs.unwrap]` with labeled arguments

Open liubko opened this issue 7 years ago • 11 comments

[email protected]

Can't use [bs.unwrap] with labeled arguments.

Code

[@bs.obj]
external func :
  (
    ~param: string,
    ~polyParam: [@bs.unwrap] [ | `Str(string) | `Int(int)],
    unit
  ) =>
  _ =
  "";

Error

bs.obj label polyParam does not support [@bs.unwrap] arguments

Try it online

liubko avatar May 23 '18 17:05 liubko

@bobzhang This would come in very handy for writing zero-cost bindings for the new ReasonReact.

cknitt avatar Mar 30 '19 07:03 cknitt

This would only work when we know the _ is abstract type, otherwise when you create an object t, the type of t##polyParam is not known. How useful when you only support abstract type? thanks

bobzhang avatar Apr 01 '19 13:04 bobzhang

For binding React components, the type of the result is the abstract type React.element. So this would definitely be very useful! 👍

Another use case would be "options" objects that are passed to React components in some apis and that you model as an abstract type.

cknitt avatar Apr 01 '19 17:04 cknitt

Where is React.element defined, what kind of ops are allowed on element if it is abstract?

bobzhang avatar Apr 02 '19 03:04 bobzhang

React.element is defined here: https://github.com/reasonml/reason-react/blob/master/src/React.re

cknitt avatar Apr 02 '19 07:04 cknitt

@rickyvetter This is the issue I mentioned yesterday. @jsiebern needs this for his material-ui bindings, too.

cknitt avatar Apr 13 '19 05:04 cknitt

what is the status of this? Is it still in progress?

shurygindv avatar Oct 16 '19 08:10 shurygindv

Hello,

Is this issue resolvable ? This would help a lot to make 0 cost bindings for React components libraries ! I don't know if I can help for this kind of issues :)

Thanks for your works

DCKT avatar Oct 28 '19 06:10 DCKT

style prop in React Native components take a single Style.t object or an array (or list) of them. Until now, we used to cast array(Style.t) as Style.t, but we have some interest in differentiating between them. There are a few possible approaches, so you may follow them in the WIP PR.

The closest to @bs.unwrap is to use @bs.ignore, but that introduces a bit more noise. IMO, defining multiple props (polyParam__string and polyParam__int for the above example) is more straightforward, if not quite as rigorous.

sgny avatar Nov 01 '19 20:11 sgny

With BuckleScript 7 out the door, I've cleaned up a lot of my bindings by removing usages of [@bs.deriving abstract]. Now I can usually use simple record mapping, except in cases where fields must be optional (as in, they should not appear in the generated JS object). In every such case I've come across it's been for generating an argument for a JS method binding. I.e., these are not objects I ever need to read from directly and the type could be abstract. Here, I've found [@bs.obj] to be nicer than [@bs.deriving abstract]; there's less generated code and I think the intent in the binding is clearer.

Unfortunately, since [@bs.unwrap] can't be used with [@bs.obj], I either can't convert such cases away from [@bs.deriving abstract], or worse, have to change my binding away from [@bs.obj] when the binding needs to be updated for something like supporting untagged union types. Since these types would be abstract anyway, I could accept the limitation indicated in https://github.com/BuckleScript/bucklescript/issues/2827#issuecomment-478825051 and it seems to be that this would be a somewhat frequent use case.

nirvdrum avatar Dec 27 '19 05:12 nirvdrum

Would be still super useful as of rescript v9.1.4 so that "variant" React props can be passed nicely to external modules, for example with Headless React:

@react.component @module("@headlessui/react")
external make: (
  ~\"as": @unwrap [#Str(string) | #El(React.element)],
  ~className: string
) => React.element = "Menu"

<Menu \"as"={#Str("div")} className="px-3 ..." />
<Menu \"as"={#El(<> </>)} className="px-3 ..." />

As far as I'm aware there's no simple alternative to this other than to define 2 separate MenuString and MenuEl components that both reference Menu but with overloaded as labeled arguments, which is getting to be very messy.

pheuter avatar Jul 28 '21 17:07 pheuter

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Aug 07 '23 05:08 stale[bot]

This is at least partly solved by unboxed variants in v11.

zth avatar Aug 07 '23 06:08 zth