Skip to content

Commit 9f8b879

Browse files
gkalpakpetebacondarwin
authored andcommitted
fix($compile): avoid calling $onChanges() twice for NaN initial values
Closes angular#15098
1 parent db4955a commit 9f8b879

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
@@ -3517,7 +3517,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
35173517
});
35183518

35193519
function recordChanges(key, currentValue, previousValue) {
3520-
if (isFunction(destination.$onChanges) && currentValue !== previousValue) {
3520+
if (isFunction(destination.$onChanges) && currentValue !== previousValue &&
3521+
// eslint-disable-next-line no-self-compare
3522+
(currentValue === currentValue || previousValue === previousValue)) {
35213523
// If we have not already scheduled the top level onChangesQueue handler then do so now
35223524
if (!onChangesQueue) {
35233525
scope.$$postDigest(flushOnChangesQueue);

test/ng/compileSpec.js

+34-2
Original file line numberDiff line numberDiff line change
@@ -4347,6 +4347,40 @@ describe('$compile', function() {
43474347
});
43484348

43494349

4350+
it('should not call `$onChanges` twice even when the initial value is `NaN`', function() {
4351+
var onChangesSpy = jasmine.createSpy('$onChanges');
4352+
4353+
module(function($compileProvider) {
4354+
$compileProvider.component('test', {
4355+
bindings: {prop: '<', attr: '@'},
4356+
controller: function TestController() {
4357+
this.$onChanges = onChangesSpy;
4358+
}
4359+
});
4360+
});
4361+
4362+
inject(function($compile, $rootScope) {
4363+
var template = '<test prop="a" attr="{{a}}"></test>' +
4364+
'<test prop="b" attr="{{b}}"></test>';
4365+
$rootScope.a = 'foo';
4366+
$rootScope.b = NaN;
4367+
4368+
element = $compile(template)($rootScope);
4369+
$rootScope.$digest();
4370+
4371+
expect(onChangesSpy).toHaveBeenCalledTimes(2);
4372+
expect(onChangesSpy.calls.argsFor(0)[0]).toEqual({
4373+
prop: jasmine.objectContaining({currentValue: 'foo'}),
4374+
attr: jasmine.objectContaining({currentValue: 'foo'})
4375+
});
4376+
expect(onChangesSpy.calls.argsFor(1)[0]).toEqual({
4377+
prop: jasmine.objectContaining({currentValue: NaN}),
4378+
attr: jasmine.objectContaining({currentValue: 'NaN'})
4379+
});
4380+
});
4381+
});
4382+
4383+
43504384
it('should only trigger one extra digest however many controllers have changes', function() {
43514385
var log = [];
43524386
function TestController1() { }
@@ -4393,7 +4427,6 @@ describe('$compile', function() {
43934427
it('should cope with changes occuring inside `$onChanges()` hooks', function() {
43944428
var log = [];
43954429
function OuterController() {}
4396-
43974430
OuterController.prototype.$onChanges = function(change) {
43984431
log.push(['OuterController', change]);
43994432
// Make a change to the inner component
@@ -4428,7 +4461,6 @@ describe('$compile', function() {
44284461

44294462
expect(log).toEqual([
44304463
['OuterController', {prop1: jasmine.objectContaining({previousValue: undefined, currentValue: 42})}],
4431-
['InnerController', {prop2: jasmine.objectContaining({previousValue: NaN, currentValue: NaN})}],
44324464
['InnerController', {prop2: jasmine.objectContaining({previousValue: NaN, currentValue: 84})}]
44334465
]);
44344466
});

0 commit comments

Comments
 (0)