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

Added Support for Syncronous Testing of Promise Presence #221

Closed
wants to merge 1 commit into from

Conversation

@bbusschots-mu
Copy link

@bbusschots-mu bbusschots-mu commented Jul 31, 2017

While the resolution of a promise is clearly an asynchronous thing, testing for the presence of a promise is not, so it should be possible to test whether or not a given value is a promise using validate() or validate.single().

This very small commit makes that possible by adding an option allowPromises to validate().

The following console output from node run from the project root folder shows the proposed change in action:

cc-dsk-2ss:validate.js bart$ node
> let validate = require('./');
undefined
> // create a custom promise validator
undefined
> validate.validators.promise = function(val, opts){
...     // implicitly pass empty values
...     if(!validate.isDefined(val)) return undefined;
...         
...     // test the passing case
...     if(validate.isPromise(val)){
.....         return undefined;
.....     }
...         
...     // if we got here, we have an error, so return an error message
...     return opts.message || this.message || 'is not a promise';
... };
[Function]
> // create a promise
undefined
> var p = Promise.resolve('thingys');
undefined
> 
> // should throw an error
undefined
> validate.single(p, {presence: true, promise: true});
Error: Use validate.async if you want support for promises
    at validate (/Users/bart/Documents/Temp/fromGitHub/validate.js/validate.js:33:17)
    at Function.single (/Users/bart/Documents/Temp/fromGitHub/validate.js/validate.js:202:14)
    at repl:1:10
    at ContextifyScript.Script.runInThisContext (vm.js:44:33)
    at REPLServer.defaultEval (repl.js:239:29)
    at bound (domain.js:301:14)
    at REPLServer.runBound [as eval] (domain.js:314:12)
    at REPLServer.onLine (repl.js:433:10)
    at emitOne (events.js:120:20)
    at REPLServer.emit (events.js:210:7)
> // should fail validation
undefined
> validate.single('stuff', {presence: true, promise: true}, {allowPromises: true});
[ 'is not a promise' ]
> // should pass validation
undefined
> validate.single(p, {presence: true, promise: true}, {allowPromises: true});
undefined
> 

I could also add the promise validator as a separate pull request?

While the resolution of a promise is clearly an asynchronous thing, testing for the presence of a promise is not, so it should be possible to test if a given value is a promise using `validate()` or `validate.single()`.

This very small commit makes that possible by adding an option `allowPromises` to `validate()`.
@coveralls
Copy link

@coveralls coveralls commented Jul 31, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling ddc1a85 on bbusschots-mu:master into 07f64ec on ansman:master.

@ansman
Copy link
Owner

@ansman ansman commented Oct 17, 2017

This has actually been a long standing bug.

@ansman
Copy link
Owner

@ansman ansman commented Oct 17, 2017

Closed via bfa56eb

@ansman ansman closed this Oct 17, 2017
ansman added a commit that referenced this pull request Oct 17, 2017
@ansman ansman added this to the 0.12.0 milestone Oct 17, 2017
@ansman ansman added the bug label Oct 17, 2017
@ansman
Copy link
Owner

@ansman ansman commented Oct 17, 2017

A fix has been released in 0.12.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.