Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui-sref-active causes a lot of extra digest cycles #2777

Closed
Ralf9977 opened this issue May 27, 2016 · 6 comments
Closed

ui-sref-active causes a lot of extra digest cycles #2777

Ralf9977 opened this issue May 27, 2016 · 6 comments
Labels
Milestone

Comments

@Ralf9977
Copy link

The $timeout in ui-sref-active directive causes a lot extra digest cycles. In our case 12!

function addClass(el, className) { $timeout(function () { el.addClass(className); }); }

Do we really need $timeout? At least removeClass does not use $timeout.

@christopherthielen
Copy link
Contributor

The $timeout is apparently a fix for #1997

@eddiemonge
Copy link
Contributor

Do you have 12 elements/classes that are being applied? I wonder if there is a way to queue them all to fire at the same time but I'm not sure. Also, I think the digest cycles might happen anyway without the timeout?

@Ralf9977
Copy link
Author

Ralf9977 commented Jun 3, 2016

Thanks for quick response!

I've created a Plunker with a very simple example:
https://plnkr.co/edit/7bM2fzRcvHK05CIE0nlD?p=preview

Just open the browser console and click on a, b, c or d. 6 Digest cycles are performed. 4 because of the timeout.

If you replace
function addClass(el, className) { $timeout(function () { console.log("timeout"); el.addClass(className); }); }

by
function addClass(el, className) { el.addClass(className); }

in ui-router.js only 2 cycles are necessary.

@christopherthielen
Copy link
Contributor

Do we really need $timeout? At least removeClass does not use $timeout.

I think we do really need $timeout. Removing it will cause a regression for a very common use case, ng-repeat over a list of ui-sref-active and ui-sref.

@christopherthielen christopherthielen added this to the 1.0.0-final milestone Oct 13, 2016
@christopherthielen
Copy link
Contributor

I think we can use applyAsync when available, and fall back to $timeout otherwise.
Targeting 0.3.2 and 1.0 final

@christopherthielen christopherthielen modified the milestones: 1.0.0-beta.4, 1.0.0-final Nov 5, 2016
@christopherthielen
Copy link
Contributor

Fixed by using $applyAsync
https://plnkr.co/edit/UbLQjuxS47YgR0OScpgK?p=preview

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants