-
Notifications
You must be signed in to change notification settings - Fork 4
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
Refactor, can-validate supplies utilities for other validate modules #48
Conversation
- Improve handling of missing key for errors when object format is desired - Impove handling of different and mised errors types in raw errors.
var Construct = require('can-construct'); | ||
var validate = require('can-validate/lib/validate'); | ||
var helpers = require('can-validate/lib/helpers'); | ||
var formatErrors = require('can-validate/format-errors/'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't use paths that end with /
so other module loaders can load files. Use can-validate/format-errors/format-errors
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I probably wouldn't have another module if this is all can-validate
exports. I'd move this code into can-valdiate.js
|
||
Shared utilities and type definitions to process validation errors. | ||
|
||
@option {function} formatErrors Formats errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be documented independently, not in here. Plus there should be a @type
describing what this module exports, which is an object.
|
||
@body | ||
|
||
## Utility Methods |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is introduced w/o any context, which doesn't "read" well.
|
||
An object that defines a validation failure. | ||
|
||
@option {string} message A reason why value is in an invalid state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@type
should be used. It should include an example of the object.
|
||
The expected response from a validator if a value fails validation. | ||
|
||
@option {undefined|null} Expected when value passes validation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
null
isn't above. Use @type
instead of @option
here.
errors = [errors]; | ||
} | ||
each(errors, function (error) { | ||
resp = resp.concat(parseErrorItem(error)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is slow compared to using [].push.apply( resp, parseErrorItem(error) )
. I'm not a fan of resp
. It's so generic.
QUnit.test('formatErrors to flat',function(){ | ||
var errors = formatErrors(errorObject, 'flat'); | ||
QUnit.equal(errors.length, 4); | ||
QUnit.equal(errors[0], numberString); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spacing mistakes
var errors = formatErrors(errorObject, 'object'); | ||
QUnit.ok(errors.name); | ||
QUnit.equal(errors.name.length, 2); | ||
QUnit.equal(errors.name[0], requireString); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spacing
QUnit.ok(validate.validations.test === noop); | ||
QUnit.test('formatErrors to flat',function(){ | ||
var errors = formatErrors(errorObject, 'flat'); | ||
QUnit.equal(errors.length, 4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's good to have a 3rd argument explaining what's being tested.
QUnit.test('formatErrors normalizes different error types', function () { | ||
var errors = formatErrors(errorObject); | ||
QUnit.ok(errors); | ||
QUnit.equal(errors.length, 4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should really use QUnit.deepEqual
here for better testing accuracy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QUnit.deepEqual on the whole errors response like:
QUnit.deepEqual( errors, [{ ... }, { ... } ])
- Moved formatErrors. Updated tests. - Improved docs based on PR comments
Resolves #42
Added
formatErrors
formatErrors
takes an errors value and converts it to a more formal structure. The errors value must match any of the types defined in the errors docs.Added docs and tests for
formatErrors
.Added types
Added
Errors
,Error
, andValidator
types documentation.Removed all kinds of stuff