Filter behavior change in 1.3.6

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.

fix(filterFilter): Fix filtering using an object expression when the …

@caitp

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 😄

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

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

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

@caitp

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 ?

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

fix(filterFilter): Fix filtering using an object expression when the …

@gkalpak

@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.

fix(filterFilter): Fix filtering using an object expression when the …

@caseyhoward

@caseyhoward: Indeed the first change wasn't necessary, because the case typeof expression === 'undefined' is handled by the default block in the switch statement. So, I guess we can leave it out :)

I'm not so sure if this is the right place, but I also found another strange behavior. It worked in 1.3.5 but it's broken in 1.3.6.

In this plunkr you'll see version 1.3.5, were you can select a type in the first select box. The options of the second selection are filtered based on the selection of the first one.
Strange for 1.3.6 is, that there is the whole selection available for the second dropdown if nothing is selected, but as soon as you select a type the second list is empty.

@rapmue

@rapmue: This is because of a regression in 1.3.6 and will be fixed by #10408.

#10408 landed

@caitp

@caitp @lgalfaso this issue was closed on Dec 17th, but is still reproducible in 1.3.8 and 1.3.12:

Can we please re-open?

@cspeper

@cspeper could you please open a new issue with a clear reproduce scenario using plunker or similar?

fix(filterFilter): Fix filtering using an object expression when the …

Navigation Menu

Repository navigation

Filter behavior change in 1.3.6 #10419

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Filter behavior change in 1.3.6 #10419

Description

Activity

gkalpak commented on Dec 11, 2014

andrewlague commented on Dec 11, 2014

caitp commented on Dec 11, 2014

gkalpak commented on Dec 11, 2014

gkalpak commented on Dec 11, 2014

caitp commented on Dec 11, 2014

caseyhoward commented on Dec 12, 2014

gkalpak commented on Dec 12, 2014

rapmue commented on Dec 15, 2014

gkalpak commented on Dec 15, 2014

lgalfaso commented on Dec 17, 2014

cspeper commented on Feb 10, 2015

pkozlowski-opensource commented on Feb 11, 2015

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions