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

Adds validate function to attributes #380

Merged
merged 1 commit into from May 12, 2023

Conversation

rpaul-stripe
Copy link
Contributor

This PR addresses issue #379. It adds support for writing a custom validate function for an individual schema attribute. The function can return an arbitrary number of Markdoc validation error objects.

After weighing the tradeoffs, I decided not to support async functions for this case because it would require us to collect and await all of them before continuing, introducing more complexity. We can still potentially add support for that in the future if there's sufficient demand.

Copy link
Contributor

@mfix-stripe mfix-stripe left a comment

Choose a reason for hiding this comment

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

@rpaul-stripe I thought we chatted about this previously. I feel like this unnecessarily expands our API surface area, when custom types already support this case, and lead to better reuse.

@matv-stripe @nkohari-stripe what do you think about this proposal?

@rpaul-stripe
Copy link
Contributor Author

@mfix-stripe I think there's value in being able to do one-off validation functions independently of the type attribute without having to create a reusable type. And I think it's consistent with the way that the validation function works on the schema object, so it doesn't feel like it clutters up the API. We can provide guidance to encourage people to still use custom types in cases where reuse is desirable.

I have been thinking a lot about how we want to handle complex object/array attribute values. Rather than adding a bunch of complexity to Markdoc's type system to explicitly support more feature-rich validation on nested values, what I want to do is provide a pattern that makes it really easy for users to pull in Zod or JSON Schema for those cases. I think a validate function on the attribute is ultimately a good way to handle this.

Copy link
Contributor

@mfix-stripe mfix-stripe left a comment

Choose a reason for hiding this comment

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

Ack, won't block your vision then 👍

@rpaul-stripe rpaul-stripe merged commit bc387e3 into main May 12, 2023
2 checks passed
@rpaul-stripe rpaul-stripe deleted the adds-attribute-validate-function branch May 12, 2023 18:59
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.

None yet

3 participants