Merge pull request #2163 from knockout/2163-deferred-shouldnt-notify-…

…future-subs Dynamically displayed component being rendered more than once
knockout · Feb 10, 2017 · ac94715 · ac94715
2 parents 1145f4d + 0b64dd8
commit ac94715
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 2 deletions.
diff --git a/spec/asyncBehaviors.js b/spec/asyncBehaviors.js
@@ -347,6 +347,28 @@ describe('Rate-limited', function() {
             expect(notifySpy).not.toHaveBeenCalled();
         });
 
+        it('Should not notify future subscribers', function() {
+            var observable = ko.observable('a').extend({rateLimit:500}),
+                notifySpy1 = jasmine.createSpy('notifySpy1'),
+                notifySpy2 = jasmine.createSpy('notifySpy2'),
+                notifySpy3 = jasmine.createSpy('notifySpy3');
+
+            observable.subscribe(notifySpy1);
+            observable('b');
+            observable.subscribe(notifySpy2);
+            observable('c');
+            observable.subscribe(notifySpy3);
+
+            expect(notifySpy1).not.toHaveBeenCalled();
+            expect(notifySpy2).not.toHaveBeenCalled();
+            expect(notifySpy3).not.toHaveBeenCalled();
+
+            jasmine.Clock.tick(500);
+            expect(notifySpy1).toHaveBeenCalledWith('c');
+            expect(notifySpy2).toHaveBeenCalledWith('c');
+            expect(notifySpy3).not.toHaveBeenCalled();
+        });
+
         it('Should delay update of dependent computed observable', function() {
             var observable = ko.observable().extend({rateLimit:500});
             var computed = ko.computed(observable);
@@ -388,6 +410,22 @@ describe('Rate-limited', function() {
             jasmine.Clock.tick(500);
             expect(computed()).toEqual('b');
         });
+
+        it('Should not update dependent computed created after last update', function() {
+            var observable = ko.observable('a').extend({rateLimit:500});
+            observable('b');
+
+            var evalSpy = jasmine.createSpy('evalSpy');
+            var computed = ko.computed(function () {
+                return evalSpy(observable());
+            });
+            expect(evalSpy).toHaveBeenCalledWith('b');
+            evalSpy.reset();
+
+            jasmine.Clock.tick(500);
+            expect(evalSpy).not.toHaveBeenCalled();
+        });
+
     });
 
     describe('Observable Array change tracking', function() {
@@ -698,6 +736,43 @@ describe('Deferred', function() {
             expect(observable()).toEqual('original');
         });
 
+        it('Should not notify future subscribers', function() {
+            var observable = ko.observable('a').extend({deferred:true}),
+                notifySpy1 = jasmine.createSpy('notifySpy1'),
+                notifySpy2 = jasmine.createSpy('notifySpy2'),
+                notifySpy3 = jasmine.createSpy('notifySpy3');
+
+            observable.subscribe(notifySpy1);
+            observable('b');
+            observable.subscribe(notifySpy2);
+            observable('c');
+            observable.subscribe(notifySpy3);
+
+            expect(notifySpy1).not.toHaveBeenCalled();
+            expect(notifySpy2).not.toHaveBeenCalled();
+            expect(notifySpy3).not.toHaveBeenCalled();
+
+            jasmine.Clock.tick(1);
+            expect(notifySpy1).toHaveBeenCalledWith('c');
+            expect(notifySpy2).toHaveBeenCalledWith('c');
+            expect(notifySpy3).not.toHaveBeenCalled();
+        });
+
+        it('Should not update dependent computed created after last update', function() {
+            var observable = ko.observable('a').extend({deferred:true});
+            observable('b');
+
+            var evalSpy = jasmine.createSpy('evalSpy');
+            var computed = ko.computed(function () {
+                return evalSpy(observable());
+            });
+            expect(evalSpy).toHaveBeenCalledWith('b');
+            evalSpy.reset();
+
+            jasmine.Clock.tick(1);
+            expect(evalSpy).not.toHaveBeenCalled();
+        });
+
         it('Is default behavior when "ko.options.deferUpdates" is "true"', function() {
             this.restoreAfter(ko.options, 'deferUpdates');
             ko.options.deferUpdates = true;

diff --git a/src/subscribables/subscribable.js b/src/subscribables/subscribable.js
@@ -31,7 +31,7 @@ function limitNotifySubscribers(value, event) {
 
 var ko_subscribable_fn = {
     init: function(instance) {
-        instance._subscriptions = {};
+        instance._subscriptions = { "change": [] };
         instance._versionNumber = 1;
     },
 
@@ -63,9 +63,10 @@ var ko_subscribable_fn = {
             this.updateVersion();
         }
         if (this.hasSubscriptionsForEvent(event)) {
+            var subs = event === defaultEvent && this._changeSubscriptions || this._subscriptions[event].slice(0);
             try {
                 ko.dependencyDetection.begin(); // Begin suppressing dependency detection (by setting the top frame to undefined)
-                for (var a = this._subscriptions[event].slice(0), i = 0, subscription; subscription = a[i]; ++i) {
+                for (var i = 0, subscription; subscription = subs[i]; ++i) {
                     // In case a subscription was disposed during the arrayForEach cycle, check
                     // for isDisposed on each subscription before invoking its callback
                     if (!subscription.isDisposed)
@@ -113,6 +114,7 @@ var ko_subscribable_fn = {
         });
 
         self._limitChange = function(value) {
+            self._changeSubscriptions = self._subscriptions[defaultEvent].slice(0);
             self._notificationIsPending = ignoreBeforeChange = true;
             pendingValue = value;
             finish();