Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 7d7efbf

Browse files
gkalpakpetebacondarwin
authored andcommitted
fix($compile): avoid calling $onChanges() twice for NaN initial values
Closes #15098
1 parent 3cb5bad commit 7d7efbf

File tree

2 files changed

+37
-3
lines changed

2 files changed

+37
-3
lines changed

src/ng/compile.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -3536,7 +3536,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
35363536
});
35373537

35383538
function recordChanges(key, currentValue, previousValue) {
3539-
if (isFunction(destination.$onChanges) && currentValue !== previousValue) {
3539+
if (isFunction(destination.$onChanges) && currentValue !== previousValue &&
3540+
// eslint-disable-next-line no-self-compare
3541+
(currentValue === currentValue || previousValue === previousValue)) {
35403542
// If we have not already scheduled the top level onChangesQueue handler then do so now
35413543
if (!onChangesQueue) {
35423544
scope.$$postDigest(flushOnChangesQueue);

test/ng/compileSpec.js

+34-2
Original file line numberDiff line numberDiff line change
@@ -4387,6 +4387,40 @@ describe('$compile', function() {
43874387
});
43884388

43894389

4390+
it('should not call `$onChanges` twice even when the initial value is `NaN`', function() {
4391+
var onChangesSpy = jasmine.createSpy('$onChanges');
4392+
4393+
module(function($compileProvider) {
4394+
$compileProvider.component('test', {
4395+
bindings: {prop: '<', attr: '@'},
4396+
controller: function TestController() {
4397+
this.$onChanges = onChangesSpy;
4398+
}
4399+
});
4400+
});
4401+
4402+
inject(function($compile, $rootScope) {
4403+
var template = '<test prop="a" attr="{{a}}"></test>' +
4404+
'<test prop="b" attr="{{b}}"></test>';
4405+
$rootScope.a = 'foo';
4406+
$rootScope.b = NaN;
4407+
4408+
element = $compile(template)($rootScope);
4409+
$rootScope.$digest();
4410+
4411+
expect(onChangesSpy).toHaveBeenCalledTimes(2);
4412+
expect(onChangesSpy.calls.argsFor(0)[0]).toEqual({
4413+
prop: jasmine.objectContaining({currentValue: 'foo'}),
4414+
attr: jasmine.objectContaining({currentValue: 'foo'})
4415+
});
4416+
expect(onChangesSpy.calls.argsFor(1)[0]).toEqual({
4417+
prop: jasmine.objectContaining({currentValue: NaN}),
4418+
attr: jasmine.objectContaining({currentValue: 'NaN'})
4419+
});
4420+
});
4421+
});
4422+
4423+
43904424
it('should only trigger one extra digest however many controllers have changes', function() {
43914425
var log = [];
43924426
function TestController1() { }
@@ -4433,7 +4467,6 @@ describe('$compile', function() {
44334467
it('should cope with changes occuring inside `$onChanges()` hooks', function() {
44344468
var log = [];
44354469
function OuterController() {}
4436-
44374470
OuterController.prototype.$onChanges = function(change) {
44384471
log.push(['OuterController', change]);
44394472
// Make a change to the inner component
@@ -4468,7 +4501,6 @@ describe('$compile', function() {
44684501

44694502
expect(log).toEqual([
44704503
['OuterController', {prop1: jasmine.objectContaining({previousValue: undefined, currentValue: 42})}],
4471-
['InnerController', {prop2: jasmine.objectContaining({previousValue: NaN, currentValue: NaN})}],
44724504
['InnerController', {prop2: jasmine.objectContaining({previousValue: NaN, currentValue: 84})}]
44734505
]);
44744506
});

0 commit comments

Comments
 (0)