additionalProperties
Closed this issue ยท 9 comments
By default, JSON schema allows additional properties; see docs
Currently tschema
allows/follows this. But this means that as a user, I'm writing t.object({ ... }, { additionalProperties: false })
quite often.
Option 1
So I'm wondering if tschema
should explicitly set additionalProperties: false
by default, which would mean that you have to explicitly allow additional properties (aka non-defined properties)... This would (already, lol) be a breaking change because it breaks/deviates from JSON schema (because of the inverse default).
Option 2
Or, I can add t.strict()
which is a common naming convention for this. This would still product t.object
types and the only place that additionalProperties: false
is automatically set.
Essentially this:
export function strict(props, options) {
return object(props, { ...options, additionalProperties: false });
}
// Current:
// ---
t.object({
name: t.string(),
}, {
additionalProperties: false
})
//-> {
//-> type: "object",
//-> additionalProperties: false,
//-> required: ['name'],
//-> properties: {
//-> name: { type: "string" }
//-> }
//-> }
// Proposed: Option 1
// ---
t.object({
name: t.string(),
})
//-> {
//-> type: "object",
//-> additionalProperties: false,
//-> required: ['name'],
//-> properties: {
//-> name: { type: "string" }
//-> }
//-> }
t.object({
name: t.string(),
}, {
additionalProperties: true
})
//-> {
//-> type: "object",
//-> additionalProperties: true,
//-> required: ['name'],
//-> properties: {
//-> name: { type: "string" }
//-> }
//-> }
// Proposed: Option 2
// ---
t.strict({
name: t.string(),
})
//-> {
//-> type: "object",
//-> additionalProperties: false,
//-> required: ['name'],
//-> properties: {
//-> name: { type: "string" }
//-> }
//-> }
in option 2, strict
is specifically for objects?
could it not be better to have a strict mode globally, like how typescript, node and others do?
i.e.
import * as tschema from 'tschema/strict.js';
// or
tschema.strict().object({...});
currently it only applies to objects, but maybe there are other types with similar decisions to be made.
maybe this is nonsense though and we'll only ever run into objects needing extra strictness. if that's the case, ignore me :D
Global modifiers are bad because they are leaky and this utility is primarily going to be used in high-traffic / high-value path. It would be very unfavorable if you set up all your schemas only to have something async/later come by and globally flip a switch, dramatically changing all your schemas' meanings.
I'm really not a fan of chaining methods. We (collectively, thankfully) moved away from this in the chalk
evolution.
But yes, hypothetically t.strict()
would only apply to objects. This is also the case/assumed with prior art like zod, superstruct, etc
in that case, maybe option 2 makes the most sense but naming it something to be clear it is a layer on top of object
?
strict
isn't obvious to me as being a strict version of object
fair point on the global modifier etc, too. was mostly a thought because thats what many others did
FWIW I'm leaning toward making additionalProperties: false
by default, making this a breaking change & thus a 3.0
release.
TBH I meant to do this for the first release... just forgot to add it to my final checklist. All my prior usage needs additionalProperties: false
and its hard to imagine that people would want to allow additionals by default.
+1 to additionalProperties: false
by default, but maybe add a convenience method for allowing them (t.allowAdditional
, t.passthrough
etc) for codebases that go that route
additionalProperties: xxx
everywhere is awkward both ways
the inverse would be t.loose()
imo.
I guess that could mean that there's a 3rd option:
t.object()
-> stays the same, bare/no changet.strict()
-> ist.object
w/additionalProperties: false
hardcodedt.loose()
-> ist.object
w/additionalProperties: true
hardcoded
Would strictObject()
and looseObject()
be too verbose? Feels like a happy medium between clarity and brevity. I do agree with @43081j that strict()
itself doesn't immediately jump out at me as being object only, and makes me think it's a chained modifier for whatever the next type call is.
Yes. I don't particularly like t.strict
or t.loose
either, but it's what I've seen/grown accustomed to over the years.
I'm leaning towards a 3.0 breaking release, where additionalProperties: false
is the default output. I can't think of a situation where you'd actually want additionalProperties: true
to happen, especially w/ a TS-inferred counterpart. The only thing I can think of is NodeJS headers, where they define a known headers and then let you add custom keys:
type OutgoingHttpHeaders = {
'accept'?: string | string[];
'accept-charset'?: string | string[];
// ...
[key: string]?: string | string[];
};
While this is technically & capably true of any JS object, it negates the point of object schemas.. or at least the (most likely) default case.
With this in mind, I think it's okay to force users to type out additionalProperties: true
the relatively fewer times it's needed.
This breaking change will also be aligned with #7, which needs some rework to properly "lock in" a TS/JS meaning of tuples
Final bit:
Will only set additionalProperties: false
if/when properties are actually defined.
This is because t.string() //-> { type: "string" }
, for example, accepts any string. So a blank { type: "object" }
should accept any object. We only only want to restrict to the defined properties if we have properties defined.
// As of 3.0
// ---
t.object();
//-> { type: "object" }
t.object({
name: t.string(),
});
//-> {
//-> type: "object",
//-> additionalProperties: false,
//-> required: ["name"],
//-> properties: {
//-> name: { type: "string" }
//-> }
//-> }
// Allow extra properties (for some reason)
t.object({
name: t.string(),
}, {
additionalProperties: true,
});
//-> {
//-> type: "object",
//-> additionalProperties: true,
//-> required: ["name"],
//-> properties: {
//-> name: { type: "string" }
//-> }
//-> }