{"action":"created","comment":{"url":"https://api.github.com/repos/jurassix/react-validation-mixin/pulls/comments/23623571","id":23623571,"diff_hunk":"@@ -1,83 +1,53 @@\n-\"use strict\";\n+'use strict';\n \n-var assign = Object.assign || require('object.assign');\n+require('object.assign').shim();\n+var result = require('lodash.result');\n var ValidationFactory = require('./ValidationFactory');\n \n var ValidationMixin = {\n-\n   /**\n-   * Validate single form field against the components state. If no field\n-   * is provided, validate entire form.\n-   *\n-   * @param {?String} field State key to validate\n-   * @return {Object} newly updated errors object keyed on state field\n-   * names. Missing key or undefined value indicates no error.\n+   * Check for sane configurations\n    */\n-  validate: function(field) {\n-    var validatorTypes = this.validatorTypes || {};\n-    if (typeof this.validatorTypes === 'function') {\n-      validatorTypes = this.validatorTypes();\n+  componentDidMount: function() {\n+    if (typeof this.validatorTypes !== 'function' && !Array.isArray(this.validatorTypes)) {\n+      throw Error('invalid `validatorTypes` type');\n     }\n-    var options = {\n-      schema: validatorTypes,\n-      state: this.state,\n-      field: field\n-    };\n-    var nextErrors = assign({}, this.state.errors, ValidationFactory.validate(options));\n-    this.setState({\n-      errors: nextErrors\n-    });\n-    return nextErrors;\n   },\n \n   /**\n-   * Convenience method to validate a field via an event handler. Useful for\n-   * onBlur, onClick, onChange, etc...\n+   * Validate single form field or entire form against the component's state.\n    *\n-   * @param {?String} field State key to validate\n-   * @param {?Boolean} preventDefault flag to indicate that this event should be canceled.\n-   * default is false.\n-   * @return {function} validation event handler\n+   * @param {?String} key. Field key to validate (entire form if undefined).\n+   * @return {Boolean} Result of `isValid` call after validation.\n    */\n-  handleValidation: function(field, preventDefault) {\n-    return function(event) {\n-      if (preventDefault === true) {\n-        event.preventDefault();\n-      }\n-      this.validate(field);\n-    }.bind(this);\n+  validate: function(key) {\n+    var schema = result(this, 'validatorTypes', {});\n+    var errors = Object.assign({}, this.state.errors, ValidationFactory.validate(this.state, schema, key));\n+    this.setState({\n+      errors: errors\n+    });\n+    return this.isValid(key);","path":"ValidationMixin.js","position":65,"original_position":65,"commit_id":"f8909da5ffd64d0d8efdc476078e40318c123e1c","original_commit_id":"f8909da5ffd64d0d8efdc476078e40318c123e1c","user":{"login":"jurassix","id":1869117,"avatar_url":"https://avatars.githubusercontent.com/u/1869117?v=3","gravatar_id":"","url":"https://api.github.com/users/jurassix","html_url":"https://github.com/jurassix","followers_url":"https://api.github.com/users/jurassix/followers","following_url":"https://api.github.com/users/jurassix/following{/other_user}","gists_url":"https://api.github.com/users/jurassix/gists{/gist_id}","starred_url":"https://api.github.com/users/jurassix/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jurassix/subscriptions","organizations_url":"https://api.github.com/users/jurassix/orgs","repos_url":"https://api.github.com/users/jurassix/repos","events_url":"https://api.github.com/users/jurassix/events{/privacy}","received_events_url":"https://api.github.com/users/jurassix/received_events","type":"User","site_admin":false},"body":"When we call setState() from `validate()` this is an ansync call, which means the call to `isValid()` is not guaranteed to have the new state to validate against. ","created_at":"2015-01-27T17:04:19Z","updated_at":"2015-01-27T17:04:19Z","html_url":"https://github.com/jurassix/react-validation-mixin/pull/8#discussion_r23623571","pull_request_url":"https://api.github.com/repos/jurassix/react-validation-mixin/pulls/8","_links":{"self":{"href":"https://api.github.com/repos/jurassix/react-validation-mixin/pulls/comments/23623571"},"html":{"href":"https://github.com/jurassix/react-validation-mixin/pull/8#discussion_r23623571"},"pull_request":{"href":"https://api.github.com/repos/jurassix/react-validation-mixin/pulls/8"}}},"pull_request":{"url":"https://api.github.com/repos/jurassix/react-validation-mixin/pulls/8","id":28106771,"html_url":"https://github.com/jurassix/react-validation-mixin/pull/8","diff_url":"https://github.com/jurassix/react-validation-mixin/pull/8.diff","patch_url":"https://github.com/jurassix/react-validation-mixin/pull/8.patch","issue_url":"https://api.github.com/repos/jurassix/react-validation-mixin/issues/8","number":8,"state":"open","locked":false,"title":"Major refactoring","user":{"login":"ivan-kleshnin","id":2128182,"avatar_url":"https://avatars.githubusercontent.com/u/2128182?v=3","gravatar_id":"","url":"https://api.github.com/users/ivan-kleshnin","html_url":"https://github.com/ivan-kleshnin","followers_url":"https://api.github.com/users/ivan-kleshnin/followers","following_url":"https://api.github.com/users/ivan-kleshnin/following{/other_user}","gists_url":"https://api.github.com/users/ivan-kleshnin/gists{/gist_id}","starred_url":"https://api.github.com/users/ivan-kleshnin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ivan-kleshnin/subscriptions","organizations_url":"https://api.github.com/users/ivan-kleshnin/orgs","repos_url":"https://api.github.com/users/ivan-kleshnin/repos","events_url":"https://api.github.com/users/ivan-kleshnin/events{/privacy}","received_events_url":"https://api.github.com/users/ivan-kleshnin/received_events","type":"User","site_admin":false},"body":"Please review this and let's discuss :turtle: \r\nI'm trying to address some lurking problems with current implementation.\r\n\r\n1) eager vs lazy validation. I think this should be up to user to decide whether to revalidate or not.\r\nFor now `isValid` may invoke validation, while `getValidationMessages` may not... this is too confusing and involved. In general, if something require comments, it's probably no so obvious. \r\n\r\nSo I put validation in `validate` and `isValid` ang `getValidationMessages` now just check / return against current validation state (`this.errors`). Mixin users should develop their own flows on top of this IMO.\r\n\r\n2) Tests used shared semi-global mutable variables. This is antipattern. It's better to redeclare arguments in functions as it's more explicit, more obvious, and protected from bugs with mutable state.\r\n\r\n3) There were some unnecessary tests which actually tests Joi and not mixin...\r\nI also restructured test sections to remove `valid` vs `invalid` distinction. When we mix valid and invalid input (and you did that in some cases) what section should it belong it? In my experience, It's inconvenient classification style so I replaced it.\r\n\r\n4) Mix of `expect` and `should` style assertions. I played with them a bit and found that `expect` provide much more readable messages in some cases. Also they are more portable. So I replace `should` styled assertions with `expect` styled.\r\n\r\n5) Inconsistent terminology in code (legacy artefacts like `validations` vs `errors`). Cleaned them up.\r\n\r\n6) Inconsistent terminology in docs. Especially `field` when you mean **form field** and **state key** in the same sentence. It may be not obvious, but I put *comparatively* a lot of time in making docs clear in this version.\r\n\r\n7) Other fixes, hopefully following *best practices*.\r\n\r\nWe may split this in chunks again, It's not \"all or nothing\" PR. \r\nI just wanted to implement everything at once, while I had free time to do this and to present the whole picture :)","created_at":"2015-01-27T13:06:37Z","updated_at":"2015-01-27T17:04:19Z","closed_at":null,"merged_at":null,"merge_commit_sha":"10ba11a4106717c1ff5813549694d59be35be14a","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/jurassix/react-validation-mixin/pulls/8/commits","review_comments_url":"https://api.github.com/repos/jurassix/react-validation-mixin/pulls/8/comments","review_comment_url":"https://api.github.com/repos/jurassix/react-validation-mixin/pulls/comments/{number}","comments_url":"https://api.github.com/repos/jurassix/react-validation-mixin/issues/8/comments","statuses_url":"https://api.github.com/repos/jurassix/react-validation-mixin/statuses/f8909da5ffd64d0d8efdc476078e40318c123e1c","head":{"label":"ivan-kleshnin:refactor","ref":"refactor","sha":"f8909da5ffd64d0d8efdc476078e40318c123e1c","user":{"login":"ivan-kleshnin","id":2128182,"avatar_url":"https://avatars.githubusercontent.com/u/2128182?v=3","gravatar_id":"","url":"https://api.github.com/users/ivan-kleshnin","html_url":"https://github.com/ivan-kleshnin","followers_url":"https://api.github.com/users/ivan-kleshnin/followers","following_url":"https://api.github.com/users/ivan-kleshnin/following{/other_user}","gists_url":"https://api.github.com/users/ivan-kleshnin/gists{/gist_id}","starred_url":"https://api.github.com/users/ivan-kleshnin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ivan-kleshnin/subscriptions","organizations_url":"https://api.github.com/users/ivan-kleshnin/orgs","repos_url":"https://api.github.com/users/ivan-kleshnin/repos","events_url":"https://api.github.com/users/ivan-kleshnin/events{/privacy}","received_events_url":"https://api.github.com/users/ivan-kleshnin/received_events","type":"User","site_admin":false},"repo":{"id":29726995,"name":"react-validation-mixin","full_name":"ivan-kleshnin/react-validation-mixin","owner":{"login":"ivan-kleshnin","id":2128182,"avatar_url":"https://avatars.githubusercontent.com/u/2128182?v=3","gravatar_id":"","url":"https://api.github.com/users/ivan-kleshnin","html_url":"https://github.com/ivan-kleshnin","followers_url":"https://api.github.com/users/ivan-kleshnin/followers","following_url":"https://api.github.com/users/ivan-kleshnin/following{/other_user}","gists_url":"https://api.github.com/users/ivan-kleshnin/gists{/gist_id}","starred_url":"https://api.github.com/users/ivan-kleshnin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ivan-kleshnin/subscriptions","organizations_url":"https://api.github.com/users/ivan-kleshnin/orgs","repos_url":"https://api.github.com/users/ivan-kleshnin/repos","events_url":"https://api.github.com/users/ivan-kleshnin/events{/privacy}","received_events_url":"https://api.github.com/users/ivan-kleshnin/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/ivan-kleshnin/react-validation-mixin","description":"Simple validation mixin for React using Joi.","fork":true,"url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin","forks_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/forks","keys_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/teams","hooks_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/hooks","issue_events_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/issues/events{/number}","events_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/events","assignees_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/assignees{/user}","branches_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/branches{/branch}","tags_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/tags","blobs_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/git/refs{/sha}","trees_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/statuses/{sha}","languages_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/languages","stargazers_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/stargazers","contributors_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/contributors","subscribers_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/subscribers","subscription_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/subscription","commits_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/commits{/sha}","git_commits_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/git/commits{/sha}","comments_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/comments{/number}","issue_comment_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/issues/comments/{number}","contents_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/contents/{+path}","compare_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/merges","archive_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/downloads","issues_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/issues{/number}","pulls_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/pulls{/number}","milestones_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/milestones{/number}","notifications_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/labels{/name}","releases_url":"https://api.github.com/repos/ivan-kleshnin/react-validation-mixin/releases{/id}","created_at":"2015-01-23T09:59:24Z","updated_at":"2015-01-26T11:13:57Z","pushed_at":"2015-01-27T12:43:41Z","git_url":"git://github.com/ivan-kleshnin/react-validation-mixin.git","ssh_url":"git@github.com:ivan-kleshnin/react-validation-mixin.git","clone_url":"https://github.com/ivan-kleshnin/react-validation-mixin.git","svn_url":"https://github.com/ivan-kleshnin/react-validation-mixin","homepage":"","size":534,"stargazers_count":0,"watchers_count":0,"language":"JavaScript","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"jurassix:master","ref":"master","sha":"4959ee9db99f125fa39082910d0f6600bdc483e5","user":{"login":"jurassix","id":1869117,"avatar_url":"https://avatars.githubusercontent.com/u/1869117?v=3","gravatar_id":"","url":"https://api.github.com/users/jurassix","html_url":"https://github.com/jurassix","followers_url":"https://api.github.com/users/jurassix/followers","following_url":"https://api.github.com/users/jurassix/following{/other_user}","gists_url":"https://api.github.com/users/jurassix/gists{/gist_id}","starred_url":"https://api.github.com/users/jurassix/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jurassix/subscriptions","organizations_url":"https://api.github.com/users/jurassix/orgs","repos_url":"https://api.github.com/users/jurassix/repos","events_url":"https://api.github.com/users/jurassix/events{/privacy}","received_events_url":"https://api.github.com/users/jurassix/received_events","type":"User","site_admin":false},"repo":{"id":29021349,"name":"react-validation-mixin","full_name":"jurassix/react-validation-mixin","owner":{"login":"jurassix","id":1869117,"avatar_url":"https://avatars.githubusercontent.com/u/1869117?v=3","gravatar_id":"","url":"https://api.github.com/users/jurassix","html_url":"https://github.com/jurassix","followers_url":"https://api.github.com/users/jurassix/followers","following_url":"https://api.github.com/users/jurassix/following{/other_user}","gists_url":"https://api.github.com/users/jurassix/gists{/gist_id}","starred_url":"https://api.github.com/users/jurassix/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jurassix/subscriptions","organizations_url":"https://api.github.com/users/jurassix/orgs","repos_url":"https://api.github.com/users/jurassix/repos","events_url":"https://api.github.com/users/jurassix/events{/privacy}","received_events_url":"https://api.github.com/users/jurassix/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/jurassix/react-validation-mixin","description":"Simple validation mixin for React.","fork":false,"url":"https://api.github.com/repos/jurassix/react-validation-mixin","forks_url":"https://api.github.com/repos/jurassix/react-validation-mixin/forks","keys_url":"https://api.github.com/repos/jurassix/react-validation-mixin/keys{/key_id}","collaborators_url":"https://api.github.com/repos/jurassix/react-validation-mixin/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/jurassix/react-validation-mixin/teams","hooks_url":"https://api.github.com/repos/jurassix/react-validation-mixin/hooks","issue_events_url":"https://api.github.com/repos/jurassix/react-validation-mixin/issues/events{/number}","events_url":"https://api.github.com/repos/jurassix/react-validation-mixin/events","assignees_url":"https://api.github.com/repos/jurassix/react-validation-mixin/assignees{/user}","branches_url":"https://api.github.com/repos/jurassix/react-validation-mixin/branches{/branch}","tags_url":"https://api.github.com/repos/jurassix/react-validation-mixin/tags","blobs_url":"https://api.github.com/repos/jurassix/react-validation-mixin/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/jurassix/react-validation-mixin/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/jurassix/react-validation-mixin/git/refs{/sha}","trees_url":"https://api.github.com/repos/jurassix/react-validation-mixin/git/trees{/sha}","statuses_url":"https://api.github.com/repos/jurassix/react-validation-mixin/statuses/{sha}","languages_url":"https://api.github.com/repos/jurassix/react-validation-mixin/languages","stargazers_url":"https://api.github.com/repos/jurassix/react-validation-mixin/stargazers","contributors_url":"https://api.github.com/repos/jurassix/react-validation-mixin/contributors","subscribers_url":"https://api.github.com/repos/jurassix/react-validation-mixin/subscribers","subscription_url":"https://api.github.com/repos/jurassix/react-validation-mixin/subscription","commits_url":"https://api.github.com/repos/jurassix/react-validation-mixin/commits{/sha}","git_commits_url":"https://api.github.com/repos/jurassix/react-validation-mixin/git/commits{/sha}","comments_url":"https://api.github.com/repos/jurassix/react-validation-mixin/comments{/number}","issue_comment_url":"https://api.github.com/repos/jurassix/react-validation-mixin/issues/comments/{number}","contents_url":"https://api.github.com/repos/jurassix/react-validation-mixin/contents/{+path}","compare_url":"https://api.github.com/repos/jurassix/react-validation-mixin/compare/{base}...{head}","merges_url":"https://api.github.com/repos/jurassix/react-validation-mixin/merges","archive_url":"https://api.github.com/repos/jurassix/react-validation-mixin/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/jurassix/react-validation-mixin/downloads","issues_url":"https://api.github.com/repos/jurassix/react-validation-mixin/issues{/number}","pulls_url":"https://api.github.com/repos/jurassix/react-validation-mixin/pulls{/number}","milestones_url":"https://api.github.com/repos/jurassix/react-validation-mixin/milestones{/number}","notifications_url":"https://api.github.com/repos/jurassix/react-validation-mixin/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/jurassix/react-validation-mixin/labels{/name}","releases_url":"https://api.github.com/repos/jurassix/react-validation-mixin/releases{/id}","created_at":"2015-01-09T15:14:26Z","updated_at":"2015-01-26T16:52:19Z","pushed_at":"2015-01-26T14:56:09Z","git_url":"git://github.com/jurassix/react-validation-mixin.git","ssh_url":"git@github.com:jurassix/react-validation-mixin.git","clone_url":"https://github.com/jurassix/react-validation-mixin.git","svn_url":"https://github.com/jurassix/react-validation-mixin","homepage":"","size":728,"stargazers_count":9,"watchers_count":9,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":2,"forks":1,"open_issues":2,"watchers":9,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/jurassix/react-validation-mixin/pulls/8"},"html":{"href":"https://github.com/jurassix/react-validation-mixin/pull/8"},"issue":{"href":"https://api.github.com/repos/jurassix/react-validation-mixin/issues/8"},"comments":{"href":"https://api.github.com/repos/jurassix/react-validation-mixin/issues/8/comments"},"review_comments":{"href":"https://api.github.com/repos/jurassix/react-validation-mixin/pulls/8/comments"},"review_comment":{"href":"https://api.github.com/repos/jurassix/react-validation-mixin/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/jurassix/react-validation-mixin/pulls/8/commits"},"statuses":{"href":"https://api.github.com/repos/jurassix/react-validation-mixin/statuses/f8909da5ffd64d0d8efdc476078e40318c123e1c"}}}}