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

Add normalize method #275

Merged
merged 2 commits into from
Oct 17, 2021
Merged

Add normalize method #275

merged 2 commits into from
Oct 17, 2021

Conversation

adam-mccormick
Copy link

This PR adds a normalize method to the Validator class. This method can take a schema, type, or string and resolve all shorthands and turn it into a 'normalized' form of a fastest-validator schema. This is useful for being able to inspect a schema and not have to worry about dealing with short hands, aliases, etc.

For example:

validator.normalize({ a: "string[]|optional" })

Will result in:

{
  a: {
    type: "array",
    optional: true,
    items: {
      type: "string"
    }
  }
}

Not a typescript guy so the ts definition of this method may not be super correct.

There's some duplication with the getRuleFromSchema method but I didn't want to make any changes there. Happy to revisit if required.

Copy link
Owner

@icebob icebob left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I don't like that the same logic is implemented in both places. Later we have an inconsistency state if somebody makes changes in getRuleFromSchem but forget to update normalize as well.

Please move the duplicated parts into one method and call it from both.

The methods getRuleFromSchema and normalize now call out to a new method; resolveType which takes care of schemas which are strings, arrays or objects with a $$type property. This removes the code duplication between getRuleFromSchema and normalize.

Also removed a nonsense test.
@adam-mccormick
Copy link
Author

@icebob I've moved the duplication to a new method. Let me know if there are any other changes you'd like to see.

@@ -720,17 +720,6 @@ describe("Test normalize", () => {
}
});
v.add("custom", () => "");
it("should not change original schema", () => {
Copy link
Owner

Choose a reason for hiding this comment

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

Why is it removed?

Copy link
Author

Choose a reason for hiding this comment

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

The getRuleFromSchema method modifies the passed schema and the refactor moves some of that logic to a method used by both it and normalize. The schema passed to normalize will be modified as well which I think makes it more in line with the rest of the API.

Copy link
Owner

@icebob icebob left a comment

Choose a reason for hiding this comment

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

Thanks, great PR!

@icebob icebob merged commit 0c9e706 into icebob:master Oct 17, 2021
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.

3 participants