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

[Feature Request] Expose validation context to onSubmit() for manually adding async errors #17

Closed
serkandurusoy opened this issue Jun 4, 2016 · 20 comments
Assignees
Labels
Type: Feature New features and feature requests Type: Question Questions and other discussions

Comments

@serkandurusoy
Copy link
Contributor

Simple schema runs validations synchronously, but some validations require server roundtrips or other async calls.

For example, I may want to check a given input if it conforms to some security related constraints or uniqueness constraints etc. And the only way to do that may be to use a server side method. And if it does not, I would like to show an error message to the user.

For such cases, simple schema allows manually adding validation errors

Currently, uniforms only provides the doc to onSubmit callback so there is no way to access the current validation context of the simple schema.

So, it would be great if onSubmit callback had access to the simple schema validation context. In fact, there may be other valuable information that can be provided. So, it would even be better if we had a way to access the whole autoform context in there, not just the document.

@radekmie
Copy link
Contributor

radekmie commented Jun 4, 2016

For such a validation you can pass an error prop to form. Provided error should be compatible with your schema (ValidationError for SimpleSchema). Also, onSubmit is designed to be a final action.


Could you give an example of a situation, where you would use the whole form's context?

@radekmie radekmie self-assigned this Jun 4, 2016
@radekmie radekmie added Type: Feature New features and feature requests Type: Question Questions and other discussions labels Jun 4, 2016
@serkandurusoy
Copy link
Contributor Author

Hmm, do you mean something like:

const PostSchema = new SimpleSchema({
  title: {type: String},
  body: {type: String}
})

class AddPost extends Component {
  constructor(props) {
    super(props)
    this.state={
      asyncErrors: null
    }
  }

  submitPost(doc) {
    PostSchema.clean(doc)

    Meteor.call('checkIfPostTitleIsUnique', doc, (err,res) => {
      if (err) {
        this.setState({
          asyncErrors: new ValidationError([{name: 'title', type: 'notUnique'}])
        })
      }
    })
  }

  render {
    return <AutoForm schema={PostSchema} 
                     error={this.state.asyncErrors}
                     onSubmit={doc => this.submitPost(doc)} />
  }

}

Does this override the form errors or does it get combined with existing errors?

And what do you mean by onSubmit being a final action? In that case, how do you propose we do asynchronous type checks and validatinons?

I can't give you an answer right now about why I might need the form context, but obviously, aldeed:autoform provides necessary mechanisms and also hooks that give you full access to the form before, during and after its submission. And there may be cases such flexibility is valuble.

@serkandurusoy
Copy link
Contributor Author

Edit:

I think a way to access the form's instance or at least the "simple schema validation context" it creates, is very important. That way, we can even define async validations within the schema itself!

@radekmie
Copy link
Contributor

radekmie commented Jun 5, 2016

Your example is 100% correct (okay, there's one thing - AutoForm by default calls .clean() automatically). Current error is either the one from props or the one from validation. By the final action, I meant onSubmit to have no callback for the user - form calls onSubmit and that's all.

@serkandurusoy
Copy link
Contributor Author

Ok, so I have two follow up questions:

  1. are you sure autoform calls .clean() automatically? Because in the forms I design now, the doc passed by onSubmit is never cleaned, for example, autorun's are not run. That's why I got the habit of cleaning the doc before submit. Maybe you are cleaning before validating but you are not passing the cleaned version to submit.
  2. Is there a way to obtain the state of the errors during validation and merge them our own errors so that we provide the error prop as a combination? This would probably require an onValidate or similar callback on the form so that we don't have to wait for submit and show all errors together.

Thanks!

@radekmie
Copy link
Contributor

radekmie commented Jun 5, 2016

  1. My fault - it calls .clean() on the validated model, not the submitted one.
  2. That makes sense. I'll think about it.

@serkandurusoy
Copy link
Contributor Author

  1. Do you plan on implementing clean on the onSubmit callback as well?
  2. Great! It would really make uniforms all more powerful!

@radekmie
Copy link
Contributor

radekmie commented Jun 5, 2016

  1. Probably not, because all schemas are currently bridged and I don't want (at least, currently) to over-complicate it.

@serkandurusoy
Copy link
Contributor Author

Ok, fair enough. I'll continue manually cleaning after my onSubmit then.

I'm also looking forward to your take on item 2.

@serkandurusoy
Copy link
Contributor Author

It looks awesome! Thank you!

@serkandurusoy
Copy link
Contributor Author

Quick question, as far as I understand, if we provide an error to the callback, then the provided error replaces any existing errors, right? It does not "merge" the errors?

So for example, if the schema validation has error on fieldA and myapi provides error for fielfB and if I pass the error from fieldB to the callback, the form only displays the error for fieldB and it "ignores" the other error, right?

Because my intention was somehow to be able to "merge" all errors together.

Or did I miss something?

@radekmie
Copy link
Contributor

radekmie commented Jun 5, 2016

You have to merge it by yourself - uniforms API don't know, what exactly the error is - you can easily create a Bridge with a numeric error. If you are using SimpleSchema, you are using ValidationError - that case is simple:

new ValidationError(error.details.concat([{name: 'field', type: 'asyncError'}]))

@serkandurusoy
Copy link
Contributor Author

Yep, this is what I thought and in fact, putting some more thought into it, I like this better due to its flexibility. It is up to the developer to decide what to do with multiple error sources.

@radekmie
Copy link
Contributor

radekmie commented Jun 5, 2016

Exactly.

@serkandurusoy
Copy link
Contributor Author

@radekmie I think this has created a regression. Now the validation is constantly running and interestingly logging warnings in the console and moving the text cursor to the end of the input!

For example try entering text into the middle of an existing input that does not pass validation

image

notice also the logs! I'm not logging this and interestingly, I have not even submitted the form and it is not autosave either.

@serkandurusoy
Copy link
Contributor Author

@radekmie I confirm that this is a regression. Reverting back to 12 fixes the problem.

@radekmie
Copy link
Contributor

radekmie commented Jun 7, 2016

Okay, I'll try to investigate it, but I think it's not exactly that commit, but some of the others between 12 and 13. You could try to do git bisect or something if you want.

@radekmie radekmie reopened this Jun 7, 2016
@serkandurusoy
Copy link
Contributor Author

I tried to do git bisect but I don't have a testing workflow.

I tried to set up a new meteor project, clone uniforms to an external folder and cd into packages/uniforms and tried to run npm pack so that I can use a local npm package for testing but npm throws me all kinds of errors and nothing useful.

I wonder how you are running your local test setup.

This is in fact quite important because I'll also be working on the examples and docs site so I need to be able to use uniforms from local (to write examples for unpublished versions) instead of from npm.

How do you do that?

@radekmie
Copy link
Contributor

radekmie commented Jun 7, 2016

In a quite hacky way - I have a meteor project with package.json with an absolute path to these packages on my disk (also, you can try npm link).

@radekmie
Copy link
Contributor

radekmie commented Jun 7, 2016

I'll move it to a new issue.

@radekmie radekmie moved this to Closed in Open Source Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New features and feature requests Type: Question Questions and other discussions
Projects
Archived in project
Development

No branches or pull requests

2 participants