sinclairzx81 / typebox

Json Schema Type Builder with Static Type Resolution for TypeScript
Other
4.79k stars 153 forks source link

Casting `uniqueItems` array with object items #736

Closed Bubz43 closed 8 months ago

Bubz43 commented 8 months ago

The current behavior as of 0.32.10 is:

  const T = Type.Array(Type.Object({ value: Type.String() }), {
    uniqueItems: true,
    default: [],
  });
  Value.Cast(T, [{ value: 'foo' }, { value: 'foo' }]); // Throws ValueCastError

This is because it tries to make the items unique using const unique = [...new Set(casted)] which won't do anything for object values.

There is the hash function now, which Check uses itself, so could use that:

const unique = [...new Map(casted.map((item) => [Hash(item), item])).values()]
sinclairzx81 commented 8 months ago

@Bubz43 Hi! Thanks for the suggestion (and apologies for the delay in reply)

Ah, there's not too much I could implement here. I think hash would work for unique arrays of string (and I suppose bigint arrays), but wouldn't generalize out to other types (which would be the preference). Note that TypeBox has quite a few "non-instantiable" types; and the library has essentially conceded that some types just cannot be instantiated in a reasonable and predictable way....

While this usually occurs when a type has a data constraint applied (like uniqueItems), it can also apply to illogical intersection (i.e. string & number) or non-constrained (or constrained) infinite recursive types ...

// ValueCreateError: Array with the uniqueItems constraint requires a default value.
const T = Type.Array(Type.Object({ value: Type.String() }), {
  uniqueItems: true,
  minItems: 2 // N + 2
});

// ValueCreateError: Intersect produced invalid value. Consider using a default value.
const T = Type.Intersect([Type.String(), Type.Number()])

// ValueCreateError: Cannot create recursive type as it appears possibly infinite. Consider using a default.
const T = Type.Recursive(This => Type.Object({
  items: Type.Array(This, { minItems: 1 }) // 1 item explodes
}))

I think in the uniqueItems case, It might be possible to update the assertion logic to check minItems is either length 0 or 1 (if specified). But for now, I am quite keen to keep the defacto default in there as it keeps the logic simple and predictable (at least for now, but will investigate following a couple of pending updates I want to take a look at on Create)

Will add a consideration tag and pick this up at a later time, but will close off for now. Thanks again! S

Bubz43 commented 7 months ago

Hey thanks for the response, but I think my post was vague in what I meant.

My suggestion is using Value.Hash to dedupe the unique array, not to create anything. The current behavior in https://github.com/sinclairzx81/typebox/blob/master/src/value/cast/cast.ts#L114 relies on using a Set to dedupe the array, which won't work with objects.

Having the default be required is fine and not a problem.