gsklee / ngStorage

localStorage and sessionStorage done right for AngularJS.
MIT License
2.34k stars 461 forks source link

debounce watcher causing heavy load in ngUpgrade apps #258

Open sime opened 7 years ago

sime commented 7 years ago

The $rootScope.$watch() causes a high load in ngUpgraded apps (2.4.x).

When I comment out the following block, the DevTools timeline shows next to no activity.

https://github.com/gsklee/ngStorage/blob/c217d5bb01b4a79bb389d05a83513c89b5d34bf9/ngStorage.js#L206-L208

bluegaspode commented 6 years ago

Personally I think this is related to: https://github.com/angular/angular/pull/13540#issuecomment-267846406

The upgrade-adapter runs an angularjs digest cycle, when microtasks are empty. Unfortunately this lines of code above regularly start a new timeout - which in turn starts the digest cycle again.

bluegaspode commented 6 years ago

This describes the problem even better, but still unresolved: https://github.com/angular/angular/pull/15157

meDavid commented 4 years ago

The fix would be to give the $watch function a watchExpression as a first argument and a listener as a second. this would remove the need of the timeout as well:

$rootScope.$watch(function () {return $storage}, function () {
                  $storage.$apply();
                });
Eugeny commented 2 years ago

Here's my shitty workaround:

diff --git a/node_modules/ngstorage/ngStorage.js b/node_modules/ngstorage/ngStorage.js
index 0bbdb69..15d138e 100644
--- a/node_modules/ngstorage/ngStorage.js
+++ b/node_modules/ngstorage/ngStorage.js
@@ -202,8 +202,10 @@

                 _last$storage = angular.copy($storage);

+                var setTimeout = window[window.Zone.__symbol__('setTimeout')];
+
                 $rootScope.$watch(function() {
-                    _debounce || (_debounce = $timeout($storage.$apply, 100, false));
+                    _debounce || (_debounce = setTimeout($storage.$apply, 100));
                 });

                 // #6: Use `$window.addEventListener` instead of `angular.element` to avoid the jQuery-specific `event.originalEvent`