VulcanJS / Vulcan

🌋 A toolkit to quickly build apps with React, GraphQL & Meteor
http://vulcanjs.org
MIT License
7.98k stars 1.89k forks source link

Better Schema Validation with SimpleSchema #1903

Open PolGuixe opened 6 years ago

PolGuixe commented 6 years ago

As I can see in: https://github.com/VulcanJS/Vulcan/blob/7d03d8008e5937cf0b9e145e07d8278202547af5/packages/vulcan-lib/lib/modules/validation.js#L59-L68

The document is validated as a whole and not for each field. I think it should be done per field, this can provide useful information for the users. e.g. "This name should be at least 3 characters", "This phone number is not valid", etc.

SachaG commented 6 years ago

Maybe we need to use a different SimpleSchema feature then. Maybe this?

https://github.com/aldeed/simple-schema-js#validate-an-object-and-get-the-errors

I'd accept a PR to improve the SS part of error handling.

PolGuixe commented 6 years ago

I'll try it. Before I want to finalize a package to allow file uploads, etc. ;) I hope I can publish it soon.

vincro commented 6 years ago

@SachaG these are the methods I use to generate a custom validation context + errors which I then pass to Formsy as validationErrors prop which renders the errors against each field. The only gotcha with SS is that you have to generate the error map, and then translate the error key into a readable text string.

nolandg commented 6 years ago

I'm not using SmartForm and I've made my own abstract base component class EditDocumentForm which say my PostsEditForm extends. I'm doing what @vincro suggested and it seems to work well so far. It will be a big job to code all those error type into user-facing strings but at least for now the errors have a specific field so I can highlight the form inputs in error and pop up a message beneath them.

Also I used the SS clean function to help coerce essentially valid data that isn't quite the right format to avoid annoying errors. Like it takes the string Tue Feb 27 2018 16:09:56 GMT-0800 (PST) and automatically converts to a Date() object if the field type is date to avoid a mess of converting back and while still keeping the document and all my controlled input components happy.

nolandg commented 6 years ago

Is there any interest in catching up with SimpleSchema 2 and getting away from this deprecated and essentially unmaintained version? I could take a look at that if people think that's the way to go. I do.

SachaG commented 6 years ago

What's SS 2? Do you mean https://github.com/aldeed/simple-schema-js ? Because that's already what we're using.

nolandg commented 6 years ago

oh, sorry, I'm confused. thought we were on the old meteor package version because the Vulcan docs link to it. I see we're on the latest now. disregard me.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.