Global watch is frequently triggered

[liggitt]

This commit added a global watch: https://github.com/hawtio/hawtio-core-navigation/commit/5982cce6541893d12213a4f8847f5712b926668a#diff-69530d95a0ae7ea0332b379468aa2fcbL661

This watch gets invoked any time any digest loop occurs. Angular triggers those in response to many things, such as $http data callbacks, $interval callbacks, etc. We're using $interval callbacks to update timestamp elements on the page.

Adding a relatively expensive method (fans out to selected(), serializes to JSON, etc) in a function that can get called multiple times any time a network request or interval fires is a problem.

[liggitt]

/cc @spadgett

[spadgett]

@liggitt I'm seeing the problem with earlier versions as well. There's another similar $watch later in the file. I'm still isolating when it happens.

[liggitt]

I think the later watch is not global, but local to the scope of a particular directive ($scope.$watch vs scope.$watch). I only saw the referenced one triggered continually

[gashcrumb]

For your timestamps why not implement a directive that just uses setInterval() so you can then limit the update to the immediate scope of the directive, rather than relying on $interval() doing a $rootScope.$apply()?

[spadgett]

I'm seeing the behavior with 2.0.33.

In 2.0.48, both watch listeners are called continuously in my testing.

[liggitt]

We're actually updating DOM elements directly so we don't need a scope apply in our $interval call, so I can turn that off and fix our particular issue. I still think it's a bad idea for the nav component to watch globally. Normally, the nav should only update on route changes. There is already a way to trigger the nav to redraw, and it seems like consumers that want that to fire on every scope change should opt into that.

[gashcrumb]

Yeah, there is, though we kinda want the nav to react to changes in the app. Will have to think about limiting redraws, it may mean we'd have to update a fair bit of code to trigger that redraw manually.

That being said I'll see what I can do to try and limit the number of times we evaluate the tabs.

[liggitt]

I'd rather have the component default to only auto-recalculating on route changes, and have an opt-in way to enable watching for in-page changes (e.g. HawtioNav.redrawOnScopeChange(true), etc)

[gashcrumb]

As there's existing code that would need to be changed I'm not ready to make that change yet. I've simplified the logic a bit and have debounced both of those watches so we don't evaluate the selected tabs as much, have pushed a new release (2.0.49) so you can give it a try and see if it helps.

[liggitt]

Updated title, since the constant triggering was in our component, not hawtio nav

[abkieling]

@liggitt I'll close this issue if you are OK with it