Handle empty objects in EqualKeys
#50
Conversation
ruimarinho
force-pushed
the
enhancement/handle-empty-objects
branch
from
4920d52 to
77374da
Compare
ruimarinho
force-pushed
the
enhancement/handle-empty-objects
branch
2 times, most recently
from
f183494 to
697cc58
Compare
|
ping @nunofgs |
| @@ -35,7 +35,7 @@ describe('EqualKeysAssert', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| it('should throw an error if an object does not have the expected keys', () => { | |||
| it('should throw an error if the object not have the expected keys', () => { | |||
|
Hey guys, not trying to be difficult but I'm still having trouble with this concept. I personally dislike mixing "empty object" validation with key matching but I will of course capitulate if the intent is to follow the should.js implementation (as mentioned here). Having said that, I believe the following should not throw an error: // This throws an error.
new Assert().EqualKeys().validate({});
// Since this also does not throw an error.
({}).should.have.keys();If you fix that issue, I have no further objections. /cc @ricardogama, @ruimarinho |
|
Also, regardless of the decision, should we also update the README with a description of the breaking change? |
| } | ||
| }); | ||
|
|
||
| it('should allow setting no keys', () => { |
There was a problem hiding this comment.
What are we setting here? WDYT of rewording to something along the lines of:
'should allow `keys` to be undefined'
7169c49 to
74fd899
Compare
|
@nunofgs I think the README is correct, but since the outcome might be different for a certain input value, I think publishing as 2.0.0 is enough. I agree with your comment regarding |
|
👍 |
|
LGTM |
ruimarinho
force-pushed
the
enhancement/handle-empty-objects
branch
from
74fd899 to
9a33251
Compare
Handle empty objects in `EqualKeys`
Closes #34.