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

Matches keeps an internal reference to spec object #1730

Merged
merged 2 commits into from
Jul 9, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions test/objects.js
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,9 @@
equal(_.matches({hair: true})(moe), true, 'Returns a boolean');
equal(_.matches({hair: true})(curly), false, 'Returns a boolean');

equal(_.matches({__x__: undefined})(5), false, 'can match undefined props on primitives');
equal(_.matches({__x__: undefined})({__x__: undefined}), true, 'can match undefined props');

equal(_.matches({})(null), true, 'Empty spec called with null object returns true');
equal(_.matches({a: 1})(null), false, 'Non-empty spec called with null object returns false');

Expand Down Expand Up @@ -690,6 +693,20 @@

Prototest.x = 5;
ok(_.matches(Prototest)({x: 5, y: 1}), 'spec can be a function');

// #1729
var o = {'b': 1};
var m = _.matches(o);

equal(m({'b': 1}), true);
o.b = 2;
o.a = 1;
equal(m({'b': 1}), true, 'changing spec object doesnt change matches result');


//null edge cases
var oCon = _.matches({'constructor': Object});
deepEqual(_.map([null, undefined, 5, {}], oCon), [false, false, false, true], 'doesnt fasley match constructor on undefined/null');
});

}());
9 changes: 5 additions & 4 deletions underscore.js
Original file line number Diff line number Diff line change
Expand Up @@ -1178,13 +1178,14 @@

// Returns a predicate for checking whether an object has a given set of `key:value` pairs.
_.matches = function(attrs) {
var keys = _.keys(attrs), length = keys.length;
var pairs = _.pairs(attrs), length = pairs.length;
return function(obj) {
if (obj === attrs) return true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How common is obj === attrs really? I assume not so very common so seems like an extraneous check.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's open a separate PR for that.

if (obj == null) return !length;
obj = Object(obj);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

obj = Object(obj); means that null will be treated as an object so that means:

var m = _.matches({ 'constructor': Object });
m(null)// -> true, which is not right

for (var i = 0, key; i < length; i++) {
key = keys[i];
if (!(key in obj) || attrs[key] !== obj[key]) return false;
for (var i = 0; i < length; i++) {
var pair = pairs[i], key = pair[0];
if (pair[1] !== obj[key] || !(key in obj)) return false;
}
return true;
};
Expand Down