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

request: setValidationEnabled should [un]wrap the validator into an PartialValidator<T> #191

Open
1 task done
kyranet opened this issue Sep 7, 2022 · 0 comments
Open
1 task done

Comments

@kyranet
Copy link
Member

kyranet commented Sep 7, 2022

Is there an existing issue or pull request for this?

  • I have searched the existing issues and pull requests

Feature description

Right now, all validators have checks for whether or not they should run validations, as seen below:

// If validation is disabled (at the validator or global level), we only run the `handle` method, which will do some basic checks
// (like that the input is a string for a string validator)
if (!this.shouldRunConstraints) {
return this.handle(value).unwrap() as R;
}

This comes with a large performance impact, specially from those who desire to use the library without conditional validation. Also goes against Shapeshift's internal design of running the least amount of conditionals as possible.

Before we added conditional validation, Shapeshift was comfortably among the fastest libraries in our benchmarks.

Desired solution

A wrapper would solve the performance impact by making the validators always run the logic and constraints, where the PartialValidator<T> would exclusively only run the handler and never the constraints (with no extra checks, of course).

For function (dynamic validation), we can also add a second class, or add a check in PartialValidator<T>, invalidating the last sentence in the previous paragraph.

Unwrapping a PartialValidator<T> should give back the underlying, fully-checked validator.

Alternatives considered

N/a.

Additional context

No response

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

No branches or pull requests

1 participant