-
-
Notifications
You must be signed in to change notification settings - Fork 699
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
Add ES6 collection support to include() #994
Changes from all commits
07de33e
86a9ea3
4d56c3d
0e809c3
defc699
039564d
df6badd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -330,6 +330,16 @@ module.exports = function (chai, _) { | |
* | ||
* expect({a: 1, b: 2, c: 3}).to.include({a: 1, b: 2}); | ||
* | ||
* When the target is a Set or WeakSet, `.include` asserts that the given `val` is a | ||
* member of the target. SameValueZero equality algorithm is used. | ||
* | ||
* expect(new Set([1, 2])).to.include(2); | ||
* | ||
* When the target is a Map, `.include` asserts that the given `val` is one of | ||
* the values of the target. SameValueZero equality algorithm is used. | ||
* | ||
* expect(new Map([['a', 1], ['b', 2]])).to.include(2); | ||
* | ||
* Because `.include` does different things based on the target's type, it's | ||
* important to check the target's type before using `.include`. See the `.a` | ||
* doc for info on testing a target's type. | ||
|
@@ -338,8 +348,8 @@ module.exports = function (chai, _) { | |
* | ||
* By default, strict (`===`) equality is used to compare array members and | ||
* object properties. Add `.deep` earlier in the chain to use deep equality | ||
* instead. See the `deep-eql` project page for info on the deep equality | ||
* algorithm: https://github.com/chaijs/deep-eql. | ||
* instead (WeakSet targets are not supported). See the `deep-eql` project | ||
* page for info on the deep equality algorithm: https://github.com/chaijs/deep-eql. | ||
* | ||
* // Target array deeply (but not strictly) includes `{a: 1}` | ||
* expect([{a: 1}]).to.deep.include({a: 1}); | ||
|
@@ -449,25 +459,24 @@ module.exports = function (chai, _) { | |
* @api public | ||
*/ | ||
|
||
function includeChainingBehavior () { | ||
flag(this, 'contains', true); | ||
function SameValueZero(a, b) { | ||
return (_.isNaN(a) && _.isNaN(b)) || a === b; | ||
} | ||
|
||
function isDeepIncluded (arr, val) { | ||
return arr.some(function (arrVal) { | ||
return _.eql(arrVal, val); | ||
}); | ||
function includeChainingBehavior () { | ||
flag(this, 'contains', true); | ||
} | ||
|
||
function include (val, msg) { | ||
if (msg) flag(this, 'message', msg); | ||
|
||
_.expectTypes(this, ['array', 'object', 'string']); | ||
_.expectTypes(this, [ | ||
'array', 'object', 'string', | ||
'map', 'set', 'weakset', | ||
]); | ||
|
||
var obj = flag(this, 'object') | ||
, objType = _.type(obj).toLowerCase() | ||
, isDeep = flag(this, 'deep') | ||
, descriptor = isDeep ? 'deep ' : ''; | ||
, objType = _.type(obj).toLowerCase(); | ||
|
||
// This block is for asserting a subset of properties in an object. | ||
if (objType === 'object') { | ||
|
@@ -504,10 +513,62 @@ module.exports = function (chai, _) { | |
return; | ||
} | ||
|
||
// Assert inclusion in an array or substring in a string. | ||
var isDeep = flag(this, 'deep') | ||
, descriptor = isDeep ? 'deep ' : '' | ||
, included = false; | ||
|
||
switch (objType) { | ||
case 'string': | ||
included = obj.indexOf(val) !== -1; | ||
break; | ||
|
||
case 'weakset': | ||
if (isDeep) { | ||
var flagMsg = flag(this, 'message') | ||
, ssfi = flag(this, 'ssfi'); | ||
flagMsg = flagMsg ? flagMsg + ': ' : ''; | ||
|
||
throw new AssertionError( | ||
flagMsg + 'unable to use .deep.include with WeakSet', | ||
undefined, | ||
ssfi | ||
); | ||
} | ||
|
||
included = obj.has(val); | ||
break; | ||
|
||
case 'map': | ||
var isEql = isDeep ? _.eql : SameValueZero; | ||
obj.forEach(function (item) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As @vieiralucas said, we might be able to optimize this by getting out of the loop when we found the included item. In order to support IE and older versions of it we can This is a cross-platform compatible implementation I've made on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lucasfcosta I think this is worth exploring. I'd prefer this kind of optimization technique be implemented in a separate PR as a refactor. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @meeber agreed. LGTM! Let's get this merged. |
||
included = included || isEql(item, val); | ||
}); | ||
break; | ||
|
||
case 'set': | ||
if (isDeep) { | ||
obj.forEach(function (item) { | ||
included = included || _.eql(item, val); | ||
}); | ||
} else { | ||
included = obj.has(val); | ||
} | ||
break; | ||
|
||
case 'array': | ||
if (isDeep) { | ||
included = obj.some(function (item) { | ||
return _.eql(item, val); | ||
}) | ||
} else { | ||
included = obj.indexOf(val) !== -1; | ||
} | ||
break; | ||
} | ||
|
||
// Assert inclusion in collection or substring in a string. | ||
this.assert( | ||
objType === 'string' || !isDeep ? ~obj.indexOf(val) | ||
: isDeepIncluded(obj, val) | ||
included | ||
, 'expected #{this} to ' + descriptor + 'include ' + _.inspect(val) | ||
, 'expected #{this} to not ' + descriptor + 'include ' + _.inspect(val)); | ||
} | ||
|
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 think it would be good to explain how the
SameValueZero
algorithm works. I think many people could get confused by that and would end up either having to read the code to understand it or just avoiding using this assertion because they're not sure of what it does.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.
@lucasfcosta Fair point. In this case, I would suggest moving the phrase "SameValueZero equality algorithm is used" from where it is currently down to the separate paragraph where
strict
anddeep
equality` is discussed. This would make the doc style more consistent anyway. And then rather than explain what "SameValueZero equality" is, we could provide a link to an explanation, just like we do with the "deep equal" algorithm.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.
We could just refer to is as it is more commonly known;
Object.is
or if you'd like to we coud keep the phrasing but link to the equality article on MDN which mentionsSameValueZero
.