Fix ngClass mixed array/object when object instance does not changes

[drpicox]

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...) bugfix

What is the current behavior? (You can also link to an open issue here): when the expression is [{}] it does not tracks changes inside the object

  it('should support changing multiple classes via variable array mixed with conditionally via a map', inject(function($rootScope, $compile) {
    $rootScope.classVar = ['', {orange: true}];
    element = $compile('<div class="existing" ng-class="classVar"></div>')($rootScope);
    $rootScope.$digest();

    $rootScope.classVar[1].orange = false;
    $rootScope.$digest();

    expect(element.hasClass('orange')).toBeFalsy(); /// <<--- this fails before this patch
  }));

What is the new behavior (if this is a feature change)?: now it tracks changes correctly

Does this PR introduce a breaking change?: nop

Please check if the PR fulfills these requirements

• The commit message follows our guidelines: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit-message-format
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

Other information:

In this plnkr you can see the failure:

• http://plnkr.co/edit/f0V9RVDGREetSW00TTRZ?p=preview

#14404 also fixes this problem by doing a refactor. (it tries to be a performance optimization)
This PR can be combined into #14394 if you ask.

[drpicox]

As I commented in #14394 there is this case that are not resolved here and it has two solutions. The problem:

  // this test actually passes
  it('should support mixed nested array and object leafs variable', inject(function($rootScope, $compile) {
    $rootScope.classVar = ['red', ['green', {blue: true}]];
    element = $compile('<div class="existing" ng-class="classVar"></div>')($rootScope);
    $rootScope.$digest();

    expect(element.hasClass('red')).toBeTruthy();
    expect(element.hasClass('green')).toBeTruthy();
    expect(element.hasClass('blue')).toBeTruthy();
  }));

  // this test fails even with this patch
  it('should support mixed nested array and object leafs variable with mutating object', inject(function($rootScope, $compile) {
    $rootScope.classVar = ['red', ['green', {blue: true}]];
    element = $compile('<div class="existing" ng-class="classVar"></div>')($rootScope);
    $rootScope.$digest();

    $rootScope.classVar[1][1].blue = false;

    expect(element.hasClass('red')).toBeTruthy();
    expect(element.hasClass('green')).toBeTruthy();
    expect(element.hasClass('blue')).toBeFalsy();
  }));

Solutions:

1. Do not accept nested array syntax: it would be "breaking changes" so any current implementation that it is using it. But it is unlikely because it is a not documented behaviour and the tracking of changes is not working properly. It have to be checked what happens in Angular2, because Angular2 uses this concept of nested array to express Injection and Component dependencies it is possible that feels comfortable for the programmer having nested arrays.
2. Deep solve change tracking only for arrays: it will not break the implementation and only will cost nothing (except when using nested arrays because the checking would be done properly). In such case the implementation of perf(ngClass): refactor to optimize the case of static map of classes with large objects #14404 is already solving this.

¿Which do you consider more acceptable?

[gkalpak]

The docs are very explicit about what types of expressions are supported in ngClass. Nested arrays are not supported, so if anyone uses them, they are out of luck imho.

According to the docs, the expression used with ngClass should evaluate to one of 3 things:

1. A string.
2. An object.
3. An array containing strings and/or objects.

(Where objects are treated as key-value pairs, with keys denoting classes and values determining the presense or absense of the corresponding key/class.)

[gkalpak]

Pleasd wrap lines at 100 chars.

[gkalpak]

I left a couple of minor comments wrt the test, but LGTM otherwise. 👍
Feel free to ping me once you address them.

[drpicox]

@gkalpak I think that is now right.
Just ask and I'll squash all commits in one.

[gkalpak]

It's OK, I'll squash while merging. Thx 👍