From 4d674880dff221c1df393f963cd9491a8f36e461 Mon Sep 17 00:00:00 2001 From: rodyhaddad Date: Wed, 2 Jul 2014 16:26:13 -0700 Subject: [PATCH 1/4] feat($rootScope): allow for a scope to have partialDigest When a scope has partialDigest, $apply-ing the scope only triggers the dirty checking in that scope and its descendants BREAKING CHANGE: Scope.$apply now requires its `this` to be a given scope. Replace any calls similar to ```js expect($scope.$apply).toThrow('...') ``` with ```js expect(function () { $scope.$apply(); }).toThrow('...'); ``` Also, with the introduction of partialDigest, `Scope.$new` now takes an object for its options instead of a boolean. Before: ```js $scope.$new(true); ``` After: ```js $scope.$new({isolate: true}); ``` Passing a parameter is still optional --- src/ng/rootScope.js | 24 ++++++++++++++------ test/ng/compileSpec.js | 6 ++--- test/ng/directive/ngSrcSpec.js | 2 +- test/ng/rootScopeSpec.js | 40 +++++++++++++++++++++++++++++++++- 4 files changed, 60 insertions(+), 12 deletions(-) diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index a52baf637634..a75deb25b8c9 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -134,6 +134,7 @@ function $RootScopeProvider(){ this.$$listeners = {}; this.$$listenerCount = {}; this.$$isolateBindings = {}; + this.$$digestTarget = true; } /** @@ -162,19 +163,25 @@ function $RootScopeProvider(){ * desired for the scope and its child scopes to be permanently detached from the parent and * thus stop participating in model change detection and listener notification by invoking. * - * @param {boolean} isolate If true, then the scope does not prototypically inherit from the - * parent scope. The scope is isolated, as it can not see parent scope properties. - * When creating widgets, it is useful for the widget to not accidentally read parent - * state. + * @param {Object=} options An object that can contain two optional boolean flags: + * + * - `isolate` If true, then the scope does not prototypically inherit from the + * parent scope. The scope is isolated, as it can not see parent scope properties. + * When creating widgets, it is useful for the widget to not accidentally read parent + * state. + * - `partialDigest` If true, then calling {@link ng.$rootScope.Scope#$apply $apply()} on the scope + * will result in {@link ng.$rootScope.Scope#$digest $digest()} only happening on the scope itself + * and all its descendants. Useful as a performance improvement for structurally isolated components. + * * * @returns {Object} The newly created child scope. * */ - $new: function(isolate) { + $new: function(options) { var ChildScope, child; - if (isolate) { + if (options && options.isolate) { child = new Scope(); child.$root = this.$root; // ensure that there is just one async queue per $rootScope and its children @@ -205,6 +212,7 @@ function $RootScopeProvider(){ } else { this.$$childHead = this.$$childTail = child; } + child.$$digestTarget = !!(options && options.partialDigest); return child; }, @@ -978,7 +986,9 @@ function $RootScopeProvider(){ } finally { clearPhase(); try { - $rootScope.$digest(); + var scope = this; + while(!scope.$$digestTarget) scope = scope.$parent; + scope.$digest(); } catch (e) { $exceptionHandler(e); throw e; diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index fb5dc525ded6..39d072c49ec2 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -3111,7 +3111,7 @@ describe('$compile', function() { expect(componentScope.ref).toBe('hello world'); componentScope.ref = 'ignore me'; - expect($rootScope.$apply). + expect(function() {$rootScope.$apply(); }). toThrowMinErr("$compile", "nonassign", "Expression ''hello ' + name' used with directive 'myComponent' is non-assignable!"); expect(componentScope.ref).toBe('hello world'); // reset since the exception was rethrown which prevented phase clearing @@ -5277,7 +5277,7 @@ describe('$compile', function() { /* jshint scripturl:true */ element = $compile('')($rootScope); $rootScope.testUrl = $sce.trustAsUrl("javascript:doTrustedStuff()"); - expect($rootScope.$apply).toThrowMinErr( + expect(function() {$rootScope.$apply(); }).toThrowMinErr( "$interpolate", "interr", "Can't interpolate: {{testUrl}}\nError: [$sce:insecurl] Blocked " + "loading resource from url not allowed by $sceDelegate policy. URL: javascript:doTrustedStuff()"); })); @@ -5323,7 +5323,7 @@ describe('$compile', function() { /* jshint scripturl:true */ element = $compile('
')($rootScope); $rootScope.testUrl = $sce.trustAsUrl("javascript:doTrustedStuff()"); - expect($rootScope.$apply).toThrowMinErr( + expect(function() {$rootScope.$apply(); }).toThrowMinErr( "$interpolate", "interr", "Can't interpolate: {{testUrl}}\nError: [$sce:insecurl] Blocked " + "loading resource from url not allowed by $sceDelegate policy. URL: javascript:doTrustedStuff()"); })); diff --git a/test/ng/directive/ngSrcSpec.js b/test/ng/directive/ngSrcSpec.js index 8f21d6e941b3..7d421bd26ca8 100644 --- a/test/ng/directive/ngSrcSpec.js +++ b/test/ng/directive/ngSrcSpec.js @@ -45,7 +45,7 @@ describe('ngSrc', function() { it('should error on non-resource_url src attributes', inject(function($compile, $rootScope, $sce) { element = $compile('')($rootScope); $rootScope.testUrl = $sce.trustAsUrl("javascript:doTrustedStuff()"); - expect($rootScope.$apply).toThrowMinErr( + expect(function() {$rootScope.$apply(); }).toThrowMinErr( "$interpolate", "interr", "Can't interpolate: {{testUrl}}\nError: [$sce:insecurl] Blocked " + "loading resource from url not allowed by $sceDelegate policy. URL: " + "javascript:doTrustedStuff()"); diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js index 1c26bed7b676..92077b9ebf4c 100644 --- a/test/ng/rootScopeSpec.js +++ b/test/ng/rootScopeSpec.js @@ -65,13 +65,51 @@ describe('Scope', function() { })); it('should create a non prototypically inherited child scope', inject(function($rootScope) { - var child = $rootScope.$new(true); + var child = $rootScope.$new({isolate: true}); $rootScope.a = 123; expect(child.a).toBeUndefined(); expect(child.$parent).toEqual($rootScope); expect(child.$new).toBe($rootScope.$new); expect(child.$root).toBe($rootScope); })); + + it('should create a child scope with partialDigest', inject(function($rootScope) { + var child = $rootScope.$new({isolate: false, partialDigest: true}); + var spyRoot = jasmine.createSpy('spyRoot'); + var spyChild = jasmine.createSpy('spyChild'); + $rootScope.$watch(spyRoot); + child.$watch(spyChild); + + child.$apply(); + expect(spyRoot).not.toHaveBeenCalled(); + expect(spyChild).toHaveBeenCalled(); + + spyRoot.reset(); + spyChild.reset(); + + $rootScope.$apply(); + expect(spyRoot).toHaveBeenCalled(); + expect(spyChild).toHaveBeenCalled(); + })); + + it('should create an isolated child scope with partialDigest', inject(function($rootScope) { + var child = $rootScope.$new({isolate: true, partialDigest: true}); + var spyRoot = jasmine.createSpy('spyRoot'); + var spyChild = jasmine.createSpy('spyChild'); + $rootScope.$watch(spyRoot); + child.$watch(spyChild); + + child.$apply(); + expect(spyRoot).not.toHaveBeenCalled(); + expect(spyChild).toHaveBeenCalled(); + + spyRoot.reset(); + spyChild.reset(); + + $rootScope.$apply(); + expect(spyRoot).toHaveBeenCalled(); + expect(spyChild).toHaveBeenCalled(); + })); }); From 2dadd872972486b803e74ca20c2fc305c9e2bcdd Mon Sep 17 00:00:00 2001 From: rodyhaddad Date: Wed, 2 Jul 2014 16:29:23 -0700 Subject: [PATCH 2/4] feat($compile): add partialDigest property in directive definition A directive asking for a child or isolated scope can now ask for that scope to have partialDigest, making any digest triggered from inside the directive only do dirty-checking for that scope and its descendants. --- src/ng/compile.js | 11 +++- test/ng/compileSpec.js | 126 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 1 deletion(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 5a8103d7f024..489faa49ce4c 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -364,6 +364,13 @@ * parameter of directive controllers. * `function([scope], cloneLinkingFn)`. * + * #### `partialDigest` + * This property is used only if `scope` is an object, implying the directive will have an isolated scope. + * + * You can specify `partialDigest` as a boolean or as a function which takes two + * arguments `tElement` and `tAttrs` (described in the `compile` function api above) and returns + * a boolean. See {@link api/ng.$rootScope.Scope#$new $new()} for more information about partialDigest. + * * * #### Pre-linking function * @@ -1462,7 +1469,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (newIsolateScopeDirective) { var LOCAL_REGEXP = /^\s*([@=&])(\??)\s*(\w*)\s*$/; - isolateScope = scope.$new(true); + isolateScope = scope.$new({isolate: true, partialDigest: isFunction(newIsolateScopeDirective.partialDigest) + ? newIsolateScopeDirective.partialDigest($compileNode, templateAttrs) + : newIsolateScopeDirective.partialDigest}); if (templateDirective && (templateDirective === newIsolateScopeDirective || templateDirective === newIsolateScopeDirective.$$originalDirective)) { diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 39d072c49ec2..91fddbc38fbe 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -2190,6 +2190,132 @@ describe('$compile', function() { }); }); }); + + describe('partialDigest', function() { + var childScope; + beforeEach(module(function() { + directive('isolatePartialDigest', valueFn({ + scope: {value: '='}, + partialDigest: true, + template: '{{value}}', + link: function(scope) {childScope = scope;} + })); + + directive('isolateTransPartialDigest', valueFn({ + scope: {value: '='}, + partialDigest: true, + transclude: true, + template: '{{value}}', + link: function(scope) {childScope = scope;} + })); + + })); + + afterEach(function() { + childScope = null; + }); + + + it('should allow you to mark an isolated scope with partialDigest', inject( + function($rootScope, $compile) { + element = $compile('
{{value}}
')($rootScope); + + $rootScope.$apply('value=1'); + expect(element.text()).toBe('11'); + + $rootScope.value = 2; + childScope.$apply(); + expect(element.text()).toBe('11'); + + $rootScope.$apply('value=3'); + expect(element.text()).toBe('33'); + + childScope.$apply('value=4'); + expect(element.text()).toBe('34'); + + $rootScope.$apply(); + expect(element.text()).toBe('44'); + })); + + + it('should digest the correct scopes with an isolate scope and transclusion', inject( + function($rootScope, $compile) { + element = $compile('
{{value}}
{{value}}
')($rootScope); + + $rootScope.$apply('value=1'); + expect(element.text()).toBe('111'); + + $rootScope.value = 2; + childScope.$apply(); + expect(element.text()).toBe('111'); + + $rootScope.$apply('value=3'); + expect(element.text()).toBe('333'); + + childScope.$apply('value=4'); + expect(element.text()).toBe('343'); + + $rootScope.$apply(); + expect(element.text()).toBe('444'); + })); + + }); + + describe('partialDigest as function', function() { + var childScope; + beforeEach(module(function() { + directive('isolatePartialDigest', valueFn({ + scope: {value: '='}, + partialDigest: function ($element, $attrs) { + expect(nodeName_($element[0])).toBe('div'); + expect($attrs.partialDigest).toBeDefined(); + return $attrs.partialDigest === "true"; + }, + template: '{{value}}', + link: function(scope) {childScope = scope;} + })); + })); + + afterEach(function() { + childScope = null; + }); + + + it('should allow you to mark an isolated scope with partialDigest', inject( + function($rootScope, $compile) { + element = $compile('
{{value}}
content
')($rootScope); + + $rootScope.$apply('value=1'); + expect(element.text()).toBe('11'); + + $rootScope.value = 2; + childScope.$apply(); + expect(element.text()).toBe('11'); + + $rootScope.$apply('value=3'); + expect(element.text()).toBe('33'); + + childScope.$apply('value=4'); + expect(element.text()).toBe('34'); + + $rootScope.$apply(); + expect(element.text()).toBe('44'); + })); + + + it('should allow you to not mark an isolated scope with partialDigest', inject( + function($rootScope, $compile) { + element = $compile('
{{value}}
content
')($rootScope); + + $rootScope.$apply('value=1'); + expect(element.text()).toBe('11'); + + $rootScope.value = 2; + childScope.$apply(); + expect(element.text()).toBe('22'); + })); + + }); }); From 7f1a690767873a55bf2af9b0120ac30625ff2ce7 Mon Sep 17 00:00:00 2001 From: rodyhaddad Date: Tue, 29 Jul 2014 11:41:38 -0700 Subject: [PATCH 3/4] feat($timeout): allow invokeApply to be a scope on which $apply will be called This is useful when combined with partialDigest. Before this, $timeout was only able to call $apply on the $rootScope. --- src/ng/timeout.js | 12 +++++++++--- test/ng/timeoutSpec.js | 41 +++++++++++++++++++++++++++++++---------- 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/src/ng/timeout.js b/src/ng/timeout.js index 4b4c28256501..15328501a8b4 100644 --- a/src/ng/timeout.js +++ b/src/ng/timeout.js @@ -26,14 +26,20 @@ function $TimeoutProvider() { * * @param {function()} fn A function, whose execution should be delayed. * @param {number=} [delay=0] Delay in milliseconds. - * @param {boolean=} [invokeApply=true] If set to `false` skips model dirty checking, otherwise - * will invoke `fn` within the {@link ng.$rootScope.Scope#$apply $apply} block. + * @param {(boolean|Scope)=} [invokeApply=true] By default, {@link ng.$rootScope.Scope#$apply $apply} + * is called on the {@link ng.$rootScope $rootScope} when the timeout is reached. + * The default behavior can be changed by setting the parameter to one of: + * - `true` (default) calls {@link ng.$rootScope.Scope#$apply $apply} on the {@link ng.$rootScope $rootScope} + * - `false` skips dirty checking completely. Can be useful for performance. + * - `Scope` calls {@link ng.$rootScope.Scope#$apply $apply} on the given {@link ng.$rootScope.Scope Scope}. + * Useful if the scope was created as partially digestible {@link ng.$rootScope.Scope#$new partialDigest}. * @returns {Promise} Promise that will be resolved when the timeout is reached. The value this * promise will be resolved with is the return value of the `fn` function. * */ function timeout(fn, delay, invokeApply) { var skipApply = (isDefined(invokeApply) && !invokeApply), + scope = isScope(invokeApply) ? invokeApply : $rootScope, deferred = (skipApply ? $$q : $q).defer(), promise = deferred.promise, timeoutId; @@ -49,7 +55,7 @@ function $TimeoutProvider() { delete deferreds[promise.$$timeoutId]; } - if (!skipApply) $rootScope.$apply(); + if (!skipApply) scope.$apply(); }, delay); promise.$$timeoutId = timeoutId; diff --git a/test/ng/timeoutSpec.js b/test/ng/timeoutSpec.js index ec5bd8d25b8b..bffe3d794598 100644 --- a/test/ng/timeoutSpec.js +++ b/test/ng/timeoutSpec.js @@ -49,19 +49,40 @@ describe('$timeout', function() { it('should NOT call $evalAsync or $digest if invokeApply is set to false', - inject(function($timeout, $rootScope) { - var evalAsyncSpy = spyOn($rootScope, '$evalAsync').andCallThrough(); - var digestSpy = spyOn($rootScope, '$digest').andCallThrough(); - var fulfilledSpy = jasmine.createSpy('fulfilled'); + inject(function($timeout, $rootScope) { + var evalAsyncSpy = spyOn($rootScope, '$evalAsync').andCallThrough(); + var digestSpy = spyOn($rootScope, '$digest').andCallThrough(); + var fulfilledSpy = jasmine.createSpy('fulfilled'); - $timeout(fulfilledSpy, 1000, false); + $timeout(fulfilledSpy, 1000, false); - $timeout.flush(); + $timeout.flush(); - expect(fulfilledSpy).toHaveBeenCalledOnce(); - expect(evalAsyncSpy).not.toHaveBeenCalled(); - expect(digestSpy).not.toHaveBeenCalled(); - })); + expect(fulfilledSpy).toHaveBeenCalledOnce(); + expect(evalAsyncSpy).not.toHaveBeenCalled(); + expect(digestSpy).not.toHaveBeenCalled(); + })); + + + it('should call $evalAsync or $digest on the scope that\'s passed as invokeApply', + inject(function($timeout, $rootScope) { + var scope = $rootScope.$new({partialDigest: true}); + + var rootScopeDigest = jasmine.createSpy('rootScopeDigest'); + var childScopeDigest = jasmine.createSpy('childScopeDigest'); + $rootScope.$watch(rootScopeDigest); + scope.$watch(childScopeDigest); + + var fulfilledSpy = jasmine.createSpy('fulfilled'); + + $timeout(fulfilledSpy, 1000, scope); + + $timeout.flush(); + + expect(fulfilledSpy).toHaveBeenCalledOnce(); + expect(rootScopeDigest).not.toHaveBeenCalled(); + expect(childScopeDigest).toHaveBeenCalled(); + })); it('should allow you to specify the delay time', inject(function($timeout, $browser) { From 66d289b6b9d85f9d417af5ae948d3b15670ebab5 Mon Sep 17 00:00:00 2001 From: rodyhaddad Date: Tue, 29 Jul 2014 11:41:38 -0700 Subject: [PATCH 4/4] feat($interval): allow invokeApply to be a scope on which $apply will be called This is useful when combined with partialDigest. Before this, $interval was only able to call $apply on the $rootScope. --- src/ng/interval.js | 12 +++++++++--- test/ng/intervalSpec.js | 22 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/ng/interval.js b/src/ng/interval.js index 9e0656eb48bd..a46f8016ac2d 100644 --- a/src/ng/interval.js +++ b/src/ng/interval.js @@ -37,8 +37,13 @@ function $IntervalProvider() { * @param {number} delay Number of milliseconds between each function call. * @param {number=} [count=0] Number of times to repeat. If not set, or 0, will repeat * indefinitely. - * @param {boolean=} [invokeApply=true] If set to `false` skips model dirty checking, otherwise - * will invoke `fn` within the {@link ng.$rootScope.Scope#$apply $apply} block. + * @param {(boolean|Scope)=} [invokeApply=true] By default, {@link ng.$rootScope.Scope#$apply $apply} + * is called on the {@link ng.$rootScope $rootScope} after each tick of the interval. + * The default behavior can be changed by setting the parameter to one of: + * - `true` (default) calls {@link ng.$rootScope.Scope#$apply $apply} on the {@link ng.$rootScope $rootScope} + * - `false` skips dirty checking completely. Can be useful for performance. + * - `Scope` calls {@link ng.$rootScope.Scope#$apply $apply} on the given {@link ng.$rootScope.Scope Scope}. + * Useful if the scope was created as partially digestible {@link ng.$rootScope.Scope#$new partialDigest}. * @returns {promise} A promise which will be notified on each iteration. * * @example @@ -136,6 +141,7 @@ function $IntervalProvider() { clearInterval = $window.clearInterval, iteration = 0, skipApply = (isDefined(invokeApply) && !invokeApply), + scope = isScope(invokeApply) ? invokeApply : $rootScope, deferred = (skipApply ? $$q : $q).defer(), promise = deferred.promise; @@ -152,7 +158,7 @@ function $IntervalProvider() { delete intervals[promise.$$intervalId]; } - if (!skipApply) $rootScope.$apply(); + if (!skipApply) scope.$apply(); }, delay); diff --git a/test/ng/intervalSpec.js b/test/ng/intervalSpec.js index 2ffa03309bdc..d441ff869ebc 100644 --- a/test/ng/intervalSpec.js +++ b/test/ng/intervalSpec.js @@ -115,6 +115,28 @@ describe('$interval', function() { })); + it('should call $evalAsync or $digest on the scope that\'s passed as invokeApply', + inject(function($interval, $rootScope, $window, $timeout) { + var scope = $rootScope.$new({partialDigest: true}); + + var rootScopeDigest = jasmine.createSpy('rootScopeDigest'); + var childScopeDigest = jasmine.createSpy('childScopeDigest'); + $rootScope.$watch(rootScopeDigest); + scope.$watch(childScopeDigest); + + var notifySpy = jasmine.createSpy('notify'); + + $interval(notifySpy, 1000, 1, scope); + + $window.flush(2000); + $timeout.flush(); // flush $browser.defer() timeout + + expect(notifySpy).toHaveBeenCalledOnce(); + expect(rootScopeDigest).not.toHaveBeenCalled(); + expect(childScopeDigest).toHaveBeenCalled(); + })); + + it('should allow you to specify the delay time', inject(function($interval, $window) { var counter = 0; $interval(function() { counter++; }, 123);