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: add standard schema support and associated tests #2588

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

GregoireBellon
Copy link

@GregoireBellon GregoireBellon commented Mar 10, 2025

Description

This PR adds the support for standard schema, by copying their interface, extending the validator with it, and mapping class validator's "Validation Error" type to standard schema's "Issue".

I also wrote some tests, which are the same as the "nested validation" ones.

Since this function would be in the hot path, I tried to make it optimized, by avoiding recursion and preferring mutation to copy.

Supporting the Standard Schema interface is important, since it's blocking the adoption of it by NestJs (see related nestjs/nest/#14539)

edit: by itself, the PR in its current form would not be enough. We need to add a wrapper function, that takes a schema as input, and that outputs a validator containing the schema. Let me know if it sounds good to you.

Disclosure

I had to ignore the ESLint warning when copying the standard schema interface.

Checklist

  • the pull request title describes what this PR does (not a vague title like Update index.md)
  • the pull request targets the default branch of the repository (develop)
  • the code follows the established code style of the repository
    • npm run prettier:check passes
    • npm run lint:check passes
  • tests are added for the changes I made (if any source code was modified)
  • documentation added or updated
  • I have run the project locally and verified that there are no errors

Fixes

addresses #2577

@GregoireBellon GregoireBellon marked this pull request as draft March 10, 2025 21:57
@0x0bit
Copy link

0x0bit commented Mar 11, 2025

@braaar @NoNameProvided

/**
* The non-existent issues.
*/
readonly issues?: undefined;

Choose a reason for hiding this comment

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

Is this type correct?

@niemyjski
Copy link

@braaar @NoNameProvided This would be massive if this can be supported. See discussion here nestjs/nest#14539 for possible rework?

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

Successfully merging this pull request may close these issues.

3 participants