Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(framework): add class-validator support #6945

Open
wants to merge 70 commits into
base: next
Choose a base branch
from

Conversation

paulwer
Copy link
Contributor

@paulwer paulwer commented Nov 11, 2024

What changed? Why was the change needed?

Class Validator support for @novu/framework

Screenhots

Class validator peer dependency import failure message

 ⨯ Error: Tried to use a class-validator schema in @novu/framework without class-validator-jsonschema installed. Please install it by running `npm install class-validator-jsonschema`.
    at se (/Users/rifont/git/framework-next-turbo/.next/server/app/api/novu/route.js:411:77618)
    at async si.canHandle (/Users/rifont/git/framework-next-turbo/.next/server/app/api/novu/route.js:411:79403)
    at async sp (/Users/rifont/git/framework-next-turbo/.next/server/app/api/novu/route.js:411:81104)
    at async Object.discover (/Users/rifont/git/framework-next-turbo/.next/server/app/api/novu/route.js:505:10531)
    at async sl.addWorkflows (/Users/rifont/git/framework-next-turbo/.next/server/app/api/novu/route.js:501:8314)
    at async (/Users/rifont/git/framework-next-turbo/.next/server/app/api/novu/route.js:505:3508)
    at async te.do (/Users/rifont/git/framework-next-turbo/node_modules/.pnpm/next@15.0.3-canary.9_react-dom@19.0.0-rc-02c0e824-20241028_react@19.0.0-rc-02c0e824-20241028__utrvdij6okqvk6cvj3mqcl5cuy/node_modules/next/dist/compiled/next-server/app-route.runtime.prod.js:18:17826)
    at async te.handle (/Users/rifont/git/framework-next-turbo/node_modules/.pnpm/next@15.0.3-canary.9_react-dom@19.0.0-rc-02c0e824-20241028_react@19.0.0-rc-02c0e824-20241028__utrvdij6okqvk6cvj3mqcl5cuy/node_modules/next/dist/compiled/next-server/app-route.runtime.prod.js:18:22492)
    at async doRender (/Users/rifont/git/framework-next-turbo/node_modules/.pnpm/next@15.0.3-canary.9_react-dom@19.0.0-rc-02c0e824-20241028_react@19.0.0-rc-02c0e824-20241028__utrvdij6okqvk6cvj3mqcl5cuy/node_modules/next/dist/server/base-server.js:1455:42) {
  statusCode: 500,
  code: 'MissingDependencyError',
  data: [Array]
}
Expand for optional sections ### Related enterprise PR

Special notes for your reviewer

closes #6682

reference #6840

paulwer and others added 30 commits November 4, 2024 14:17
Co-authored-by: Richard Fontein <32132657+rifont@users.noreply.github.com>
Co-authored-by: Richard Fontein <32132657+rifont@users.noreply.github.com>
…validator-support' into feat-package-class-validator-support
Copy link

netlify bot commented Nov 11, 2024

👷 Deploy request for novu-stg-vite-dashboard-poc pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit d9c952c

@paulwer paulwer changed the title Feat package class validator support feat(framework): add class-validator support Nov 11, 2024
@paulwer
Copy link
Contributor Author

paulwer commented Nov 12, 2024

@rifont is it possible to include this in Release 2.4.0
#6952

@rifont
Copy link
Contributor

rifont commented Nov 12, 2024

Hey @paulwer , we still need to address the comments from here regarding polymorphism, this is the last blocker to release. Would it be possible for you to take a look at those?

@paulwer
Copy link
Contributor Author

paulwer commented Nov 12, 2024

i could resolve them tomorrow morning, if that fits your schedule.

@paulwer
Copy link
Contributor Author

paulwer commented Nov 12, 2024

when trying to use a discriminated union type an error occurs because class-validator-jsonschema cannot directly infer a discriminated union type using decorators like @type inside the union classes. Since class-validator-jsonschema doesn’t fully support conditional @type decorators.

the following is therefore not possible to implement:

class StringType {
  @IsIn(['stringType'])
  type!: 'stringType';

  @IsString()
  stringVal!: string;
}

class NumberType {
  @IsIn(['numberType'])
  type!: 'numberType';

  @IsNumber()
  numVal!: number;
}

class BooleanType {
  @IsIn(['booleanType'])
  type!: 'booleanType';

  @IsBoolean()
  boolVal!: boolean;
}

class DiscriminatedUnionType {
  @IsIn(['stringType', 'numberType', 'booleanType'])
  type!: 'stringType' | 'numberType' | 'booleanType';

  @ValidateNested()
  @Type((obj) => {
    switch (obj?.object.type) {
      case 'stringType':
        return StringType;
      case 'numberType':
        return NumberType;
      case 'booleanType':
        return BooleanType;
      default:
        throw new Error(`Invalid type: ${obj?.object.type}`);
    }
  })
  element!: StringType | NumberType | BooleanType;
}

export class NestedUnionSchema {
  @ArrayNotEmpty()
  @ValidateNested({ each: true })
  @Type(() => DiscriminatedUnionType)
  elements!: DiscriminatedUnionType[];
}

Feature Request: epiphone/class-validator-jsonschema#114

@paulwer
Copy link
Contributor Author

paulwer commented Nov 12, 2024

similar for oneOf

@paulwer
Copy link
Contributor Author

paulwer commented Nov 12, 2024

@rifont ready again:

  1. add tests for enum support of class-validator/-transformer.
    => done

  2. You may also need to recheck anyOf/allOf etc.
    f.ex. https://stackoverflow.com/questions/75782271/how-to-validate-nested-array-of-multiple-types-using-class-validator
    => done (see above)

  3. add tests for validating array of objects should be added as a test (ValidateNested({ each }))
    => done

Comment on lines +217 to +219
"reflect-metadata": {
"optional": true
},
Copy link
Contributor Author

@paulwer paulwer Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rifont should we remove reflect-metadata from the peer-deps and deps also?
technicaly they are only required for development and the user has to use it on their server entrypoint in order to have a working class-validator/class-transformer setup.
So i am not sure how to interpret this in scope of this lib :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🚀 Feature: @novu/framework Integration with class-validator
2 participants