sinclairzx81 / sidewinder

Type Safe Micro Services for Node
Other
61 stars 3 forks source link

AJV validation fails when using custom SchemaOptions #20

Closed waikikamoukow closed 2 years ago

waikikamoukow commented 2 years ago

I'm having an issue at the moment where if I use a custom SchemaOption, for example:

welcomePage: Type.String({ default: '', label: 'Welcome Page' })

I get an AJV validation error:

Error: strict mode: unknown keyword: "label"

It only appears to happen when building the SideWinder Server (the SideWinder client builds without issue). Is there something I need to do on the server side to enable custom SchemaOptions?

sinclairzx81 commented 2 years ago

@waikikamoukow Hi, Sidewinder currently doesn't allow for arbitrary properties to be applied to top level schemas (this constraint helps mitigate breaking changes in future revisions, but also to keep schemas inline with the JSON schema specification). The validator does however provide an escape hatch for design time / UI metadata with the non-standard design property.

Type.String({ default: '', label: 'Welcome Page' })             // error: unknown property label
Type.String({ default: '', design: { label: 'Welcome Page' } }) // ok

The following is a small function to safely extract a label from a schema.

TypeScript Link Here

import { Type, TSchema } from '@sidewinder/type'

function resolveLabel<T extends TSchema>(schema: T): string {
   if(schema.design && schema.design.label && typeof schema.design.label === 'string') {
     return schema.design.label
   } else {
     return ''
   }
}

const A = Type.String({ design: { label: 'a string' }})
const B = Type.Number({ design: { label: 'a number' }})

const X = resolveLabel(A)
const Y = resolveLabel(B)

Hope this helps! S

waikikamoukow commented 2 years ago

@sinclairzx81 Thanks for the clarification. I'm just wondering if this is the case then should the [prop: string]: any; line be removed from SchemaOptions? That way TypeScript will complain about adding properties to top-level schemas.

sinclairzx81 commented 2 years ago

@waikikamoukow Yeah possibly. The [prop:string]: any is included in TypeBox to enable users to extend schemas however they need (noting that Ajv can be configured for a variety of cases, strict or otherwise), so in this scenario it makes sense for SchemaOptions to be open for additional properties, however as Sidewinder constrains schemas to strict, there is a case to be made for prohibiting additional properties on the schema (likely via @sidewinder/type which is just a direct clone of TypeBox)

I think at this stage, im going to keep on with [prop: string]: any as the plan in future is to have TypeBox taken as a direct dependency at some point (meaning Sidewinder would see the [prop:string]: any on SchemaOptions eventually). However I may potentially look at exporting a StrictSchemaOptions object on TypeBox (which would allow implementations to explicitly type assert for correctness)

export interface StrictSchemaOptions {
  $schema?: string
  /** Id for this schema */
  $id?: string
  /** Title of this schema */
  title?: string
  /** Description of this schema */
  description?: string
  /** Default value for this schema */
  default?: any
  /** Example values matching this schema. */
  examples?: any
}

export interface SchemaOptions extends StrictSchemaOptions {
  [prop: string]: any
}

Alternatively, I've also been considering just using strict all in, and requiring downstream applications to extend the interfaces via module augmentation...something like the following....

// somewhere inside sidewinder in future
declare module '@sinclair/typebox' {
  // augments SchemaOptions with the design property
  export interface SchemaOptions {
     design: { 
        label?: string
        type?: string
     }
  }
}

Generally the latter is a bit frowned upon (despite there being a clear use case for it). Will be reviewing some of these things when I next get around to doing the next revision of Sidewinder and TypeBox.

Will close off this issue for now though Cheers S