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

fix(filterFilter): Fix filtering using an object expression when the filter value is undefined #10424

Closed
wants to merge 1 commit into from
Closed

fix(filterFilter): Fix filtering using an object expression when the filter value is undefined #10424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/ng/filter/filter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ function deepCompare(actual, expected, comparator, matchAgainstAnyProp) {
} else if (expectedType === 'object') {
for (key in expected) {
var expectedVal = expected[key];
if (isFunction(expectedVal)) {
if (isFunction(expectedVal) || isUndefined(expectedVal)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This broke things in our project. It changed the behaviour of $filter. I don't think it is OK to change the behaviour in minor releases.

Copy link
Member

Choose a reason for hiding this comment

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

@zuzusik I was under the impression that c62fa6b was only merged to 1.4.x beta so a release where we've got breaking changes. Do you see it on any of the 1.3.x branches as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

@pkozlowski-opensource It was successfully landed to 1.3.15, look at the changelog

Copy link
Contributor

Choose a reason for hiding this comment

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

@pkozlowski-opensource here it is: 63b9956

Copy link
Contributor

Choose a reason for hiding this comment

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

@zuzusik - This change was actually fixing a regression that occurred between 1.3.5 and 1.3.6 (see #10419). I am sorry that it took so long for us to fix this regression that you started using the filter in its regressed state.

continue;
}

Expand Down
9 changes: 9 additions & 0 deletions test/ng/filter/filterSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ describe('Filter: filter', function() {
});


it('should filter when a specific property is undefined', function() {
Copy link
Member

Choose a reason for hiding this comment

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

@caitp: A better description suggestion ?

Maybe 'should ignore undefined properties of the expression object` ?

var items = [{name: 'a'}, {name: 'abc'}];
expect(filter(items, {name: undefined})).toEqual([{name: 'a'}, {name: 'abc'}]);

items = [{first: 'misko'}, {deep: {first: 'misko'}}, {deep: {last: 'hevery'}}];
expect(filter(items, {deep: {first: undefined}})).toEqual([{deep: {first: 'misko'}}, {deep: {last: 'hevery'}}]);
});


it('should take function as predicate', function() {
var items = [{name: 'a'}, {name: 'abc', done: true}];
expect(filter(items, function(i) {return i.done;}).length).toBe(1);
Expand Down