additionalProperties:false in Type.Intersect([Type.Record, ...])

[m-ronchi]

Hi, Type.Record(...) automatically add an additionalProperties: false to the schema, so this is wrong:

const Foo = Type.Record(Type.Number(), Type.String());
const Bar = Type.Intersect([Type.Object({ bar: Type.Number() }), Foo]);
const BC = TypeCompiler.Compile(Bar);
type Bar = Static<typeof Bar>;
const bar: Bar = { "42": "42", "bar": 42 }; // this is ok
console.log(Bar.allOf, BC.Check(bar)); // check fails

ouputs:

[
  {
    type: 'object',
    properties: { bar: [Object] },
    required: [ 'bar' ],
    [Symbol(TypeBox.Kind)]: 'Object'
  },
  {
    type: 'object',
    patternProperties: { '^(0|[1-9][0-9]*)$': [Object] },
    additionalProperties: false,
    [Symbol(TypeBox.Kind)]: 'Record'
  }
] false

ps: is additionalProperties on record necessary? also, I wish I could use Type.Composite on records

[sinclairzx81]

@m-ronchi Hi,

Type.Record(...) automatically add an additionalProperties: false to the schema, so this is wrong:

I can't remember the exact reason why additionalProperties: false was applied to Record types, but you're quite right to point out the constraint will break allOf intersections. Give me a couple of days to take another look at this, I think it makes sense to remove this constraint.

[sinclairzx81]

@m-ronchi Hey, so had a deeper look into this.

I have a PR waiting on https://github.com/sinclairzx81/typebox/pull/390 which implements changes to support Record intersection, however this is probably going to be a minor revision. The reason for the additionalProperties: false is due to Record types with Numeric keys needing the constraint to prevent additional non-numeric properties.

Consider the following example.

const R = Type.Record(Type.Number(), Type.Number())

Value.Check(R, { a: 'hello' }) // true????

This is resolved with the following.

const R = Type.Record(Type.Number(), Type.Number(), { additionalProperties: false })

Value.Check(R, { a: 'hello' }) // false

This is a difficult call to make. I do think this is the correct implementation (if interpreted through the lens of JSON Schema), but is somewhat surprising behavior if viewed from a TypeScript perspective. The JSON Schema rule book for schema composition mandates that "Schemas should not be constrained until final composition", so this PR will likely be the implementation going forward, but it is somewhat non-obvious that for numeric keys, one must also specify the additionalProperties: false to have a sound value assertion.

This may warrant a documentation update.

UnevaluatedProperties

The PR also includes some changes to Intersection to support Record composition. From your original example, consider the following which permits additional properties.

const R = Type.Record(Type.Number(), Type.String())
const O = Type.Object({ x: Type.Number() })
const I = Type.Intersect([O, R])
const C = TypeCompiler.Compile(I)

const bar = { 42: "42", x: 42, foo: 'unknown' } // foo is unknown
console.log(C.Check(bar)) // true

It is possible to use unevaluatedProperties as per Draft 2019-09 to constrain the intersect

const R = Type.Record(Type.Number(), Type.String())
const O = Type.Object({ x: Type.Number() })
const I = Type.Intersect([O, R], { unevaluatedProperties: false })
const C = TypeCompiler.Compile(I)

const bar = { 42: "42", x: 42, foo: 'unknown' } // foo is unknown
console.log(C.Check(bar)) // false

Will do a bit more testing on this tomorrow, but open to thoughts on the implementation. If you want to clone the project, the updates are on the record branch. Would be interested to get some feedback on these changes.

Cheers S

[m-ronchi]

Hi,

the actual type I am trying to build is something like

type Foo = {
  foo: string;
  bar: string;
  // ...orher fixed props
  [baz: `${string}_baz`]: Record<string, unknown>;
  [rest: string]: unknown
}

(I'm trying to parse data from a legacy source)

so I do actually want it like your first example.

I think that the biggest problem (in general) is the mismatch between json schema and typescript wrt. undeclared properties: ts forbids them, while jsonschema allows them by default. I don't know if it is possible to close the gap, e.g. by either adding additionalProperties: false to everything and removing it when composing types, or by adding { [x: string]: unknown } to object types and using HKT magic to remove it when composing

[sinclairzx81]

@m-ronchi I just pushed an update this afternoon to support the Foo type shown in your example (the additional property constraint was still being applied to record types with template literal keys). Updates are in the record branch.

The following should work for the reference type.

// type Foo = {
//   foo: string;
//   bar: string;
// } & {
//   [baz: `${string}_baz`]: Record<string, unknown>;
// } & {
//   [rest: string]: unknown
// }
const T1 = Type.Object({ 
  foo: Type.String(), 
  bar: Type.String()
})
const T2 = Type.Record(
  Type.TemplateLiteral([Type.String(), Type.Literal('_baz')]), 
  Type.Record(Type.String(), Type.Unknown())
)
const T3 = Type.Record(
  Type.String(), 
  Type.Unknown()
)
const Foo = Type.Intersect([T1, T2, T3], { unevaluatedProperties: true })

console.log(Value.Check(Foo, { foo: '', bar: '' }))            // true
console.log(Value.Check(Foo, { foo: '' }))                     // false - bar required
console.log(Value.Check(Foo, { foo: '', bar: '', x_baz: {} })) // true
console.log(Value.Check(Foo, { foo: '', bar: '', x_baz: 1 }))  // false - x_baz not record
console.log(Value.Check(Foo, { foo: '', bar: '', qux: 1 }))    // true

I think that the biggest problem (in general) is the mismatch between json schema and typescript wrt. undeclared properties: ts forbids them, while jsonschema allows them by default. I don't know if it is possible to close the gap, e.g. by either adding additionalProperties: false to everything and removing it when composing types, or by adding { [x: string]: unknown } to object types and using HKT magic to remove it when composing

It's not strictly true that TS forbids additional properties. TypeScript will run excess property checks for explicit type assignment, but in all other cases it uses structural (or polymorphic) assignment

interface T { x: number, y: number }

function test(value: T) {}

// ---------------------------------------------------
test({ x: 1, y: 2, z: 3 })           // not allowed

// ---------------------------------------------------
test({ x: 1, y: 2, z: 3 } as T)      // allowed

// ---------------------------------------------------
const A = { x: 1, y: 2, z: 3 } as const

test(A)                             // allowed
// ---------------------------------------------------
const B = { x: 1, y: 2, z: 3 } as T

test(B)                             // allowed

In this respect, the additionalProperties: false constraint is somewhat analogous to explicit type assignment.

const A: T = { x: 1, y: 2, z: 3 }    // additionalProperties: false

const B = { x: 1, y: 2, z: 3 } as T  // additionalProperties: true

It's an interesting one, where compositional aspects (intersect) cross over with language supported excess property checks, but I do think requiring the user to specify additionalProperties: false or unevaluatedProperties: false is the correct approach. I've tried to automatically apply excess property checks to object types in the past, unfortunately this lead to a lot of surprising behavior (as applying constraints for things users haven't specifically asked for does lead to issues, particularly as it changes the semantics of what the user has defined, as well as leading to a compositional dead end (even if you do factor for removing the constraints on things like intersect)).

Not keen to revisit automatic constraints in TypeBox given past issues. Moving forward, all constraints should be explicitly defined by the user. In all tho, I do think that TS and JSON Schema are more aligned than what might be immediately obvious, it's just that the ways of representing the same semantics differ between runtime and syntax supported expressions.

[sinclairzx81]

@m-ronchi Hey, I might need to defer rolling this PR through for a little bit. The removal of the additionalProperties constraint on Record types is a fairly broad breaking change (so will need to defer till the next minor release), plus I'm currently considering Indexed Access types on https://github.com/sinclairzx81/typebox/issues/391 as well. I think it might be best to ensure both features are aligned before pushing either one through.

In the interim, If you're using the TypeCompiler, you might be able to manually validate this structure using a custom type. This should let you validate the type, then swap to the updated Record types in a subsequent release.

import { TypeSystem } from '@sinclair/typebox/system'

const FooType = TypeSystem.Type<{
  foo: string;
  bar: string;
  [baz: `${string}_baz`]: Record<string, unknown>;
  [rest: string]: unknown
}>('Foo', (_, value) => {
  return true // code here to manually validate the value
})

type Foo = Static<typeof Foo>
const Foo = FooType()

const R1 = Value.Check(Foo, { /** data */ })
const R2 = TypeCompiler.Compile(Foo).Check({ /** data */ })

Will report back on this issue once I've had a deeper look into indexers.

[m-ronchi]

I am currently validating it manually (e.g. x => Outer.Check(x) && Object.entries(x).every(([k, v])=>/_foo$/.test(k) ? Inner.Check(v) : true)

meanwhile, I discovered TS is unsound: this code compiles without any errors...

[sinclairzx81]

@m-ronchi Hi.

I've removed the additionalProperties: false constraint on Record types and published on minor semver 0.28.0. The following should be possible (as per reference implementation above)

// type Foo = {
//   foo: string;
//   bar: string;
// } & {
//   [baz: `${string}_baz`]: Record<string, unknown>;
// } & {
//   [rest: string]: unknown
// }
const T1 = Type.Object({ 
  foo: Type.String(), 
  bar: Type.String()
})
const T2 = Type.Record(
  Type.TemplateLiteral([Type.String(), Type.Literal('_baz')]), 
  Type.Record(Type.String(), Type.Unknown())
)
const T3 = Type.Record(
  Type.String(), 
  Type.Unknown()
)
const Foo = Type.Intersect([T1, T2, T3], { unevaluatedProperties: true })

console.log(Value.Check(Foo, { foo: '', bar: '' }))            // true
console.log(Value.Check(Foo, { foo: '' }))                     // false - bar required
console.log(Value.Check(Foo, { foo: '', bar: '', x_baz: {} })) // true
console.log(Value.Check(Foo, { foo: '', bar: '', x_baz: 1 }))  // false - x_baz not record
console.log(Value.Check(Foo, { foo: '', bar: '', qux: 1 }))    // true

Other changes for 0.28.0 can be found on changelog located here https://github.com/sinclairzx81/typebox/blob/master/changelog/0.28.0.md

Will close off this issue for now, but feel free to ping on this thread if you're still running into issues. Can reopen and review.

Cheers! S