Isolated scope property for directive is being re-digested in version 1.4.1

[kikhtenko]

Hi,

the issue is not reproducible in 1.4.0 and appears only in 1.4.1 as you can see in the link below
http://plnkr.co/edit/kbIJflj4sL5kcCcYPzw7?p=preview

The problem is when scope property is changed in directive's controller then in the next digest cycle it's being rewritten.

Regards,
Andrew
@kikhtenko

[Nickproger]

+1

[vovashevchenko]

+1

[soutlan]

+1

[pbiryukov]

+1

[gkalpak]

It's probably a6339d3.

[Narretz]

Probably. Also doesn't happen with =

[asadovnikov]

+1

[petebacondarwin]

This issues is reopened as the fix was reverted.

[caitp]

@petebacondarwin missing a link/reason for the revert

[Narretz]

@caitp #12259

(If material relied on buggy behavior, we should fix it there rather than in core)

[gkalpak]

+1 @Narretz
It seems quite bizarre that we reverted a fix to a (rather serious imo) bug/regression, because the fix broke an external module.

Is anyone looking into why ngMaterial broke with the fix ?

[caitp]

I was looking into it earlier, but I can't get any tests to fail when the angular module has the revert reverted, and i couldn't find any demos that behaved weirdly (under tabs or autocomplete)

[caitp]

I'm not actually sure where the material tests are, because the file in the test directory just has a bunch of matchers, no tests

[caitp]

nvm, found em :> there's a ddescribe() in there preventing tests from running, sending pr

[petebacondarwin]

Thanks for looking into this everyone. I agree if material relied on buggy behaviour then it should be fixed there.

[petebacondarwin]

But since angular-material is a first class citizen of the Angular team then we ought to have a process in place to run angular-material tests before we do a release.

[caitp]

The main problem I'm seeing is that they're using =? isolate scope bindings to represent boolean attributes, which doesn't make a lot of sense. It doesn't die in the reverted version because the reverted version ignores expressions valued at the empty string, if they're optional (which we can probably still do)

[caitp]

All of the angular-material tests pass for me (with the revert reverted) with the following change:

           case '=':
             if (!hasOwnProperty.call(attrs, attrName)) {
               if (optional) break;
               attrs[attrName] = void 0;
             }
+            if (optional && isString(attrs[attrName]) && !attrs[attrName].trim()) {
+              break;
+            }

I don't know if there are still other problems, but if there are, they aren't causing test failures (there are 2 disabled tests in the tree still)