Directive with replace fails to merge its attributes with its template's attributes (e.g. ngClass, style etc)

[dariocravero]

Say I have the a directive called myDirective which has the following template:

<div class="my-directive" ng-class="{'something': something}" ng-transclude><div>

Say I want to add more conditions to ng-class:

<my-directive ng-class="{'else': else}">my-directive</my-directive>

The expected result would be:

<div class="my-directive" ng-class="{'something': something, 'else': else}"><div>

However, Angular is producing this:

<div class="my-directive" ng-class="{'something': something} {'else': else}"><div>

Which of course causes an error since it isn't a valid expression.

My question is: should a directive that has an ng-class attribute merge it with whatever the caller send on its ng-class attribute?

I asked on IRC and @caitp suggested it might be an issue on the way Expressions are resolved and that I should wrote this issue.
Thanks! :)

[caitp]

oh no don't credit me, what if it turns out to be a terrible idea! but it's something that we might be able to improve if people want it.

To summarize, when merging the attributes from the compiled node into the template node, this can screw up ngClass or other directives expecting expressions.

I'm not totally sure how to fix this (I don't think it would be a super trivial fix), but down the road it might be good to try

[dariocravero]

Didn't mean it that way, sorry.

BTW, here's how I'm solving it right now (fiddle):

compile: function compile(tElement, tAttrs, transclude) {
  tAttrs.ngClass = tAttrs.ngClass.replace(/}\s*{/g, ', ');
  // ...
}

angular.module('app', []).directive('myDirective', ['$templateCache', function($templateCache) {
        return {
            restrict: 'E',
            transclude: true,
            replace: true,
            compile: function compile(tElement, tAttrs, transclude) {
                tAttrs.ngClass = tAttrs.ngClass.replace(/}\s*{/g, ', ');

                return {
                    post: function postLink(scope, iElement, iAttrs) {
                        scope.something = true;
                        scope.else = true;
                    }
                };
            },
            template: $templateCache.get('template.html')
        };
    }]);

It's hacky but it does the trick for now.

[gsklee]

Interesting issue, subscribing.

[jenslukowski]

I have a (failing) test case reproducing this issue but I wonder how to merge the attribute values in other cases. What would happen when you have a variable and an object?

[caitp]

yeah, unfortunately this is not a super easy thing :(

[jeffwhelpley]

Ah, @caitp this touches on the issue I ran into earlier. Understood that it is complex but hopefully someone can implement a solution for this.

[plumpNation]

@dariocravero Thanks for the quick fix suggestion.

[teropa]

There's an additional complication here, which is that if the directive uses isolate scopes or scope inheritance, the ng-class entries defined in the original element won't behave as expected if the expression is merged and then evaluated in the isolated directive scope.

Since replace is being deprecated, this probably isn't worth the trouble?

[caitp]

it's not worth the trouble --- you can't really merge the ng-class attributes correctly, we can't special case certain attributes because it could apply to any attribute/directive that takes an expression --- there's basically no real fix, other than finding a way to link the ngClass directive twice for the same element. (and we can't really generalize that to other directives --- at least not without extending the directive API :()

[KamBha]

@teropa, can you clarify the comment about replace being deprecated? What should we be using instead?