wix-incubator / angular-tree-control

Angular JS Tree
http://wix.github.io/angular-tree-control
MIT License
708 stars 276 forks source link

Broken when used with custom interpolation symbols #264

Open slavafomin opened 7 years ago

slavafomin commented 7 years ago

Hello!

Thank you for this great library!

However, when it's used in projects with custom interpolation symbols it's not working (Angular is throwing expression syntax exceptions).

For example, we use the following interpolation symbols to avoid conflicts with Twig templating engine which we use server-side:

It's not safe to rely on default interpolation symbols when developing a library, because they can be easily overridden in end-developer's project. It's better to rely on ngBind directive.

This Plunk demonstrates the issue: https://plnkr.co/edit/NXnu2P5FLzcjYn2OjKNe?p=preview

Thanks!

slavafomin commented 7 years ago

As I see here: https://github.com/wix/angular-tree-control/blob/master/angular-tree-control.js#L294-L305.

You are using Angular's $compile service to pre-compile the template according to the specified options. This is the source of this bug.

Here's the very simple "naive" solution that works:

if(!template) {
    var open = $interpolate.startSymbol();
    var close = $interpolate.endSymbol();
    template =
        '<ul ' + open + 'options.ulClass' + close + ' >' +
        '<li ng-repeat="node in node.' + open + 'options.nodeChildren' + close + ' | filter:filterExpression:filterComparator ' + open + 'options.orderBy' + close + '" ng-class="headClass(node)" ' + open + 'options.liClass' + close + '' +
        'set-node-to-data>' +
        '<i class="tree-branch-head" ng-class="iBranchClass()" ng-click="selectNodeHead(node)"></i>' +
        '<i class="tree-leaf-head ' + open + 'options.iLeafClass' + close + '"></i>' +
        '<div class="tree-label ' + open + 'options.labelClass' + close + '" ng-class="[selectedClass(), unselectableClass()]" ng-click="selectNodeLabel(node)" tree-transclude></div>' +
        '<treeitem ng-if="nodeExpanded()"></treeitem>' +
        '</li>' +
        '</ul>';
}

However, is it really necessary to use $compile for such a trivial purpose? I think the better approach would be to just concatenate template source with options using vanilla JavaScript. Actually this will improve performance of this operation, because template compilation is a complex and slow process.

slavafomin commented 7 years ago

I've created a PR which fixes this issue. It contains the code from my example above and takes into consideration effective interpolation symbols. I believe this is the best approach to preserve backward-compatibility for developers who use custom templates (i.e. templateUrl).

slavafomin commented 7 years ago

I've updated the PR with cleaner solution:

if(!template) {
    template =
        '<ul {{options.ulClass}} >' +
        '<li ng-repeat="node in node.{{options.nodeChildren}} | filter:filterExpression:filterComparator {{options.orderBy}}" ng-class="headClass(node)" {{options.liClass}}' +
        'set-node-to-data>' +
        '<i class="tree-branch-head" ng-class="iBranchClass()" ng-click="selectNodeHead(node)"></i>' +
        '<i class="tree-leaf-head {{options.iLeafClass}}"></i>' +
        '<div class="tree-label {{options.labelClass}}" ng-class="[selectedClass(), unselectableClass()]" ng-click="selectNodeLabel(node)" tree-transclude></div>' +
        '<treeitem ng-if="nodeExpanded()"></treeitem>' +
        '</li>' +
        '</ul>';
    template = template
        .replace(/{{/g, $interpolate.startSymbol())
        .replace(/}}/g, $interpolate.endSymbol())
    ;
}

The following addition to the original code should do the trick:

template = template
    .replace(/{{/g, $interpolate.startSymbol())
    .replace(/}}/g, $interpolate.endSymbol())
;
lie-nielsen commented 7 years ago

I had same problem. This looks good. Why no merge.

slavafomin commented 7 years ago

We should ask @yoavaa ;)