Filter behavior change in 1.3.6

[pvenable]

Plunker: http://plnkr.co/edit/HKQwBiVxFh6bxXzP12bB?p=preview

It looks like something changed with filtering by an object expression in 1.3.6. In the example above no items in the list render until you type in the input bound to the fooFilter model. If you change back to angular 1.3.5 you'll see that the list of items render prior to any user interaction.

Workaround is to initialize fooFilter to empty string.

[gkalpak]

This had been brought up in #9757 (example #9757 (comment), #9757 (comment) and #9757 (comment)) and it was intentionally implemented like this.

Note that a better approach to filtering imo (and one that does work as expected even in 1.3.6) is: http://plnkr.co/edit/6FLGXYBk0WP7P47MegmG?p=preview

But, I can imagine some people wanting this to match pre 1.3.6 behaviour, right @caitp 😄

[andrewlague]

The fact remains that this is a change of behavior in a minor release--perhaps this should be documented.

[caitp]

this is why i really wish we actually reviewed that big refactoring --- tests were passing, but it changed a bunch of things that people actually depended on, it was just too hard to spot the differences :\ we do need to change itb ack

[gkalpak]

There is already a PR for fixing this, so...

[gkalpak]

The problem with filterFilter is that it's behaviour is largely undocumented (there are a lot of possible cases and only a small fraction covered in the docs), the comparator was mistreated (in many cases the expression object played the role of the comparator) and the test coverage left several cases out. (Besides, of course, not working as expected with "deep" objects (either as items or as expression objects).)

So, basically, there was no "official" way to determine how exactly it works and people depended on behaviours often disvovered by trial and error (and then sticking to what worked).

Thus, there was no reasonable way to determine exactly what would behave differently (afaik).

As I have said before, in order to fix something broken, you have to "break its broken-ness". And that is what it takes to fix filterFilter 😞

I know; we don't want breaking changes between minor versions (by which we actually mean "we don't want fixing broken behaviour that many people are likely to depend on").
(And I do understand this btw; I am just not thrilled about it.)

---

@caitp: Did you mean change the undefined behaviour back or revert the whole refactoring ?

[caitp]

Change the undefined case back --- it looks like a pretty small change based on the PR.

Re: things not being tested, we should really have some test coverage reports happening :\ but it's hard since we have like 5 different test harnesses

[caseyhoward]

@gkalpak: The only change that was needed was the one on line 199. I tried writing the following test for the other change you had and it already passed:

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

I also merged the tests into one "it" block and made the test for deep objects slightly more thorough.