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

New plugin: useExtendedValidation and implementation of @oneOf directive #149

Merged
merged 4 commits into from
May 2, 2021

Conversation

dotansimha
Copy link
Collaborator

@dotansimha dotansimha commented Apr 28, 2021

Background for this plugin:

At the moment, the existing phases of the requests pipeline are consist of: parse, validate, contextBuilding, execute.

Variables parsing happens on a separate flow (as part of the request pipeline), and it's being provided to execute externally. Input coercion happens during execution, as an internal step. The implication of this is the fact that validation rules only have access to the AST of the documents (and to the validation context in general), but not to variables or execution params.

Modern implementation often needs to access variables in order to provide a more complete validation phase.

To work around that, we can implement in envelop a plugin for extended validation. We follow the same ValidationRule interface, with the addition of ExecutionArgs provided. And Envelop makes sure to pass run that before execute runs.
The flow is the same as validate and it uses the same visitors flow, but allows developers to write more simplified code, and do more enriched validations.

As a simple example for this POC, I implemented the basics of this plugin and also implemented @oneOf directive for adding support for input unions.

More ideas that we might be able to implement:

  • Allow having arguments/directives for scalars at the level of the usage, something like field: String @max(5).
  • Allow having validation rules that are aware of the user: field: String! @authorization(role: "test").

This might also introduce another way of doing directive-based validations, since it does not require composing resolvers.

@theguild-bot
Copy link
Collaborator

theguild-bot commented Apr 28, 2021

The latest changes of this PR are available as alpha in npm (based on the declared changesets):

@envelop/core@0.1.4-alpha-831a172.0
@envelop/extended-validation@0.0.1-alpha-831a172.0
@envelop/types@0.1.4-alpha-831a172.0

@codecov-commenter
Copy link

codecov-commenter commented May 2, 2021

Codecov Report

Merging #149 (d1a4665) into main (e25d704) will increase coverage by 2.48%.
The diff coverage is 98.43%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #149      +/-   ##
==========================================
+ Coverage   75.49%   77.97%   +2.48%     
==========================================
  Files          20       24       +4     
  Lines         510      572      +62     
  Branches       89      105      +16     
==========================================
+ Hits          385      446      +61     
- Misses        125      126       +1     
Impacted Files Coverage Δ
packages/plugins/extended-validation/src/common.ts 92.30% <92.30%> (ø)
packages/core/src/create.ts 75.00% <100.00%> (+0.78%) ⬆️
packages/plugins/extended-validation/src/index.ts 100.00% <100.00%> (ø)
packages/plugins/extended-validation/src/plugin.ts 100.00% <100.00%> (ø)
...es/plugins/extended-validation/src/rules/one-of.ts 100.00% <100.00%> (ø)
packages/testing/src/index.ts 91.17% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e25d704...d1a4665. Read the comment docs.

@dotansimha dotansimha changed the title WIP: useExtendedValidation New plugin: useExtendedValidation and implementation of @oneOf directive May 2, 2021
@dotansimha dotansimha merged commit ced704e into main May 2, 2021
@dotansimha dotansimha deleted the extended-validation branch May 2, 2021 14:15
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.

4 participants