Support @decco.default for null fields

[mrmurphy]

It looks like [@deco.default "foo"] is designed only to work for fields that are entirely missing from the object. But it'd also be very nice to have that work for fields which are null or undefined.

[ryb73]

That makes sense. If a missing key can be defaulted to some value, why can't that behavior be extended to null and undefined? Why wouldn't it be?

On one hand, it seems safe to assume that a missing field and null are the same thing. But consider an API that takes multiple parameters, and say one of those parameters is an optional parameter called myMessage. The user can pass a string, they can pass in null, or they can omit the parameter altogether. Maybe if they omit the parameter we want the default message to be "Hello World". In this case the user can pass in null, but null isn't the default.

Translating that into reason/decco terms, that might look like:

[@decco]
type t = {
  [@decco.default Some("Hello World")] myMessage: option(string),
};
Js.Json.parseExn("{ myMessage: 'Hi' }") |> t_decode |> Js.log; // Some("Hi")
Js.Json.parseExn("{ myMessage: null }") |> t_decode |> Js.log; // None
Js.Json.parseExn("{}") |> t_decode |> Js.log; // Some("Hello World")

Changing decco.default would break that use case. Is that a use case that's important to decco users? I'm not sure.

There's another possible solution that's been floating around my head that would approach it in a more generic way. Instead of each type corresponding to a single codec, what if multiple codecs were allowed? (or at that point, would "middleware" describe it better?)

[@decco]
open Decco;
type myRecord = {
  field: [@decco.codec Codecs.default("my default")] [@decco.codec Codecs.string] string
  // or? // field: [@decco.middleware Middleware.default("my default")] string
};

In this example, the decoder would first pass the json through Middleware.default, which would either pass through the JSON as is, or would return "my default" if the value is null. (if the key is missing, then Js.Json.null is used as a default. this is the existing behavior today, which might be worth documenting)

This would solve the problem in a non-breaking way, eliminate the need for @decco.default, and add a lot more power and flexibility without changing the core underlying ideas. It also could be confusing and venturing into YAGNI territory. Do you have any thoughts?

[mrmurphy]

Interesting, so would there be a new type for decoding middleware? Essentially something like:

type decoderMiddleware = 'previous => Result.t('next, 'previous);

So the middleware can either decide to handle the JSON and return the expected type, or pass it along to some other middleware?

Would the middleware be composable? So as long as I had all the necessary decorators I could have a middleware chain like this: json -> a -> b -> c where the expected type is c?

It certainly does seem flexible, but is quickly sounding more complex than necessary. If we went down that path the lib might quickly lose the simple & approachable status that makes it so attractive right now.

Most of my use cases have actually been defined by just writing custom codecs when I need them. That's really pretty simple once the time is taken to figure it out. Maybe the need for middleware would be largely negated by making the custom codec process more user-friendly? Even if it's just by adding some docs.

I think there for sure are cases where the user wants to distinguish between a missing field and a null field, though I think they are a small minority. A key example would be an endpoint for updating a resource that leaves a field alone if the field isn't included, but erases the field from the database if the field is included and is null.

What if we just had two decorators that could be used either in isolation, or in combination?

• [@decco.onMissing "default"]
• [@decco.onNullUndefined "default"

I don't love how verbose this ends up being for what I'm considering to be the common case (a default for null, undefined, and missing). If my assumption is correct, there should probably be one a single easy-to-remember decorator for that, and two others for the minority cases.

I think my personal ideal would be changing default to cover all three cases, and add the other two decorators for the corner cases. But I don't love that users who are upgrading could have some surprise bugs if they aren't paying attention to that breaking change. The compiler won't help them. This could possibly be remedied by changing the name of default to fallback or something?

That's a long thought stream, what do you think?

[ryb73]

Yeah, I think you're right that it's getting more complex than necessary.

Maybe the need for middleware would be largely negated by making the custom codec process more user-friendly? Even if it's just by adding some docs.

I think that's a good thought. Do you have ideas for what changes might help (through docs or otherwise)?

[em]

I just want:

@decco.default("")
name: string,

to handle name: null, as "".

I really don't follow the explanation above. Today I get an error "message: "Not a string" which doesn't seem to make a lot of sense.

[em]

This still is a daily source of frustration for us and the cause of most of our runtime exceptions. Please make @decco.default also handle null. Especially considering that Decco already encodes None to null, but in rescript, None is represented as undefined and null must be handled through Js.Nullable.t. So Decco already considers undefined and null to be equivalent on encoding (because json lacks undefined), but is inconsistent with itself decoding by not treating them as equivalent.

In 99.99% of all cases you want them to be the same. And in the very rare case where you really needed null to express something else, you could write a customer decoder.

[ryb73]

I think you've mostly convinced me that this is worth doing. Unfortunately I'm no longer using Reason so Decco is not really being actively maintained. I'm open to accepting PRs. Sorry for the inconvenience.

[danielo515]

This still is a daily source of frustration for us and the cause of most of our runtime exceptions. Please make @decco.default also handle null. Especially considering that Decco already encodes None to null, but in rescript, None is represented as undefined and null must be handled through Js.Nullable.t. So Decco already considers undefined and null to be equivalent on encoding (because json lacks undefined), but is inconsistent with itself decoding by not treating them as equivalent.

In 99.99% of all cases you want them to be the same. And in the very rare case where you really needed null to express something else, you could write a customer decoder.

Decco is wrongly encoding option as null. It should be undefined, there is no way it makes sense to be encoded as null, for that there is Nullable. Option should be encoded as undefined.
Is there a chance to patch this?