Optional type encodes to option<Js.Json.t> instead of Js.Json.t

[reebalazs]

I see this on 0.2.2. This has changed recently, breaking my existing code.

@spice
type mystring = option<string>

let encoding = (str): Js.Json.t => {
  str->mystring_encode
}

This code gives an error:

  16 │
  17 │ let encoding = (str): Js.Json.t => {
  18 │   str->mystring_encode
  19 │ }
  20 │

  This has type: option<Js.Json.t>
  But it's expected to have type: Js.Json.t (defined as Js_json.t)

So for some reason the payload type becomes option<Js.Json.t> if the item itself is optional. Shouldn't the encode result always be just Js.Json.t?

At the same time this works, so the problem is only with the first-level option.

@spice
type mystring = {content: option<string>}

let encoding = (str): Js.Json.t => {
  str->mystring_encode
}

I'll test this eventually with the newest code. (I am on 0.2.2 currently because Rescript v11 does not run with 0.2.3.)

[mununki]

I have a few questions.

1. was this code built in a version prior to v0.2.2?

@spice
type mystring = option<string>

let encoding = (str): Js.Json.t => {
  str->mystring_encode
}

2. The code below builds well and seems to be a correct representation of encoding to a value of type mystring. Why do you think the mystring_encode function should return a value of type Js.Json.t instead of `option<Js.Json.t> as the encoding result? If the value of mystring is None, what value do you think the encoding function returns?

@spice
type mystring = option<string>

let encoding = (str): option<Js.Json.t> => {
  str->mystring_encode
}

[reebalazs]

Correct, the code was working prior to v0.2.2.

I now understand that this change was introduced as part of the recent fix to support null and undefined. Since Js.Json cannot directly encode undefined, encoding to option<Js.Json.t> is a reasonable approach.

However, in this specific case, the correct solution would be for the encode function to always return option<Js.Json.t>, regardless of whether the input type is an option or not.

At the moment, the function’s signature varies based on the input type. For an option, the signature is:

type encoder<'a> = 'a => option<Js.Json.t>

But for non-option types, the signature is:

type encoder<'a> = 'a => Js.Json.t

This inconsistency makes it difficult to use the encode function in a generic way. The function’s output type should remain consistent and always return (if we go this way) option<Js.Json.t>, irrespective of the input type 'a.

My use case: I’m building an encoder/decoder for my application’s network API. This ensures the protocol data is correct and raises a serialization error if it isn’t. This approach is much more robust than simply casting data to the expected type. However, for the encoder/decoder to work generically with all types (including option<'a>), the function signatures for encoding and decoding must be consistent and independent of the type being converted. Right now option types break my existing code, so I work around it by not using option types.

I don’t know OCaml well enough to implement this change myself, but I believe the ideal fix would ensure that non-option types are also encoded as option<Js.Json.t>. This way, the encode/decode function signatures remain invariant regardless of the input type, which then could also be updated in Spice.res:

type encoder<'a> = 'a => option<Js.Json.t>

To maintain symmetry, the decoder would be:

type decoder<'a> = option<Js.Json.t> => result<'a>

This change would break compatibility for some applications, but migration would be simple. It would also ensure consistency, unlike the current state, which only introduces breaking changes for option types.

An alternative solution would be to revert the support for undefined and treat it the same as null. In my opinion, this approach could also work well for most use cases.

[mununki]

It makes sense to me. How about the signatures of encoder/decoder functions:

type encoder<'a> = 'a => result<Js.Json.t, Spice.encodeError>

type decoder<'a> = Js.Json.t => result<'a, Spice.decodeError>

[reebalazs]

That would also be good, if I understand correctly.

So this means that encoding undefined would give Spice.encodeError, and encoding anything else would give a Js.Json.t (with no option)?

This would actually be much simpler and better looking then changing Js.Json.t to option<Js.Json.t> everywhere, as I described above.

Can you please clarify and elaborate, if this is what you meant, or something else?

[mununki]

So this means that encoding undefined would give Spice.encodeError, and encoding anything else would give a Js.Json.t (with no option)?

Exactly, yes.

[reebalazs]

So this means that encoding undefined would give Spice.encodeError, and encoding anything else would give a Js.Json.t (with no option)?

Exactly, yes.

I agree this would be the best solution.

[reebalazs]

Something came into my mind. This is important to keep options usable.

So this, as we discussed above, resolves to encodeError:

@spice
type t = option<string>

let str = None
switch t_encode(v) {
  | Ok(v) => Js.log(t_encode(json)->Js.Json.stringify)  
  | Error(e) => Js.log("No json representation")
  },
  // => "No json representation"   

But, if option is in a record, what would it give? It should not give encodeError but it should convert to something, right?

@spice
type t = {
  foo: string,
  value: option<string>
}

let v = { foo: "bar", value: None} 
switch t_encode(v) {
  | Ok(v) => Js.log(t_encode(json)->Js.Json.stringify)  
  | Error(e) => Js.log("No json representation")
  },
  // => {"foo": "bar"}      

Same with optional fields:

@spice
type t = {
  foo: string,
  value?: string
}

let v = {foo: "bar"} 
switch t_encode(v) {
  | Ok(v) => Js.log(t_encode(json)->Js.Json.stringify)  
  | Error(e) => Js.log("No json representation")
  },
  // => {"foo": "bar"}      

I would assume that in both cases they end up to be valid json, with the optional field simply missing from the record? Am I correct with what these examples would log? Iow the record containing options would encode just as currently does.