bs.deriving abstract: start with a record type or a creator function?

[chenglou]

Here's an overview of bs.deriving abstract currently:

[@bs.deriving abstract]
type foo = {
  high: int,
  low: option(int),
};

This generates:

• An abstract type t
• A creator function: let foo: (~high: int, ~low: option(int)=?, unit) => t
• Getters: high, low
• Setters: highSet, highLow`
• The original record type is erased
• You can annotate the labels to make the creation function/getter/setter use a different name that's otherwise invalid, e.g. high(s) might compile to s["aria-high"].
• Marking a record field as mutable generates the correct setter that mutates.

The goal is that we get some nice helpers, but also that it can generate platform-agnostic code. E.g. for JS backend, we can compile that abstract type to a JS object.

The other option is to start with a creator function:

[@bs.deriving abstract]
let foo = (~high: int, ~low: option(int)=?, unit) => "";

And generate the same helpers. This one has some advantages:

• It's clear which labels are optional. Front-end might use a lot of optional args. In the current record-based deriving, if you write low: foo you wouldn't know whether foo is option(int) and accidentally make low non-optional.
• You get to specify default values. This is limited, but nice.
• You can customize the body of the function for future purposes.
• The function name is customizable.
• Likely less confusing for newcomers, because the first one has a record type which cannot be used (it's erased). So no weird errors when trying to create a value of that record type.

The drawback is that you for mutable field you'd need a new annotation.

I feel that the creation-based option might make more sense, but I'm not too sure.

[bsansouci]

This is cool because it might make the native interop story better. Right now to talk to any C or objc library you need to write some C code that will translate the data structures around, or that will allow you to access the different fields you need.
For example UIKit uses CGRects all over the place, and those are C structs.
Because the creation-based deriving don’t enforce the shape of the data backing it, you could imagine it working with C structs underneath. Also this would be cheaper than converting the struct back and forth, which is currently the “best” solution (it’s nicer on the user at least)

Generally I’m not sure how we’ll do this on the native side of things, because we need type information (I think?) to generate the right C code.

[bobzhang]

the original record type can still be used, but the helpers wouldn't work on those record values, since the helpers all use the abstract type t

The original record will be completely erased.

For the ocaml native ppx, it would be straightforward. I did not think about C FFI, but it could be useful.

Note it is more useful than getter/setters.

1. Since it is abstract type, you can tweak the label to be something like "Content-Type", while still exporting idiomatic code
2. Allows mutability
3. The record types will be also useful in mli files, so you can optionally hide some record fields.

We will change high : low option to high : low [@bs.optional] to make it less magic

[jaredly]

oooh I quite like the "start from creator function" -- providing easy defaults & obvious optionals feels like a total win. I guess the main downside is missing mutability?

[rickyvetter]

Definitely prefer the creator function option because I see the record type as a big negative. If it gets erased then It'll be distracting to have it as the only representation visible.

[cristianoc]

The creator function looks pretty clean. Perhaps one can control mutability with dedicated attributes.

…

On Tue, Mar 27, 2018 at 7:02 PM, Ricky Vetter ***@***.***> wrote: Definitely prefer the creator function option because I see the record type as a big negative. If it gets erased then It'll be distracting to have it as the only representation visible. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2680 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHmKl6wREkQl-q9oCpIMUBI4I2p1qr-Lks5tinCngaJpZM4S8att> .

[bobzhang]

Note it is not just syntax. The semantics is isomorphic to record,

type ( + 'a, - 'b ) t =  {
    mutable a : 'a ; (* The compiler will complain about the variance of 'a *)
   b : 'b ; 
} [@@bs.deriving abstract]

Also it is pretty neat to provide more access control:

module S : sig 
   type t = private { a : int } [@@bs.deriving abstract]
end  = struct 
   type t = {mutable a : int; b  : int } [@@bs.deriving abstract]
end

In the implementation, we use the isomorphic record types for various checking, lie variance check, but it will be erased after such checks.

So its syntax is similar to creation function, but its semantics is isomorphic to record

[chenglou]

The original record will be completely erased

That's gonna be even more confusing to newcomers I feel; whereas the creator function doesn't confuse.

I'll amend the post with your other points

[bobzhang]

Note this is not really FFI.
It is an extension to the language, unlike FFI, the generated code is always sound, so users don't have to worry about its correctness.
The cool thing is this extension allows us to tweak the memory representation so that it could be used in the FFI, [@bs.optional] is bolted on later to make it appeal to the use case of config

[chenglou]

I guess at least you can merge default value and bs.optional? Don't want too many new tags.

I maintain that having a record there is very confusing for folks though. Especially since it's already confusing that records are nominally types and can't be found globally, etc. Folks expected their record value usage to magically "just work"

[chenglou]

More precision: default value in general is hard to support either way; currently the creation function is inlined (doesn't generate any function actually), so 100% free. Complex default values prevents this.

Record type version will use a bs.optional instead of magically read into e.g. option(int). This way a field with a type foo can still be made optional.

The justifications at #2680 (comment) are fine... I didn't consider the parametrized type's case much. So I guess I'm fine with the record version. But we do need a way to indicate that the record type is erased though; enough newcomers trip over "record type not found" already

[jchavarri]

After taking a look at bs.deriving abstract in master I have a few questions:

The goal is that we get some nice helpers, but also that it can generate platform-agnostic code. E.g. for JS backend, we can compile that abstract type to a JS object.

There are two conflating issues trying to be solved at once, but they are very different:

• underlying representation of records as JS objects
• helpers generation

Would it be possible to change the underlying representation of records to JS objects first (like it's being proposed in #2639), and then (or in parallel) figure out the helpers generation? The fact that the type is being erased has quite some consequences. Not only for the initial confusion in the short term that this proposal is trying to address, but also for the "syntactic losses" that are imposed on any Reason / OCaml code that interacts with these functions in the mid-long term: it will be forced to interact through functions.

The other question is about immutability. The setters in [@bs.deriving abstract] seem to only be generated if the properties are marked as mutable. For me, this is a deal breaker if [@bs.deriving abstract] is being introduced as a replacement of records for JS interop, as many performance techniques rely on immutability (like memoization or shouldComponentUpdate in React). Is it possible to generate setters even if the property is not marked as mutable? (so they generate a full new object).

---

For my case at hand (using Reason records to replace ImmutableJS records) it would be enough with generating immutable setters & getters on top of records for now. These "lenses" are already abstracting over the internal representation of the data, so the structure the record is being compiled to internally becomes a non-issue. Using arrays would be fine, as they could be accessed from JS through the functions.

[jchavarri]

I just realized [@bs.deriving accessors] can be applied to records as well as variants, which is kind of what I was proposing above. But it doesn't generate setters.

Aren't [@bs.deriving accessors] and [@bs.deriving abstract] trying to solve the same problem?

[chenglou]

I'd like records to compile to objects too, but we shouldn't do that first. We'd want these helpers generated anyway, so record-as-object shouldn't block this. When/if we do compile records to objects, this feature wouldn't break anyway (the generated type is abstract).

The record type removal makes sense (but yes, it's confusing; so we should find way to clarify this). We don't want you to directly create the record type. It's just reusing the syntax for its convenience and type checking. Think Swift structs and their init function, I guess. The whole point is that you'd create/access this one using a backend-agnostic data structure.

The setters in [@bs.deriving abstract] seem to only be generated if the properties are marked as mutable
Is this the case @bobzhang? If so, I think that should be changed... Though it's a little bit more cumbersome doing this through ppx? We'd need Object.assign. @jchavarri one thing worth noting is that currently these helpers are inlined; there's no runtime code generated!

bs.deriving accessors indeed has a confusing overlap. I've expressed that before. But its purpose is to generate a converter; this one's purpose isn't.

[jchavarri]

Thanks @chenglou. I think I understand better the difference now. [@bs.deriving abstract] is for those cases that require the underlying structure to be a JS object, and [@bs.deriving accessors] for those cases that don't need it necessarily, but in contrast could require it to be immutable. In the particular case I was mentioning above (replace immJS records), the second would be enough (specially if updaters were added) but I can imagine cases where the first would be useful too.

one thing worth noting is that currently these helpers are inlined; there's no runtime code generated

imo, that's even better. One can always opt-in to have that code added to the output by adding let high = high; let low = low.

[bobzhang]

hi @jchavarri for such type, it is quite trivial to have a functional update:

let udpate this ?(x= t |. x) ?(y= t |. y) () = 
   t ~x ~y 

It is not derived currently for some reasons: a) it is not as optimized as hand written code b) it bloat the code size for people who don't rely on it, our current solution is very clean, no extra code generated

[bobzhang]

ICYMI, the deriving works for mutual recursive types type .. and as well

[chenglou]

Postmortem: the record declaration is working out fine after a few hundred codemods