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

FilterFilter nested expression support renders strict option useless #7323

Closed
shineli-not-used-anymore opened this issue May 1, 2014 · 3 comments

Comments

@shineli-not-used-anymore

Since https://github.com/angular/angular.js/pull/6215/files#r9626594, passing true to strict in the filterFilter is no longer working when nested predicate object is used to filter nested properties of an object.

<div ng-repeat="it in items | filter:{ address: { country: 'Canuckistan'} }"></div>

is working

<div ng-repeat="it in items | filter:{ address: { country: 'Canuckistan'} }:true"></div>

is not.

@Narretz Narretz added this to the 1.3.0 milestone Jun 23, 2014
@albertboada
Copy link

Would it work as you expect with a simple POJO predicate (not nested) like this one below, assuming it wasn't nested either?

<div ng-repeat="it in items | filter:{ country: 'Canuckistan' }:true"></div>

I don't think this would work as you expect, either. This is because (I think) nested objects are not the ones to blame here.

The strict comparator compares the whole it object with the predicate object. Im sure
it !== { address: { country: 'Canuckistan'} } in your application (it sure has more fields, so the objects are not equal). This is why the filter doesn't work as you expect. https://github.com/angular/angular.js/blob/master/src/ng/filter/filter.js#L134

Should this continue this way? Should this logic be changed for a more intuitive approach? I say you better write your own comparator for the moment.

@shineli-not-used-anymore
Copy link
Author

Hey @albertboada ,

I agree with you that the bottom of the problem is not the nested objects. Apparently, I have different interpretation of what the strict means when it comes to playing with nested predicate.

BTW, <div ng-repeat="it in items | filter:{ country: 'Canuckistan' }:true"></div> had been working before the nested predicate was introduced.

So I went back to the doc https://docs.angularjs.org/api/ng/filter/filter. the expression parameter is described as:

The predicate to be used for selecting items from array.

And the comparator is as:

is used in determining if the expected value (from the filter expression) and actual value (from the object in the array) should be considered a match

While I understand it !== { address: { country: 'Canuckistan'} }, should the condition to be predicate(it) === 'Canuckistan' ---pseud code when strict is true since the purpose of predicate is to select the value from it instead of being the value to be compared to.

From the source code I can see that when strict is true, the code to handle the nested predicate is not effective since it is in the comparator not predicate. The author of that pull request @caitp left a comment:

It would be nicer to add these nested checks as predicates to avoid a stack overflow, but this would be a much more complicated fix

If my understanding is correct, the existence of this bug is because the nested predicate support is done in the comparator instead of predicate.

I indeed wrote my own filter to handle this. However, IMHO, the code to handle nested predicate should be in the predicate not comparator.

@btford btford removed the gh: issue label Aug 20, 2014
@albertboada
Copy link

Not sure why I'm coming back to this, but, some months later, I absolutely agree the strict option on object predicates should check for exact properties values, instead of equality of the whole predicate object. It is scary, tho, that not even @caitp wanted to go thru all this when she did the tweaks. I would not know where to start, but +1 👍.

gkalpak added a commit to gkalpak/angular.js that referenced this issue Nov 18, 2014
Previously, trying to use a deep expression object (i.e. an object whose
properties can be objects themselves) did not work correctly.
This commit refactors `filterFilter`, making it simpler and adding support
for filtering collections of arbitrarily deep objects.

Closes angular#7323
Closes angular#9698
gkalpak added a commit to gkalpak/angular.js that referenced this issue Nov 19, 2014
Previously, trying to use a deep expression object (i.e. an object whose
properties can be objects themselves) did not work correctly.
This commit refactors `filterFilter`, making it simpler and adding support
for filtering collections of arbitrarily deep objects.

Closes angular#7323
Closes angular#9698
gkalpak added a commit to gkalpak/angular.js that referenced this issue Nov 21, 2014
Previously, trying to use a deep expression object (i.e. an object whose
properties can be objects themselves) did not work correctly.
This commit refactors `filterFilter`, making it simpler and adding support
for filtering collections of arbitrarily deep objects.

Closes angular#7323
Closes angular#9698
gkalpak added a commit to gkalpak/angular.js that referenced this issue Nov 23, 2014
Previously, trying to use a deep expression object (i.e. an object whose
properties can be objects themselves) did not work correctly.
This commit refactors `filterFilter`, making it simpler and adding support
for filtering collections of arbitrarily deep objects.

Closes angular#7323
Closes angular#9698
gkalpak added a commit to gkalpak/angular.js that referenced this issue Nov 24, 2014
Previously, trying to use a deep expression object (i.e. an object whose
properties can be objects themselves) did not work correctly.
This commit refactors `filterFilter`, making it simpler and adding support
for filtering collections of arbitrarily deep objects.

Closes angular#7323
Closes angular#9698
gkalpak added a commit to gkalpak/angular.js that referenced this issue Nov 27, 2014
Previously, trying to use a deep expression object (i.e. an object whose
properties can be objects themselves) did not work correctly.
This commit refactors `filterFilter`, making it simpler and adding support
for filtering collections of arbitrarily deep objects.

Closes angular#7323
Closes angular#9698
gkalpak added a commit to gkalpak/angular.js that referenced this issue Dec 1, 2014
Previously, trying to use a deep expression object (i.e. an object whose
properties can be objects themselves) did not work correctly.
This commit refactors `filterFilter`, making it simpler and adding support
for filtering collections of arbitrarily deep objects.

Closes angular#7323
Closes angular#9698
gkalpak added a commit to gkalpak/angular.js that referenced this issue Dec 1, 2014
Previously, trying to use a deep expression object (i.e. an object whose
properties can be objects themselves) did not work correctly.
This commit refactors `filterFilter`, making it simpler and adding support
for filtering collections of arbitrarily deep objects.

Closes angular#7323
Closes angular#9698
gkalpak added a commit to gkalpak/angular.js that referenced this issue Dec 2, 2014
Previously, trying to use a deep expression object (i.e. an object whose
properties can be objects themselves) did not work correctly.
This commit refactors `filterFilter`, making it simpler and adding support
for filtering collections of arbitrarily deep objects.

Closes angular#7323
Closes angular#9698
@gkalpak gkalpak closed this as completed in f7cf846 Dec 2, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants