From 1316c3e391e183e31ccb75b4f0dac91d64c08f22 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Wed, 21 Oct 2015 13:49:52 -0700 Subject: [PATCH] fix(ChangeDetector): support for NaN Closes #4853 --- .../change_detection_jit_generator.ts | 4 ++-- .../change_detection/change_detection_util.ts | 11 ++++++++- .../src/core/change_detection/coalesce.ts | 4 ++-- .../dynamic_change_detector.ts | 11 ++------- .../pregen_proto_change_detector.dart | 5 ---- .../change_detector_config.ts | 3 ++- .../change_detection/change_detector_spec.ts | 23 ++++++++++++++++++- .../change_detector_codegen.dart | 8 +++---- 8 files changed, 43 insertions(+), 26 deletions(-) diff --git a/modules/angular2/src/core/change_detection/change_detection_jit_generator.ts b/modules/angular2/src/core/change_detection/change_detection_jit_generator.ts index eb85d7f818..c10bbac233 100644 --- a/modules/angular2/src/core/change_detection/change_detection_jit_generator.ts +++ b/modules/angular2/src/core/change_detection/change_detection_jit_generator.ts @@ -284,7 +284,7 @@ export class ChangeDetectorJITGenerator { var condition = `!${pipe}.pure || (${contexOrArgCheck.join(" || ")})`; var check = ` - if (${oldValue} !== ${newValue}) { + if (${this.changeDetectionUtilVarName}.looseNotIdentical(${oldValue}, ${newValue})) { ${newValue} = ${this.changeDetectionUtilVarName}.unwrapValue(${newValue}) ${this._genChangeMarker(r)} ${this._genUpdateDirectiveOrElement(r)} @@ -311,7 +311,7 @@ export class ChangeDetectorJITGenerator { `; var check = ` - if (${newValue} !== ${oldValue}) { + if (${this.changeDetectionUtilVarName}.looseNotIdentical(${oldValue}, ${newValue})) { ${this._genChangeMarker(r)} ${this._genUpdateDirectiveOrElement(r)} ${this._genAddToChanges(r)} diff --git a/modules/angular2/src/core/change_detection/change_detection_util.ts b/modules/angular2/src/core/change_detection/change_detection_util.ts index 380b2de6a4..f115fa0869 100644 --- a/modules/angular2/src/core/change_detection/change_detection_util.ts +++ b/modules/angular2/src/core/change_detection/change_detection_util.ts @@ -1,4 +1,11 @@ -import {CONST_EXPR, isPresent, isBlank, Type, StringWrapper} from 'angular2/src/core/facade/lang'; +import { + CONST_EXPR, + isPresent, + isBlank, + Type, + StringWrapper, + looseIdentical +} from 'angular2/src/core/facade/lang'; import {BaseException} from 'angular2/src/core/facade/exceptions'; import {ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/core/facade/collection'; import {ProtoRecord} from './proto_record'; @@ -202,4 +209,6 @@ export class ChangeDetectionUtil { static directiveIndex(elementIndex: number, directiveIndex: number): DirectiveIndex { return new DirectiveIndex(elementIndex, directiveIndex); } + + static looseNotIdentical(a: any, b: any): boolean { return !looseIdentical(a, b); } } diff --git a/modules/angular2/src/core/change_detection/coalesce.ts b/modules/angular2/src/core/change_detection/coalesce.ts index 7febe376dd..80edefbf35 100644 --- a/modules/angular2/src/core/change_detection/coalesce.ts +++ b/modules/angular2/src/core/change_detection/coalesce.ts @@ -1,4 +1,4 @@ -import {isPresent, isBlank, looseIdentical} from 'angular2/src/core/facade/lang'; +import {isPresent, isBlank, looseIdentical, StringWrapper} from 'angular2/src/core/facade/lang'; import {ListWrapper, Map} from 'angular2/src/core/facade/collection'; import {RecordType, ProtoRecord} from './proto_record'; @@ -52,7 +52,7 @@ function _findMatching(r: ProtoRecord, rs: ProtoRecord[]) { return ListWrapper.find( rs, (rr) => rr.mode !== RecordType.DirectiveLifecycle && _sameDirIndex(rr, r) && rr.mode === r.mode && looseIdentical(rr.funcOrValue, r.funcOrValue) && - rr.contextIndex === r.contextIndex && looseIdentical(rr.name, r.name) && + rr.contextIndex === r.contextIndex && StringWrapper.equals(rr.name, r.name) && ListWrapper.equals(rr.args, r.args)); } diff --git a/modules/angular2/src/core/change_detection/dynamic_change_detector.ts b/modules/angular2/src/core/change_detection/dynamic_change_detector.ts index 654e413486..eb389cd09d 100644 --- a/modules/angular2/src/core/change_detection/dynamic_change_detector.ts +++ b/modules/angular2/src/core/change_detection/dynamic_change_detector.ts @@ -245,7 +245,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector { if (proto.shouldBeChecked()) { var prevValue = this._readSelf(proto, values); - if (!isSame(prevValue, currValue)) { + if (ChangeDetectionUtil.looseNotIdentical(prevValue, currValue)) { if (proto.lastInBinding) { var change = ChangeDetectionUtil.simpleChange(prevValue, currValue); if (throwOnChange) this.throwOnChangeError(prevValue, currValue); @@ -348,7 +348,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector { if (proto.shouldBeChecked()) { var prevValue = this._readSelf(proto, values); - if (!isSame(prevValue, currValue)) { + if (ChangeDetectionUtil.looseNotIdentical(prevValue, currValue)) { currValue = ChangeDetectionUtil.unwrapValue(currValue); if (proto.lastInBinding) { @@ -443,10 +443,3 @@ export class DynamicChangeDetector extends AbstractChangeDetector { return res; } } - -function isSame(a, b): boolean { - if (a === b) return true; - if (a instanceof String && b instanceof String && a == b) return true; - if ((a !== a) && (b !== b)) return true; - return false; -} diff --git a/modules/angular2/src/core/change_detection/pregen_proto_change_detector.dart b/modules/angular2/src/core/change_detection/pregen_proto_change_detector.dart index 76f1f24903..3fe7ce1094 100644 --- a/modules/angular2/src/core/change_detection/pregen_proto_change_detector.dart +++ b/modules/angular2/src/core/change_detection/pregen_proto_change_detector.dart @@ -49,8 +49,3 @@ class PregenProtoChangeDetector extends ProtoChangeDetector { @override instantiate(dynamic dispatcher) => _instantiateMethod(dispatcher); } - -/// Provided as an optimization to cut down on '!' characters in generated -/// change detectors. See https://github.com/angular/angular/issues/3248 for -/// for details. -bool looseNotIdentical(a, b) => !looseIdentical(a, b); diff --git a/modules/angular2/test/core/change_detection/change_detector_config.ts b/modules/angular2/test/core/change_detection/change_detector_config.ts index 271ace03f7..6fe181e692 100644 --- a/modules/angular2/test/core/change_detection/change_detector_config.ts +++ b/modules/angular2/test/core/change_detection/change_detector_config.ts @@ -418,7 +418,8 @@ var _availableDefinitions = [ 'a()(99)', 'a.sayHi("Jim")', 'passThrough([12])', - 'invalidFn(1)' + 'invalidFn(1)', + 'age' ]; var _availableEventDefinitions = [ diff --git a/modules/angular2/test/core/change_detection/change_detector_spec.ts b/modules/angular2/test/core/change_detection/change_detector_spec.ts index a63c4fd889..19bdacbe48 100644 --- a/modules/angular2/test/core/change_detection/change_detector_spec.ts +++ b/modules/angular2/test/core/change_detection/change_detector_spec.ts @@ -17,8 +17,10 @@ import { CONST_EXPR, isPresent, isBlank, + isNumber, isJsObject, FunctionWrapper, + NumberWrapper, normalizeBool } from 'angular2/src/core/facade/lang'; import {BaseException, WrappedException} from 'angular2/src/core/facade/exceptions'; @@ -258,6 +260,19 @@ export function main() { expect(_bindSimpleValue('a.sayHi("Jim")', td)).toEqual(['propName=Hi, Jim']); }); + it('should support NaN', () => { + var person = new Person('misko'); + person.age = NumberWrapper.NaN; + var val = _createChangeDetector('age', person); + + val.changeDetector.detectChanges(); + expect(val.dispatcher.log).toEqual(['propName=NaN']); + val.dispatcher.clear(); + + val.changeDetector.detectChanges(); + expect(val.dispatcher.log).toEqual([]); + }); + it('should do simple watching', () => { var person = new Person('misko'); var val = _createChangeDetector('name', person); @@ -1420,7 +1435,13 @@ class TestDispatcher implements ChangeDispatcher { getDebugContext(a, b) { return null; } - _asString(value) { return (isBlank(value) ? 'null' : value.toString()); } + _asString(value) { + if (isNumber(value) && NumberWrapper.isNaN(value)) { + return 'NaN'; + } + + return isBlank(value) ? 'null' : value.toString(); + } } class _ChangeDetectorAndDispatcher { diff --git a/modules_dart/transform/lib/src/transform/template_compiler/change_detector_codegen.dart b/modules_dart/transform/lib/src/transform/template_compiler/change_detector_codegen.dart index e6002daae1..4b78f4f268 100644 --- a/modules_dart/transform/lib/src/transform/template_compiler/change_detector_codegen.dart +++ b/modules_dart/transform/lib/src/transform/template_compiler/change_detector_codegen.dart @@ -354,7 +354,7 @@ class _CodegenState { var pipeType = r.name; var init = ''' - if (${_genPrefix}$_IDENTICAL_CHECK_FN($pipe, ${_genPrefix}$_UTIL.uninitialized)) { + if ($pipe == ${_genPrefix}$_UTIL.uninitialized) { $pipe = ${_names.getPipesAccessorName()}.get('$pipeType'); } '''; @@ -368,7 +368,7 @@ class _CodegenState { var condition = '''!${pipe}.pure || (${contexOrArgCheck.join(" || ")})'''; var check = ''' - if (${_genPrefix}$_NOT_IDENTICAL_CHECK_FN($oldValue, $newValue)) { + if (${_genPrefix}$_UTIL.looseNotIdentical($oldValue, $newValue)) { $newValue = ${_genPrefix}$_UTIL.unwrapValue($newValue); ${_genChangeMarker(r)} ${_genUpdateDirectiveOrElement(r)} @@ -394,7 +394,7 @@ class _CodegenState { '''; var check = ''' - if (${_genPrefix}$_NOT_IDENTICAL_CHECK_FN($newValue, $oldValue)) { + if (${_genPrefix}$_UTIL.looseNotIdentical($newValue, $oldValue)) { ${_genChangeMarker(r)} ${_genUpdateDirectiveOrElement(r)} ${_genAddToChanges(r)} @@ -532,8 +532,6 @@ const _CHANGES_LOCAL = 'changes'; const _GEN_PREFIX = '_gen'; const _GEN_PREFIX_WITH_DOT = _GEN_PREFIX + '.'; const _GEN_RECORDS_METHOD_NAME = '_createRecords'; -const _IDENTICAL_CHECK_FN = 'looseIdentical'; -const _NOT_IDENTICAL_CHECK_FN = 'looseNotIdentical'; const _IS_CHANGED_LOCAL = 'isChanged'; const _PREGEN_PROTO_CHANGE_DETECTOR_IMPORT = 'package:angular2/src/core/change_detection/pregen_proto_change_detector.dart';