Add a non-abstract @bs.deriving for records

[tsnobip]

BS7 record as object representation led to much easier bindings, but two use cases are not covered by this new representation:

• bindings to objects that need to be JSON-compatible (e.g. without any undefined fields)
• bindings to objects with many optional fields (often used as configuration objects), that are too cumbersome to be created with the record creation syntax

Today, these two use cases are covered by the use of @bs.deriving abstract or @bs.obj annotations, but that makes the target type abstract, which removes the ability to use the very convenient features of records in OCaml like pattern matching.

Having a non-abstract @bs.deriving annotation that would keep the type as record would bring the best of both worlds and would make @bs.deriving abstract or @bs.obj annotations almost useless.

An optional private subannotation ([@bs.deriving private]) would make the type private and would force the use of the generated creation function to create a record of this type (for JSON-compliant objects for example).

A safer approach would be to make the type private by default and to provide a public or non-private subannotation to opt-out this effect.

[jfrolich]

Can you describe how this would work in code? It would be awesome to have a non-abstract @bs.deriving function. This doesn't solve the case of not generating object keys for None values. But I think we can create a serialization function as part of the @bs.deriving.

There is also another use-case for dealing with JSON. Sometimes you'd like the keys to be hidden when having a JSON compatible data structure. But in other cases you'd want the None keys to be explicitly null. I think that might be an even more common use case when the schema is fixed (keys not existing is then not possible).

[jfrolich]

I think the following are interesting solutions:

• having an annotation on the field level to make the key hidden if the value is undefined
• having an annotation on the record level to make the key hidden
• making undefined keys hidden by default for records and having the annotation the other way around

This could look like this:

type foo: {
  [@bs.hidden]
  bar: option(string)
}

let foo = {foo: None}

would compile to

var foo = {};

[jfrolich]

Incidentally this make a lot of sense for a number of libraries. For instance in the reason-react-native library we couldn't convert the style object to a record because it would create a JS object with every possible style as an key with an undefined value.

[tsnobip]

Yes, all this was written with very limited knowledge of bucklescript internals, I have no idea if it's possible to have hidden-field objects to represent records and if the two representations are interchangeable for Bucklescript.

What I had in mind was that the following code:

module Foo =
  struct
    type t = {
      foo: int option [@bs.optional ];
      bar: int option;
      baz: string;
  } [@@bs.deriving]
  end

let foo = Foo.t ~bar:None ~baz:"baz" ()

would produce:

module Foo :
  sig
    type t = private {
      foo: int option;
      bar: int option;
      baz: string;}
    val t : ?foo:int -> bar:int option -> baz:string -> unit -> t
  end =
  struct
    type t = {
      foo: int option;
      bar: int option;
      baz: string;}
    let t ?foo  ~bar ~baz () = (* generated creator function *)
  end 

let foo = Foo.t ~bar:None ~baz:"baz" ()

that would be compiled in JS to:

var foo = {
  bar: undefined,
  baz: "baz"
};

[bobzhang]

This is bad for performance of generated code — you want shape to be fixed. I am thinking we only apply flexible shapes for very large object which contains more than ten optionals

[cristianoc]

Supporting sparse records with threshold could be good. (Based on the number of fields or perhaps just the number of optional fields). And perhaps let people override the heuristic too.

[jfrolich]

Supporting sparse records with threshold could be good. (Based on the number of fields or perhaps just the number of optional fields). And perhaps let people override the heuristic too.

Overriding the heuristic would indeed be great, as the behavior slightly changes (js dependencies can check for keys instead of checking if the value is undefined).

[cristianoc]

This is one should really be clean up:
https://github.com/reasonml/reason-react/blob/master/src/ReactDOM.re#L56

It would be great to remove the magic: drop both @bs.deriving abstract and @bs.optional.

[tsnobip]

Is it bad for performance when creating a record or when using it?

If we want finer control on performances, maybe we can have two different annotations, one that would just generate a creator function and one for hiding the fields.

[tsnobip]

I think we can close this thanks to #5423.

[TheSpyder]

Sort of. Optional fields specified as None using a variable still emit the field directly meaning the value is undefined - I’ve been meaning to log an issue about it.

that’s the one thing deriving abstract and @obj did that optional fields don’t do.

[tsnobip]

Sort of. Optional fields specified as None using a variable still emit the field directly meaning the value is undefined - I’ve been meaning to log an issue about it.

that’s the one thing deriving abstract and @obj did that optional fields don’t do.

Are you sure @TheSpyder?

type t = {
  name: string,
  age?: int,
}

let foo = {
  name: "foo",
  age: ?None,
}

let makeFoo = (name, age) => {
  name,
  ?age,
}

let foo2 = makeFoo("foo2", None)

compiles to:

// Generated by ReScript, PLEASE EDIT WITH CARE
'use strict';


function makeFoo(name, age) {
  return {
          name: name,
          age: age
        };
}

var foo2 = {
  name: "foo2"
};

var foo = {
  name: "foo"
};

exports.foo = foo;
exports.makeFoo = makeFoo;
exports.foo2 = foo2;
/* No side effect */

cf playground

[TheSpyder]

WIthin a single scope, they are equivalent. But when you have cross-scope functions such as this one the problem becomes apparent:

function makeFoo(name, age) {
  return {
          name: name,
          age: age
        };
}

@obj and deriving abstract add extra runtime checks to not set the age field when it's None:

function makeFoo(name, age) {
  var tmp = {
    name: name
  };
  if (age !== undefined) {
    tmp.age = age;
  }
  return tmp;
}

In some JS contexts there is unfortunately a difference between a value not set and it being set to undefined.

Updated playground link