Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(filterFilter): let expression object {$: '...'} also match primitive items #10437

Closed
wants to merge 1 commit into from
Closed
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
4 changes: 4 additions & 0 deletions src/ng/filter/filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ function filterFilter() {

// Helper functions for `filterFilter`
function createPredicateFn(expression, comparator, matchAgainstAnyProp) {
var shouldMatchPrimitives = isObject(expression) && ('$' in expression);
var predicateFn;

if (comparator === true) {
Expand All @@ -163,6 +164,9 @@ function createPredicateFn(expression, comparator, matchAgainstAnyProp) {
}

predicateFn = function(item) {
if (shouldMatchPrimitives && !isObject(item)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Just avoiding having to execute isObject(expression) && ('$' in expression) for every item in the array.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fine

return deepCompare(item, expression.$, comparator, false);
}
return deepCompare(item, expression, comparator, matchAgainstAnyProp);
};

Expand Down
27 changes: 27 additions & 0 deletions test/ng/filter/filterSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,33 @@ describe('Filter: filter', function() {
});


it('should match primitive array values against top-level `$` property in object expression',
function() {
var items, expr;

items = ['something', 'something else', 'another thing'];
expr = {$: 'some'};
expect(filter(items, expr).length).toBe(2);
expect(filter(items, expr)).toEqual([items[0], items[1]]);

items = [{val: 'something'}, {val: 'something else'}, {val: 'another thing'}];
expr = {$: 'some'};
expect(filter(items, expr).length).toBe(2);
expect(filter(items, expr)).toEqual([items[0], items[1]]);

items = [123, 456, 789];
expr = {$: 1};
expect(filter(items, expr).length).toBe(1);
expect(filter(items, expr)).toEqual([items[0]]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you've brought it up, lets add a test case showing that other properties in the expression object are ignored when comparing against primitives (if the object has a $ property)

Copy link
Member Author

Choose a reason for hiding this comment

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

Added the test below.


items = [true, false, 'true'];
expr = {$: true, ignored: 'false'};
expect(filter(items, expr).length).toBe(2);
expect(filter(items, expr)).toEqual([items[0], items[2]]);
}
);


it('should take object as predicate', function() {
var items = [{first: 'misko', last: 'hevery'},
{first: 'adam', last: 'abrons'}];
Expand Down