Missing ability to strip "undefined" properties from Object

[matthieusieben]

I'm trying to encode objects which may contain undefined values into DAG-CBOR.

The approach I tried was to copy the Object implementation but there is an issue: objectToTokens and sortMapEntries are not exported by the module, so we basically have to re-implement the whole encoder logic, which is not really suitable. Note that we could also mutate the object in place (and return null to let the default handler do the job) but I don't want to be altering user provided data as this could lead to unexpected behaviors.

Would it be possible to either:

• Add a dedicated option (like stripUndefinedObjectValues or filterObjectEntries: (key, value) => boolean) ?
• Expose objectToTokens and sortMapEntries as part of the public API of the package ?
• Have the ability to return some kind of "ignore" token from custom type encoders (e.g. undefined: () => Token.ignore) and have the default Object encoder skip these entries ?
• Use a special Error that can be thrown when encoding nested values, resulting in them being ignored ?
• Other ?

The use case for us (at @bluesky-social ) is to be able to encode values generated using typescript code, into DAG_CBOR. TS allows to assign undefined to optional properties, requiring us to manually pre-process object in order to strip undefined values.

I'd basically want to be able to do:

type Foo = { bar ?: string }
const foo: Foo = {
  bar: undefined // "legal" in typescript
}

const options = {
  typeEncoders: {
    undefined: () => {
      throw new Error('Undefined is not allowed in DAG-CBOR')
    }
  }
}

// I want this to silently ignore the undefined property somehow.
const encoded = encode(foo, options)

// Works but is quite slow
encode(stripUndefineds(foo), options)

// These should still result in an error
encode(undefined, options)
encode([undefined], options)
encode({x:[undefined]}, options)

[matthieusieben]

Also note: this should probably only apply to Objects and not Maps as, with Maps, the presence of any value (including undefined) is typically deliberate.

[rvagg]

OK, to be clear - you want to ignore undefined values, not just reject them? Intercepting the token stream seems like a logical option, but that'd require you getting pretty dirty. I think that it's a common enough bit of functionality that we could just add an option to do that. Probably just an ignoreUndefinedValues that's documented as a boolean that will skip over any object property whose value is undefined, defaults to false.

I'm concerned about your claim about Maps being a special case though - I don't really see the difference, key presence but undefined value could equally be used as a sentinel for both Object and Map. Maybe it's more probable in the case of a Map but consider the relatively common Object -> Map conversion which brings us right back to the ambiguity you're trying to deal with:

> const obj = { yes: 'no', undef: undefined }
> const map = new Map(Object.entries(obj))
> console.log(map)
Map(2) { 'yes' => 'no', 'undef' => undefined }

Is this key important? Can we know? This is the kind of ambiguity that makes us throw in the canonical dag-cbor encoders - sort out your data yourself, make your intention clear, don't make us guess cause there's a good chance we guess wrong.

So personally I'd recommend against this, but if you feel strongly enough that your developer-base is likely to be using undefined as a kind of Set-sentinel then you might need a second option just so we don't end up with internal inconsistency. But then you also have the problem of dealing with the fact that undefined is not valid DAG-CBOR, so are you going to convert them to null? Arguably that, or '', or some other cheap sentinel is the right way to represent a Set with a Map rather than the ambiguous undefined.

[matthieusieben]

My goal here is really only to tackle undefined properties in plain object, which are only made possible because of the way Typescript works.

Typescript, when exactOptionalPropertyTypes is disabled, will allow to assign undefined as property value when a property is defined as optional:

// OK
const foo: { bar?: number } = { bar: undefined }

That is not the case for Map and Set. With these data structures, the presence of an undefined value is usually a deliberate choice, which is why I think that for these (as for Arrays), the typeEncoders.undefined should still be used.

const map = new Map<string, number>()
map.set('foo', undefined) // Not allowed

delcare const someMap: Map<unknown, unknown>
for (const [key, value] of someMap) {
  if (value === undefined) {
    // "undefined" was deliberately added to the map
  }
}

I agree that there are some cases (as with your Object.entries() example) through which undefined values may end-up being added to the Map, but, since that operation happens before calling cborg, I don't think it's the responsibility of the lib to handle that particular use case, and it's fine for cborg to only handle caveats directly introduced by Typescript, with plain objects.

In our particular use case, the presence of keys with undefined values in plain objects (only) is not important (same semantical meaning as with optional TS properties). Also, we do not use cborg with Set or Map so we don't need or want sentinel values. Because of that, I have no strong opinion on the effect that new option would have wrt Sets or Maps. I just explained my POV above to give you more context.

[matthieusieben]

Here is what I propose: #155