sinclairzx81 / typebox

Json Schema Type Builder with Static Type Resolution for TypeScript
Other
4.77k stars 152 forks source link

QoL helper: `TypeCheck.Ensure` #938

Closed eropple closed 4 weeks ago

eropple commented 1 month ago

Hey there! As always, thanks for Typebox; it's still great. :)

One thing I find myself regularly doing, though, is writing something like this:

import type { Static, TSchema } from '@sinclair/typebox';
import type { TypeCheck, ValueError, ValueErrorIterator } from '@sinclair/typebox/compiler';

export class TypeCheckEnsureFailedError extends Error {
  readonly typeCheckErrors: ReadonlyArray<ValueError>;

  constructor(errors: ValueErrorIterator) {
    super("Object validation failed. Consult this.typeCheckErrors for more information.");
    this.typeCheckErrors = [...errors];
  }
}

export function EnsureTypeCheck<T extends TSchema>(value: unknown, checker: TypeCheck<T>): Static<T> {
  const result = checker.Check(value);

  if (!result) {
    throw new TypeCheckEnsureFailedError(checker.Errors(value));
  }

  return value;
}

Have you ever thought about including something like this on TypeCheck for ergonomics purposes? (If it's hiding somewhere and I never noticed, I'm going to laugh.)

sinclairzx81 commented 1 month ago

@eropple Hi, sorry for the delay in reply (Have been extremely busy of late)

Have you ever thought about including something like this (Ensure) on TypeCheck for ergonomics purposes? (If it's hiding somewhere and I never noticed, I'm going to laugh.)

Yeah, have considered it before (both for ergonomics and to align with x(..): assert value as T functions in TS). Historically my reluctance to include it was mostly to prevent TypeBox validation exceptions propagating through stack traces (where the correct error should be a Json Schema error and where this should be agnostic across all Json Schema validators). There was also some uncertainty as to what the Error should be (i.e. should it include the first validation error, all validation errors or only the first)

Historically, my general take on this function was just to let users implement it (as TypeBox and exception handling is usually integrated into wider application infrastructure. I'm still somewhat of the mind that exceptions should belong as part of application infrastructure than specific libraries, but I'm open to revising this (if only to get a TypeBoxValidationError type uses can catch on)

Let me give this some thought :)

eropple commented 1 month ago

Right on, I can definitely understand that viewpoint. And in a lot of cases, personally speaking, I like a library that does that. My suggestion was as much for TypeBoxValidationError as the rest of it; Typebox is a relatively hard library for folks on my teams to wrap their heads around sometimes and being able to provide a "shortest path" that can then be wrapped by that application infrastructure (not super-skilled TypeScript folks, so a message of "this is an error, we know what to do with it") feels like it might be good porcelain over the plumbing.

Thanks for thinking about it! 😃

sinclairzx81 commented 4 weeks ago

@eropple Hiya,

So, decided to go ahead with this. Have introduced two new functions, Assert + AssertError as well as Parse. The Parse function is essentially the suggested Ensure function, but encompasses all the Value operations (Convert, Clean, Default, Decode) related to processing a value.

The associated PR can be found

https://github.com/sinclairzx81/typebox/pull/953

With documentation on the new functions found

https://github.com/sinclairzx81/typebox#values-assert

https://github.com/sinclairzx81/typebox#values-parse

Will close off this issue for now as this should resolve this issue (and make the API a bit simpler to use). Happy to field any questions on the update on this thread. Also keen to get some feedback on AssertError (specifically yielding back ValueErrors) and Parse.

Hope this helps S