ng-repeat filter behavior

[danmindru]

Updated from 1.3.2 to 1.3.6 and one of my ng-repeat filters broke.
The solution was to change item in items | filter:search to item in items | filter:search.$.
Couldn't find it in the changelog. Am I missing something?

See Plunker

[caitp]

This is a regression, Fix is already in the works. Would get the issue # but on mobile

[caitp]

So yeah, I think this is a mix of two problems, tbh. @gkalpak, using a build of angular with a mix of your fix and @caseyhoward's fix, the second case is still broken. I think we should use this as a test case and fix it up in one of your two PRs

[gkalpak]

@caitp: Making this works mean matching item 'some string' against expression {$: 'some string'}.

{$: 'some string'} is documented to mean "accept a match against any property of the object". The {...} expression format is intended for matching against object items (which have properties), not against strings.

IMO, opinion, it makes absolutely no sense to have an object expression match against primitive types.

WDYT ?

[caitp]

I think if people depended on it, we should fix it

[gkalpak]

@caitp: Been wondering:
filterFilter used to work incorrectly and in many cases differently than documented (e.g. this issue).
#9757 was about making it right (i.e. make it work as documented for cases that were documented and make it work reasonably and consistently across nesting levels for undocumented cases).

Unavoidably, people have been depending on the broken behaviour of filterFilter. We can't fix it and retain full backward compatibility at the same time. So, if the plan is to revert to the broken behaviour piece by piece, it might make more sense to revert #9757 altogether (and maybe land it on 1.4 or something).

---

Regarding this particular case:

If {$: 'xyz'} should match 'xyz', doesn't it mean that {level1: {$: 'xyz'}} should match {level1: 'xyz'} and so on ? This seems really weird (even if people depend on it 😄).

[caitp]

I think the top-level $ could apply to primitive values, since it used to

[gkalpak]

So, you say we should have a top-level {$: 'xyx'} also match primitive values, but a non-top-level {$: 'xyz'} only match objects ?

_{Sure, makes sense ! Then we only need to break deep object filtering and we'll have our nice pre-1.3.6 implementation (with a little more readable code maybe) !}

[caitp]

I don't think we need to break deep object filtering, we just need to unbreak peoples applications. please stop misreading this as an attack on your patch --- it was very hard to review that thing, and we obviously were missing test cases so it wasn't obvious that we were breaking things. But we need to get peoples code working again.

You can add comments like // TODO(gkalpak): remove this line in 1.4 so that we know to get rid of it later, when we CAN make breaking changes.

[gkalpak]

Sorry, if I sound like this. I don't mean to 😇 I certainly don't read anything as an attack on my patch.

I'll submit a fix soon.