angular / material

Material design for AngularJS
https://material.angularjs.org/
MIT License
16.55k stars 3.39k forks source link

mdTabs: always recompiling template on tab switch (scope/watchers leak) #3692

Closed andreialecu closed 9 years ago

andreialecu commented 9 years ago

I'm seeing a huge scope leak when switching through tabs with watchers on them.

I was able to track it down to this: https://github.com/angular/material/blob/f5959cc2e1d3a960c8df344669731c1a16c3e617/src/components/tabs/js/templateDirective.js#L35 Always being called on switching the tab, and the whole directive re-running from the beginning. The disconnect() function is never called. This seems to create a new scope and compile the template every single time.

I suspect it is related to the performance related change that was made recently, where tab content is ng-if'd out when not active, but I'm not sure.

Related: https://github.com/angular/material/issues/3508

The repro there can also be used to reproduce this problem.

andreialecu commented 9 years ago

I found a fix for this, at: https://github.com/angular/material/blob/f5959cc2e1d3a960c8df344669731c1a16c3e617/src/components/tabs/js/templateDirective.js#L28

Needs to be changed to:

      scope.$on('$destroy', function(){
        compileScope.$destroy();
      });

Can someone familiar (@robertmesserle) with that code explain why the $destroy watcher would reconnect the scope?

I see no unintended sideffects with the above code in my scenario, but I'm unsure what the intention was there.

piyushchauhan2011 commented 9 years ago

:+1:

ThomasBurleson commented 9 years ago

@anderson-arlen - Have you tried to using the mdNoDisconnect Tabs attribute:

<md-tabs md-no-disconnect>
  ...
</md-tabs>

And does this problem still exist with the latest code from master ?

Please provide a CodePen or Plunkr that demonstrates this issue. Here are some starter demo templates that you can use/fork:

May be related to #3617 and #3508

anderson-arlen commented 9 years ago

@andreialecu, I think @ThomasBurleson meant to tag you instead of me. Pesky autocomplete... :D