MichalLytek / class-transformer-validator

A simple plugin for class-transformer and class-validator which combines them in a nice and programmer-friendly API.
MIT License
200 stars 19 forks source link

Simplified usage for type guards #14

Open dbartholomae opened 5 years ago

dbartholomae commented 5 years ago

I'm currently using this package with a helper to create type guards:

/** Create a type guard function for a class decorated with validators. */
export function isValidFactory<T extends object>(classType: ClassType<T>): (instance: object) => instance is T {
  return (instance: object): instance is T => {
    try {
      transformAndValidateSync(classType, instance)
      return true
    } catch (err) {
      return false
    }
  }
}

Would you be interested in a PR that adds this helper?

MichalLytek commented 5 years ago

So you propose that instead of throwing an error:

async (req, res) => {
    try {
        // transform and validate request body
        const userObject = await transformAndValidate(User, req.body);
        // infered type of userObject is User
        // you can access all class prototype properties and methods
    } catch (error) {
        // your error handling
        console.err(error);
    }
}

We can return boolean that indicates if the value object passed validation:

async (req, res) => {
    if (isValid(User, req.body)) {
        // infered type of req.body is User
        // but you can't access all class prototype properties and methods
    } else {
        // your error handling
        console.err(`Invalid body value: ${req.body}`);
    }
}

The problem is that it won't transform the object into an instance of the class (like class-transformer is doing) without mutating the provided object - neither of them sounds good 😕

dbartholomae commented 5 years ago

It's not a replacement for the current functionality. I agree that there are many situations where the full spectrum is needed. But there might also be those where a slimmed down version is all you need and could make the code more readable, specifically if the class is a replacement for a simple interface.

E. g. let's say I have a configuration object

enum LogLevel {
  error: 'error';
  info: 'info';
}
class LoggerOptions {
  @IsEnum(LogLevel)
  level: LogLevel
}

const isLoggerOptions = isValidFactory(LoggerOptions)

then I could write

class Logger {
  constructor (private options: LoggerOptions) {
    if (!isLoggerOptions(options)) {
      throw new TypeError(`Expected LoggerOptions, received ${JSON.stringify(options)} instead`)
    }
  }
}

To be honest I am not 100% it is worth it myself, so we could also arrive at the conclusion this functionality doesn't belong in this package :)

MichalLytek commented 5 years ago

It's not a replacement for the current functionality

I mean replacement in your app code, not in this library. We can export more functions without a problem 😉

The use case is nice but I would expose only a isValid function - you can then create your own factory function easily in one function const isValidFactory = cls => data => isValid(cls, data). Or just create it inline: const isLoggerOptions = data => isValid(LoggerOptions, data)

dbartholomae commented 5 years ago

We've worked more on our codebase and came to the realization that for us it is fine to always use errors instead of books and for type checking, so from my side this issue could be closed.

MichalLytek commented 5 years ago

@dbartholomae I will left that open, maybe someone else will be interested in this kind of function.

We can make it typesafe by using conditional types and skipping the function properties.