Set class-validator as an optional dependencies

[FBerthelot]

Hello,

I just want to open a debate to set class-validator as an optional dependencies.

I personally use something else for extra validation and without class-validator it seam to be working.

I guess, this small PR isn't enough (the documentation is missing, we should update .lock, make sure it's optional everywhere in the code, ...).

[MichalLytek]

I'm preparing v2 major release so we can do such breaking change then. Leaving this open to not forget about that. Updating package.json is not enough 😛

[kf6kjg]

After reviewing the code for my own issues the NPM package output of this, as of 2.0.0-beta.1, utilizes class-validator in the following locations:

1. The ArgumentAssertionError class.
2. The validate-arg resolver.
3. The abstract BuildContext class.

[kf6kjg]

I'm thinking a middleware-style approach could solve this. Or even a pattern like is used all over the place for making libraries agnostic to the exact logger library being used could be made to work.

[MichalLytek]

Sorry, some ghosts are on my keyboard 😄

[MichalLytek]

@FBerthelot @kf6kjg The problem with peer dependency is that then types are optional. So we can't just easily stick with any integration, like calling that automatically, predefined options, validate property being options for class-validator, etc.

It's already a soft peer-dependency, as it does await import(). However typings are blocking from making it truly optional.

I'm thinking a middleware-style approach could solve this.

There's already support for validateFn so no need for any middleware or anything, it works with yup, joi, etc.

I just don't want to make class-validator a recipe only, it has to be super easy to use.

Maybe there's some CDN that makes packages that are only typings of the selected package?

[kf6kjg]

Speaking of the await import call, if class-validator is not installed that line will generate an error like

Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'class-validator' imported from ...

and thus crash the program. You can test this by building the library, editing the built JS file to import a package name that doesn't exist, e.g. await import("class-validatorx");, and then testing.

[MichalLytek]

I have no idea why you have such error. There's no reason why it would even call validateArg with validate: false.

[MichalLytek]

@kf6kjg I even created a fresh repo with a minimal setup, deleted class-validator from node_modules, and all works with validate: false and skipLibCheck.

[kf6kjg]

Ok. I probably came from a false premise on that potential error - my assumption was that I could validate using a different lib than class-validator AND that validateArg would then be called. In hindsight that was probably an unsafe set of assumptions.

Which exposes the fact that I've not personally used type-graphql in a couple years, but due to past experience am supporting a co-worker who is using it while I work on another project - after which point I might be able to work on a project that does use type-graphql again. :)

[MartynasZilinskas]

Hey guys, I would propose another solution for optional class-validator dependency.

NPM supports additional property called peerDependenciesMeta.

In this case it would be:

  "peerDependenciesMeta": {
    "class-validator": {
      "optional": true
    }
  }

This will make package managers to ignore not installed peer dependency.

[MichalLytek]

Ok. I probably came from a false premise on that potential error - my assumption was that I could validate using a different lib than class-validator AND that validateArg would then be called. In hindsight that was probably an unsafe set of assumptions.

This is also working, because there is a return before the await import() in that case.

import "reflect-metadata";
import { graphql } from "graphql";
import {
  Arg,
  Field,
  InputType,
  Query,
  Resolver,
  buildSchema,
} from "type-graphql";

async function main() {
  @InputType()
  class SampleInput {
    @Field()
    message!: string;
  }
  @Resolver()
  class SampleResolver {
    @Query(() => String)
    echo(@Arg("input") input: SampleInput): string {
      return input.message;
    }
  }
  const schema = await buildSchema({
    resolvers: [SampleResolver],
    validate: () => {
      throw new Error("Validation failed");
    },
  });

  const result = await graphql({
    schema,
    source: `{ echo(input: { message: "Hello World!" }) }`,
  });
  console.log(result);
}

main();

➜  typegraphql-test npx ts-node src/index.ts
{
  errors: [
    Error: Validation failed
        at validate (/home/majkel/development/typegraphql-test/src/index.ts:29:13)
        at validateArg (/home/majkel/development/typegraphql-test/node_modules/type-graphql/dist/resolvers/validate-arg.js:35:15)
        at /home/majkel/development/typegraphql-test/node_modules/type-graphql/dist/resolvers/helpers.js:18:55
        at Array.map (<anonymous>)
        at getParams (/home/majkel/development/typegraphql-test/node_modules/type-graphql/dist/resolvers/helpers.js:13:10)
        at /home/majkel/development/typegraphql-test/node_modules/type-graphql/dist/resolvers/create.js:29:52
        at applyMiddlewares (/home/majkel/development/typegraphql-test/node_modules/type-graphql/dist/resolvers/helpers.js:59:16)
        at /home/majkel/development/typegraphql-test/node_modules/type-graphql/dist/resolvers/create.js:28:47
        at executeField (/home/majkel/development/typegraphql-test/node_modules/graphql/execution/execute.js:481:20)
        at executeFields (/home/majkel/development/typegraphql-test/node_modules/graphql/execution/execute.js:413:20) {
      path: [Array],
      locations: [Array],
      extensions: [Object: null prototype] {}
    }
  ],
  data: null
}

[MichalLytek]

This could be a way to make the package optional - import type + ts-ignore:

It maches our case exactly, because when someone installs class-validator, he will use validate option and see ValidationOptions while others would see any but they do not use that option.

[MichalLytek]

This approach works like a charm 🎉

Implemented in 90d23f1, so closing this one 🔒

[kf6kjg]

I do suggest to keep class validator as a peer dependency as that helps provide the version compatibility. Just use the meta section to mark it as optional, thus preventing it from being installed by default but making sure the compatible versions are installed if the consumer does install it.

[MichalLytek]

Fair point, fixed in 1f2efa4 👍

[kf6kjg]

Sorry to be persnickety, but now the peer dependencies are not in alphabetical sort order :)