Rescript-compiler: Support non-primitive types in deriving (jsConverter)

[tomraithel]

The new jsConverter from bs 2.1.0 is currently not able to convert nested types. It works well with primitives, but nested types can not be derived from the generated converter functions.

See this simple example:

type address = {
  street: string,
  houseNumber: int
};

[@bs.deriving jsConverter]
type user = {
  name: string,
  address: option(address)
};

let test = (uJs) => {
  let u = userFromJs(uJs);
  let a =
    switch u.address {
    | None => " (no address)"
    | Some(ad) => " (" ++ ad.street ++ " " ++ string_of_int(ad.houseNumber) ++ ")"
    };
  Js.log("Hello" ++ u.name ++ a)
}; 

If I call test in JS like so:

const demo = require('./demo.bs');

demo.test({
  name: 'Tom',
  address: { street: 'Foo', houseNumber: 123 },
});

Current behaviour

I´ll get an error for access on the address:

     a = " (" + (ad[/* street */0] + (" " + (Pervasives.string_of_int(ad[/* houseNumber */1]) + ")")));
                  ^

TypeError: Cannot read property '0' of undefined

Expected output

Hello Tom (Foo 123)

[bobzhang]

currently, we don't apply FromJs recursively, there are good reasons for this, since addressFromJs is not sure to exist. For the work around, you can do it this way:

[@bs.deriving jsConverter]
type address = {
  street: string,
  houseNumber: int
};

[@bs.deriving jsConverter]
type user = {
  name: string,
  address: Js.nullable (abs_address)
};

In the future, we may provide an option to turn on recursive conversion for example:

[@bs.deriving {jsConverter = recursive} ]
type user = {
  name : string ,
  address : address
}

Note things would still get complicated with parameterized types

[Ebuall]

We could specify where we want to apply recursively or even pass implementation if needed.

[@bs.deriving jsConverter]
type address = {
  street: string,
  houseNumber: int
};

let addressOptionToJs = (x) => x 
  |> Js.Option.map([@bs] addressToJs) 
  |> Js.Nullable.from_opt
;

let addressOptionFromJs = (x) => x
  |> Js.Nullable.to_opt
  |> Js.Option.map([@bs] addressFromJs)
;

[@bs.deriving jsConverter]
type user = {
  name: string,
  [@bs.rec (addressOptionToJs, addressOptionFromJs)]
  address: option(address) 
};

[anmonteiro]

@bobzhang in your example above, did you mean to write {jsConverter: newType} for the addres type?
Or where does abs_address come from?

[jaredly]

We should definitely apply FromJs & ToJs recursively -- currently the behavior is just broken for non-primitive types.

[ryuheechul]

We really should and I think that will make Reason/BuckleScript to be more attractive to JS developers, too.

[cullophid]

👍 for this. Right now converting between Js and Reason is quite cumbersome.
Currently they are not well suited for the most common use cases, like JSON conversion or js interop.

[cullophid]

I would also argue that it should no be an option, but the default :)
If you have a nested structure you would always want the whole thing to be converted.
if you have a shallow structure the cost is irrelevant.

[flash-gordon]

@bobzhang this is not an issue anymore in BS 7 since records are now compiled to JS objects, right?

[bobzhang]

indeed, jsconverter is not very useful after such change

[tomraithel]

I can confirm, with BS 7 the jsConverter is not needed anymore and my example from above now works in a simplified version:

type address = {
  street: string,
  houseNumber: int,
};

type user = {
  name: string,
  address: option(address),
};

let test = u => {
  let a =
    switch (u.address) {
    | None => " (no address)"
    | Some(ad) =>
      " (" ++ ad.street ++ " " ++ string_of_int(ad.houseNumber) ++ ")"
    };
  Js.log("Hello " ++ u.name ++ a);
};

Well done and thanks for this great language! 👏