treatment of omission of optional props

[harrysolovay]

How might we address cases such as the following?

interface SomeType {
  a: string
  b?: string
}
const $someType: $.Codec<SomeType> = $.object(
//    ^
//    Type 'SomeType' is not assignable to type '{ a: string; b: string | undefined; }'
  ["a", $.str],
  ["b", $.option($.str)],
)

[ryanleecode]

If we take the Expand type and make it [K in keyof T]?: T[K] from [K in keyof T]: T[K], this would make all fields omitable. So likely some manipulation of this type with a conditional would make this possible. And a new codec field "omitable".

https://github.com/paritytech/scale-ts/blob/1c2b3544ae90a67465fc5677fb46bd6279a423f4/common/util.ts#L31

[tjjfvi]

Oh, didn't realize I hadn't responded to this earlier.

This change would be in NativeObject, not Expand; off the top of my head this would probably look something like

export type NativeObject<O extends AnyField[]> = Expand<
  U2I<
    | {}
    | {
      [K in keyof O]: undefined extends Native<O[K][1]> ? Partial<Record<O[K][0], Native<O[K][1]>>> : Record<O[K][0], Native<O[K][1]>>
    }[number]
  >
>

This will just add ? to the fields whose value includes undefined. In theory it could be worthwhile to be able to distinguish between x: T | undefined, x?: T, and x?: T | undefined (the caveat being that the latter two are only distinct under exactOptionalPropertyTypes) but I think this adds unwarranted complexity and that it's best to only support x?: T | undefined.

[tjjfvi]

Actually, the above could be inconvenient for generic functions.

[tjjfvi]

Then again, I don't know that that is a very common use case. Let's go with this for now and we can revise if needed.

[tjjfvi]

Ah, exactOptionalPropertyTypes isn't supported in Deno. So we only need to potentially distinguish between x: T | undefined and x?: T | undefined.

The other question is how this should work:

const $foo = $.object(["val", $.option($.u8)])
$foo.decode($foo.encode({}))
$foo.decode($foo.encode({ val: undefined }))

These encode identically, so only one of them can roundtrip. Should it decode to {} or { val: undefined }? I lean towards the latter, as it's more explicit, and it simplifies the decoding logic.

[tjjfvi]

Perhaps it would actually make sense to pivot a bit:

Add a $.field:

const $foo = $.field("val", $.u8)
$foo // Codec<{ val: number }>

Rename $.spread to $.object and make it variadic:

const $bar = $.object(
  $.field("a", $.u8),
  $.field("b", $.str),
)
$bar // Codec<{ a: number, b: string }>

const $baz = $.object(
  $bar,
  $.field("c", $.bool)
)
$baz // Codec<{ a: number, b: string, c: boolean }>

const $qux = $.object(
  $foo,
  $.taggedUnion("type", [
    ["xy", $.field("x", $.u8), $.field("y", $.u8)],
    ["abc", $baz],
  ])
)
$qux
// Codec<
//   | { type: "xy", val: number, x: number, y: number }
//   | { type: "abc", val: number, a: number, b: string, c: boolean }
// >

Then:

const $foo = $.object($.field("val", $.option($.u8)))
$foo // Codec<{ val: number | undefined }>
const $bar = $.object($.option($.field("val", $.u8)))
$bar // Codec<{ val?: number }>

[ryanleecode]

That looks good i like it. Re: decode. i agree with decoding the fields as undefined rather than omitting the fields since that would be consistent with how its decoded in rust as field: None.

The only reason to omit would be if the user wants to decode it as JSON and then send that JSON (as is) over a network request which would create additonal bandwidth requirements cuz it would be bigger.... but im pretty sure nobody is doing this.

[tjjfvi]

JSON encodes { foo: undefined } as {}, so this wouldn't be a problem.

With the $.option($.field(...)) method, I was actually thinking of decoding as {} instead, as this aligns with { ...undefined } and { ...{ foo: 123 } }. If people want the other behavior ({ foo: undefined }), they can use $.field(..., $.option(...)) instead.

[ryanleecode]

the flow i envisioned is something like this

1. user decodes SCALE payload into JSON
2. user attaches this JSON decoded payload to his own JSON as a nested field.
3. user sends this payload over HTTPs as JSON format (maybe to another API that only accepts JSON)

In this scenario the nested JSON would have these unnecessary fields and if this flow comes from a client side application then it would use more bandwidth.

however i don't think this is our responsibility anyways, just something i thought of.

[harrysolovay]

• The addition of $.field and rename of $.spread to $.object feels like the right path
• I'd also air on the side of decoding into {} instead of { key: undefined }, but not if it complicates the decoding impl
• Is $.option the right codec for the job? Perhaps $.field should have a 3rd (optional) param. If true is applied to this third param, the field would be omit-able (maybe separate codecs are merited, ie. $.field and $.optionalField). $.undefOr might make more sense as a name for $.option... I'll admit, $.undefOr is mildly unattractive... but it's a little more JS-oriented.

Whatever the right re-work––ideally it doesn't involve boiling in more conditional checks / logic to the already-somewhat-complex mapping to native types.

[tjjfvi]

I'd also air on the side of decoding into {} instead of { key: undefined }, but not if it complicates the decoding impl

Great; with this new approach, it doesn't complicate it.

Is $.option the right codec for the job?

I think so – given that $.object spreads the members, and { ...undefined } is {}, this behavior falls out naturally.

$.undefOr might make more sense as a name for $.option

I think $.option is fine. Sure, it's not fully a term in JS, but "optional" for | undefined is pretty well-established, and I don't think

type Option<T> = T | undefined

is very foreign, either.

[tjjfvi]

In this scenario the nested JSON would have these unnecessary fields

@ryanleecode No matter what, undefined properties in JSON are simply omitted:

JSON.stringify({ a: { b: 1, c: undefined }, d: undefined })
// '{"a":{"b":1}}'

So this isn't an issue.

And scale-decoded values aren't really designed to be json-compatible without additional transformation anyway.

[ryanleecode]

Omg ur right haha. Yah it was just a contrived thought experiment. Nobody would use it in this way.

[tjjfvi]

I think so – given that $.object spreads the members, and { ...undefined } is {}, this behavior falls out naturally.

Oh, this falls out naturally for decoding, but not for encoding. I think I'll move to $.optionalField.