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

fix: optional field validation #124

Merged
merged 9 commits into from
Nov 11, 2022
Merged

fix: optional field validation #124

merged 9 commits into from
Nov 11, 2022

Conversation

Gozala
Copy link
Collaborator

@Gozala Gozala commented Nov 10, 2022

Fixes #123

Overview

  • Adds optional parameter to the parse function used by validator to parse capabilities.
  • .select method which selects matching proofs from delegation chain passes option:true to parser so nb fields are never required in delegations.
  • derives of the capability infers type nb fields as optionals.
  • derives in cap.derive({ to, derives }) infers nb fields as optionals.
  • .delegate(opts).capabilities[0].nb infers fields as optionals.
  • .invoke(opts).capabilities[0].nb infers fields as defined in schema.

I hate introducing more utility types to make this work, but unfortunately there was no simple way to fix all the problems. It is possible to rework validator such that we would be able to remove bunch of these utility types, however it is non trivial change and I'd rather do it later on when there's less urgency.

@Gozala Gozala marked this pull request as draft November 10, 2022 20:45
@Gozala Gozala marked this pull request as ready for review November 11, 2022 11:22
Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

This looks ok to me, although with the caveat that I don't have a a very deep level of understanding of ucanto yet...

packages/validator/test/inference.spec.js Outdated Show resolved Hide resolved
packages/validator/src/capability.js Outdated Show resolved Hide resolved
@Gozala Gozala merged commit 87b70d2 into main Nov 11, 2022
@Gozala Gozala mentioned this pull request Dec 2, 2022
This was referenced Dec 14, 2022
This was referenced Apr 11, 2023
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.

Validator fails if some nb fields are optional & omitted in delegations
2 participants