lukeed/tschema

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 change
  • t.strict() -> is t.object w/ additionalProperties: false hardcoded
  • t.loose() -> is t.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" }
//->   }
//-> }