sinclairzx81 / typebox

Json Schema Type Builder with Static Type Resolution for TypeScript
Other
5.08k stars 161 forks source link

`CloneType` method overwrites `options` with `schema`, but it should be the opposite. #1056

Closed cjimenezsaiz closed 4 days ago

cjimenezsaiz commented 2 weeks ago

Current Implementation

/** Clones a Type */
export function CloneType<T extends TSchema>(schema: T, options?: SchemaOptions): T {
  return options === undefined ? Clone(schema) : Clone({ ...options, ...schema });
}

Expected Implementation

/** Clones a Type */
export function CloneType<T extends TSchema>(schema: T, options?: SchemaOptions): T {
  return options === undefined ? Clone(schema) : Clone({ ...schema, ...options });
}

This was modified in PR #941, and the implementation at that time was as follows:

/** Clones a Type */
export function CloneType<T extends TSchema>(schema: T, options: SchemaOptions = {}): T {
  return { ...Clone(schema), ...options };
}

A workaround is easy to implement, but I believe that the previous behavior was correct.

sinclairzx81 commented 4 days ago

@cjimenezsaiz Hi, sorry for the delay.

Expected Implementation

export function CloneType<T extends TSchema>(schema: T, options?: SchemaOptions): T {
  return options === undefined ? Clone(schema) : Clone({ ...schema, ...options });
}

TypeBox uses the order { ...options, ...schema } to prevent options from overriding schematics that constitute the type. Also, this order needs to be preserved as TypeBox's internals are dependent on the ordering (more so the knowledge of the ordering) as it relates to quite a few internal mapping operations.

If you need the inverse ordering, it's possible to create this with the public API.

import { Clone, TSchema, SchemaOptions } from '@sinclair/typebox'

export function CloneTypeRight<T extends TSchema>(schema: T, options?: SchemaOptions): T {
  return options === undefined ? Clone(schema) : Clone({ ...options, ...schema });
}

Will close off this issue for now as things are working by design. Happy to discuss more on this thread if you have other questions around this functionality. Cheers S