tjjfvi/subshape

$assert on value that is null

davit-b opened this issue · 3 comments

davit-b commented

Code

const breaking_object = {
  author: "acqq",
  num_comments: null, // this value === number | null
  objectID: "12343",
  points: 34,
  title: null,  // this value === string | null
  url: null, // this value === string | null
}

const $shape = $.object(
  $.field("author", $.str),
  
  $.optionalField("num_comments", $.u32), 
  // optionalField doesn't work for the null input. I tried a literal union here 
  // but it gave me an error `typeof value.num_comments !== "string"` 
  // maybe the literal union only allows string literals? 
  
  $.field("objectID", $.str),
  $.field("points", $.u32),
  
  // literalUnions with null constant does not work for the null inputs above. 
  $.optionalField("title", $.literalUnion([$.str, $.constant(null)])), 
  $.optionalField("url", $.literalUnion([$.str, $.constant(null)])), 
)

$.assert($shape, breaking_object)

Error

typeof value.num_comments !== "number" 
  ​​​​​at ​​​​​​​​AssertState.typeof​​​ ​./node_modules/scale-codec/src/common/assert.ts:25​
  ​​​​​at ​​​​​​​​Codec._assert​​​ ​./node_modules/scale-codec/src/codecs/int.ts:37​
  ​​​​​at ​​​​​​​​Codec._assert​​​ ​./node_modules/scale-codec/src/codecs/option.ts:34​
  ​​​​​at ​​​​​​​​Codec._assert​​​ ​./node_modules/scale-codec/src/codecs/object.ts:40​
  ​​​​​at ​​​​​​​​Codec._assert​​​ ​./node_modules/scale-codec/src/codecs/object.ts:79​
  ​​​​​at ​​​​​​​​Object.assert​​​ ​./node_modules/scale-codec/src/common/codec.ts:140​
  ​​​​​at ​​​​​​​​test​​​ ​src/utility/play.ts:33:3​
  ​​​​​at ​​​​​​​​Object.<anonymous>​​​ ​src/utility/play.ts:36:1

typeof value.title !== "string" 
  ​​​​​at ​​​​​​​​AssertState.typeof​​​ ​./node_modules/scale-codec/src/common/assert.ts:25​
  ​​​​​at ​​​​​​​​Codec._assert​​​ ​./node_modules/scale-codec/src/codecs/union.ts:87​
  ​​​​​at ​​​​​​​​Codec._assert​​​ ​./node_modules/scale-codec/src/codecs/option.ts:34​
  ​​​​​at ​​​​​​​​Codec._assert​​​ ​./node_modules/scale-codec/src/codecs/object.ts:40​
  ​​​​​at ​​​​​​​​Codec._assert​​​ ​./node_modules/scale-codec/src/codecs/object.ts:79​
  ​​​​​at ​​​​​​​​Object.assert​​​ ​./node_modules/scale-codec/src/common/codec.ts:140​
  ​​​​​at ​​​​​​​​test​​​ ​src/utility/play.ts:33:3​
  ​​​​​at ​​​​​​​​Object.<anonymous>​​​ ​src/utility/play.ts:36:1
  
typeof value.url !== "string" 
  ​​​​​at ​​​​​​​​AssertState.typeof​​​ ​./node_modules/scale-codec/src/common/assert.ts:25​
  ​​​​​at ​​​​​​​​Codec._assert​​​ ​./node_modules/scale-codec/src/codecs/union.ts:87​
  ​​​​​at ​​​​​​​​Codec._assert​​​ ​./node_modules/scale-codec/src/codecs/option.ts:34​
  ​​​​​at ​​​​​​​​Codec._assert​​​ ​./node_modules/scale-codec/src/codecs/object.ts:40​
  ​​​​​at ​​​​​​​​Codec._assert​​​ ​./node_modules/scale-codec/src/codecs/object.ts:79​
  ​​​​​at ​​​​​​​​Object.assert​​​ ​./node_modules/scale-codec/src/common/codec.ts:140​
  ​​​​​at ​​​​​​​​test​​​ ​src/utility/play.ts:33:3​
  ​​​​​at ​​​​​​​​Object.<anonymous>​​​ ​src/utility/play.ts:36:1​

I'm not sure what I'm doing wrong here.
The literalUnion does not work for null fields, and the optionalField doesn't work for the null input.

Do I need some new function like zod's Nullable?

Would appreciate any guidance :)

tjjfvi commented

Well, there are two things going on here.

$.literalUnion is supposed to work for non-string values (as of #137), but it seems I failed to remove the typeof === "string" assertion.

However, even if it was fixed, it wouldn't exactly work for your usecase, as $.literalUnion([$.str, $.constant(null)]) isn't a $.Codec<string | null>, but a $.Codec<$.Codec<string> | $.Codec<null>>.

We don't have a built-in codec for T | null – we prefer to instead use T | undefined with $.option (as you're doing with $.optionalField), but you can write a custom one:

export function $nullable<T>($inner: Codec<T>): Codec<T | null> {
  return createCodec({
    _metadata: metadata("$nullable", $nullable, $inner),
    _staticSize: 1 + $inner._staticSize,
    _encode(buffer, value) {
      if (value === null) {
        buffer.array[buffer.index++] = 0
      } else {
        buffer.array[buffer.index++] = 1
        $inner._encode(buffer, value)
      }
    },
    _decode(buffer) {
      return buffer.array[buffer.index++] ? $inner._decode(buffer, value) : null
    }
    _assert(assert) {
      if (assert.value !== null) {
        $inner._assert(assert)
      }
    },
  })
}

It's worth noting, however, that if you write $.optionalField("foo", $nullable($bar)) it will encode two discriminant bytes (the first for if foo is defined, and the second for if it's non-null). If you're using this codec for your own purposes (i.e., you don't care about the exact format), this is fine, but if you're trying to encode/decode data to another implementation of scale, this kind of codec probably isn't what you want. You'll either want $.optionalField("foo", $bar) or $.field("foo", $nullable($bar)), which will both only encode one discriminant byte (and also only support one way to represent None).

  • 🚀1
davit-b commented

encode two discriminant bytes (the first for if foo is defined, and the second for if it's non-null).

So I should determined how the breaking object represents None, and decide from there.

  • If None is represented as defined | undefined --> use $.optionalField("foo", $bar) [uses 1 byte discriminant]
  • If None is represented as non-null | null --> use $.field("foo", $nullable($bar)) [uses 1 byte discriminant]
  • If it's represented both ways, then I'm forced to use $.optionalField("foo", $nullable($bar)) [uses 2 bytes for 2x discriminants]
    It's just a waste because it will never be undefined && null.

I just tested all 3 cases and the above $nullable works. :) :) Love this lib.

Btw, I had to remove the value field from the following to get it working.

_decode(buffer) {
  return buffer.array[buffer.index++] ? $inner._decode(buffer) : null
},

I looked through https://github.com/paritytech/scale-ts/tree/main/codecs in case the other Codec's took a value argument on the _decode function and couldn't find any, so I assume it shouldn't be there.

  • ❤️1
tjjfvi commented

So I should determined how the breaking object represents None, and decide from there.

Exactly!

I just tested all 3 cases and the above $nullable works. :) :) Love this lib.

Glad you're enjoying it :)

Btw, I had to remove the value field from the following to get it working.

Ah, yes, you're right. I suppose I should've tested (or at least type-checked) the code before I posted it ;)

  • 👍1