diff --git a/src/modules/scopes.js b/src/modules/scopes.js index 0cce42ac..7d4efcdc 100644 --- a/src/modules/scopes.js +++ b/src/modules/scopes.js @@ -79,37 +79,20 @@ function decorateRootScope($delegate, $parse) { var _digestEvents = []; var skipNextPerfWatchers = false; scopePrototype.$watch = function (watchExpression, reactionFunction) { - // if `skipNextPerfWatchers` is true, this means the previous run of the - // `$watch` decorator was a one time binding expression and this invocation - // of the $watch function has the `oneTimeInterceptedExpression` (internal angular function) - // as the `watchExpression` parameter. If we decorate it with the performance - // timers function this will cause us to invoke `oneTimeInterceptedExpression` - // on subsequent digest loops and will update the one time bindings - // if anything mutated the property. - if (skipNextPerfWatchers) { - skipNextPerfWatchers = false; - return _watch.apply(this, arguments); - } - - if (typeof watchExpression === 'string' && - isOneTimeBindExp(watchExpression)) { - skipNextPerfWatchers = true; - return _watch.apply(this, arguments); - } - var watchStr = humanReadableWatchExpression(watchExpression); - var scopeId = this.$id; - var expressions = null; - if (typeof watchExpression === 'function') { - expressions = watchExpression.expressions; - if (Object.prototype.toString.call(expressions) === '[object Array]' && - expressions.some(isOneTimeBindExp)) { - skipNextPerfWatchers = true; - return _watch.apply(this, arguments); - } - - arguments[0] = function () { + // Convert the `watchExpression` to a function (if not already one). + // This is also the first thing `Scope.$watch()` does. + var parsedExpression = $parse(watchExpression); + + // Only intercept this call if there is no `$$watchDelegate`. + // (With `$$watchDelegate` there will be subsequent calls to `$watch` (if necessary)). + if (!parsedExpression.$$watchDelegate) { + var scopeId = this.$id; + var watchStr = humanReadableWatchExpression(watchExpression); + + // Intercept the `watchExpression` (if any). + arguments[0] = simpleExtend(function() { var start = perf.now(); - var ret = watchExpression.apply(this, arguments); + var ret = parsedExpression.apply(this, arguments); var end = perf.now(); _digestEvents.push({ eventType: 'scope:watch', @@ -118,36 +101,23 @@ function decorateRootScope($delegate, $parse) { time: end - start }); return ret; - }; - } else { - var thatScope = this; - arguments[0] = function () { - var start = perf.now(); - var ret = thatScope.$eval(watchExpression); - var end = perf.now(); - _digestEvents.push({ - eventType: 'scope:watch', - id: scopeId, - watch: watchStr, - time: end - start - }); - return ret; - }; - } - - if (typeof reactionFunction === 'function') { - arguments[1] = function () { - var start = perf.now(); - var ret = reactionFunction.apply(this, arguments); - var end = perf.now(); - _digestEvents.push({ - eventType: 'scope:reaction', - id: scopeId, - watch: watchStr, - time: end - start - }); - return ret; - }; + }, parsedExpression); + + // Intercept the `reactionFunction` (if any). + if (typeof reactionFunction === 'function') { + arguments[1] = function() { + var start = perf.now(); + var ret = reactionFunction.apply(this, arguments); + var end = perf.now(); + _digestEvents.push({ + eventType: 'scope:reaction', + id: scopeId, + watch: watchStr, + time: end - start + }); + return ret; + }; + } } return _watch.apply(this, arguments); @@ -355,12 +325,13 @@ function humanReadableWatchExpression (fn) { return fn.toString(); } -function isOneTimeBindExp(exp) { - // this is the same code angular 1.3.15 has to check - // for a one time bind expression - return exp.charAt(0) === ':' && exp.charAt(1) === ':'; -} - function convertIdToOriginalType(scopeId) { return (angular.version.minor < 3) ? scopeId : parseInt(scopeId, 10); } + +function simpleExtend(dst, src) { + Object.keys(src).forEach(function(key) { + dst[key] = src[key]; + }); + return dst; +} diff --git a/test/scopes.spec.js b/test/scopes.spec.js index f229c60d..b6b5ca7a 100644 --- a/test/scopes.spec.js +++ b/test/scopes.spec.js @@ -72,33 +72,120 @@ describe('ngHintScopes', function() { }); if (angular.version.minor >= 3) { - it('should not run perf timers for one time bind expressions passed to watch', function() { - var calls = hint.emit.calls; - scope.$watch('::a.b', function() {}); - expect(calls.count()).toBe(0); - - scope.$apply(); - var evt = calls.mostRecent().args[1].events[0]; - expect(calls.count()).toBe(1); - expect(evt).toBeUndefined(); - }); - - it('should not run perf timers for one time template bindings', function() { - var elt = angular.element( - '
' + - '{{::a}}' + - '' + - '
' - ); - scope.a = 'foo'; - var view = $compile(elt)(scope); - scope.$apply(); - var $binding = view.find('span'); - var $button = view.find('button'); - - $button.triggerHandler('click'); - scope.$apply(); - expect($binding.text()).toBe('foo'); + describe('one-time expressions', function() { + // Helpers + function getDigestCallArgs() { + var allArgs = hint.emit.calls.allArgs(); + var digestCallArgs = allArgs.filter(function(args) { + return args[0] === 'scope:digest'; + }); + + return digestCallArgs; + } + + function countWatchEvents(args) { + var events = args[1].events; + var watchEvents = events.filter(function(evt) { + return evt.eventType === 'scope:watch'; + }); + + return watchEvents.length; + } + + it('should correctly trigger perf timers when passed to `$watch`', function() { + var calls = hint.emit.calls; + var args; + + var reactions = [0, 0, 0]; + var exps = [ + '::c.d', + ' ::c.d ', + ' :: c.d ' + ].forEach(function(exp, idx) { + scope.$watch(exp, function(v) { reactions[idx]++; }); + }); + + expect(calls.count()).toBe(0); + + scope.$apply(); + args = getDigestCallArgs(); + expect(args.length).toBe(1); + expect(countWatchEvents(args[0])).toBe(6); // Initial $digest: 2 loops + expect(reactions).toEqual([1, 1, 1]); // Initial $digest always calls listeners + + calls.reset(); + scope.$apply(); + args = getDigestCallArgs(); + expect(args.length).toBe(1); + expect(countWatchEvents(args[0])).toBe(3); // No change: 1 loop + expect(reactions).toEqual([1, 1, 1]); // No change: listeners not called + + calls.reset(); + scope.$apply('c.d = "foo"'); + args = getDigestCallArgs(); + expect(args.length).toBe(1); + expect(countWatchEvents(args[0])).toBe(6); // First change: 2 loops + expect(reactions).toEqual([2, 2, 2]); // First change to defined value calls listeners + + calls.reset(); + scope.$apply('c.d = "bar"'); + args = getDigestCallArgs(); + expect(args.length).toBe(1); + expect(countWatchEvents(args[0])).toBe(0); // Already settled: 0 loops + expect(reactions).toEqual([2, 2, 2]); // Already settled: listeners not called + }); + + it('should correctly trigger perf timers when used in template bindings', function() { + var calls = hint.emit.calls; + var args; + + $compile( + '
' + + // Interpolation in node text: 6 bindings (1 + 1 + 1 + 3) + '{{::c.d}}' + + '{{ ::c.d }}' + + '{{ :: c.d }}' + + '{{::c.d}}{{ ::c.d }}{{ :: c.d }}' + + + // Interpolation in attribute value: 6 bindings (1 + 1 + 1 + 3) + '' + + '' + + '' + + '' + + + // Expressions: 3 bindings (1 + 1 + 1) + '' + + '' + + '' + + + // Total: 15 watchers (6 + 6 + 3) + '
' + )(scope); + + calls.reset(); + scope.$apply(); + args = getDigestCallArgs(); + expect(args.length).toBe(1); + expect(countWatchEvents(args[0])).toBe(30); // Initial $digest: 2 loops + + calls.reset(); + scope.$apply(); + args = getDigestCallArgs(); + expect(args.length).toBe(1); + expect(countWatchEvents(args[0])).toBe(15); // No change: 1 loop + + calls.reset(); + scope.$apply('c.d = "foo"'); + args = getDigestCallArgs(); + expect(args.length).toBe(1); + expect(countWatchEvents(args[0])).toBe(30); // First change: 2 loops + + calls.reset(); + scope.$apply('c.d = "bar"'); + args = getDigestCallArgs(); + expect(args.length).toBe(1); + expect(countWatchEvents(args[0])).toBe(0); // Already settled: 0 loops + }); }); }