Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opt In Additional Properties #77

Merged
merged 4 commits into from
Jun 26, 2021
Merged

Opt In Additional Properties #77

merged 4 commits into from
Jun 26, 2021

Conversation

sinclairzx81
Copy link
Owner

This PR introduces a potential breaking change with respect to additionalProperties handling in TypeBox.

History 0.16.x

As of 0.16.x, an update was introduced to enforce additionalProperties: false for all Type.Object({...}) and Type.Intersect({ ... }) types. The reasoning behind this was that the default data validation over the wire would probably want to disallow unknown properties appearing on the objects validated with TypeBox. From TypeBox's standpoint, this update initial appeared to have minimal implication with respect to compositing and inferring types.

Problem

As of the 0.16.x update, It became clear that the default additionalProperties: false constraint on object types resulted in a number of unforeseen compositional issues, specifically around Type.Intersect({ ... }) handling in TypeBox. Consider the following.

const A = Type.Dict(Type.Number())  // additionalProperties: { type: 'number' }
const B = Type.Object({ data: Type.String() }) // additionalProperties: false
const C = Type.Intersect([A, B]) // additionalProperties ???

// expect C to be
type C = {
   [key: string]: number,
   data: string
}

Because B has additionalProperties: false, this raised immediate problems for Type.Dict('...'), causing quite a bit of ambiguity with respect to what TypeBox should actually do in these cases. For example, should B all of a sudden allow additionalProperties: { type: number } even though the default is to disallow additionalProperties? The specific issue raised can be read about here #60

In addition, the internal JSON schema representation of intersected type C also raised some questions, specifically around "What is the best way to express an inferable schema for C with additionalProperties:false constraints".

This lead to a lengthy discussion with one of the JSON schema engineers (which can be read about here json-schema-org/json-schema-spec#1087). The main take away from this discussion was that the decision to mandate on additionalProperties: false is ultimately a violation of JSON schema architectural principles which has some fairly nasty unforeseen downstream consequences with respect to how JSON schema composition can be leveraged (as above).

Update 0.17.x

As of 0.17.x, as a step towards more sophisticated composition, TypeBox no longer applies additionalProperties: false to Type.Object({ ... }) schemas. This change will require users to explicitly disallow unknown properties by passing additionalProperties: false on all top level schemas expected to be received over the wire.

const T = Type.Object({
   a: Type.String(),
   b: Type.String()
}, { additionalProperties: false })

As for Type.Dict({}) intersections, TypeBox that may leverage unevaluatedProperties in draft 2019-9 at a later time.

@sinclairzx81 sinclairzx81 merged commit cff3ccd into master Jun 26, 2021
@sinclairzx81 sinclairzx81 deleted the additionalProperties branch July 20, 2021 17:00
nktpro pushed a commit to shopstic/typebox that referenced this pull request Dec 31, 2021
* remove additionalProperties:false on object and intersect

* replace typescript-bundle for hammer | esbuild

* minor version tick

* rename docs for media
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant