feat(change_detect): Guard checkNoChanges behind assertionsEnabled

Tim Blasi · Tim Blasi · commit 63e853dcd7b6 · 2015-11-06T11:59:03.000-08:00
Always generate `checkNoChanges` tests, but guard them behind `assertionsEnabled` tests. Closes #4560
diff --git a/modules/angular2/src/compiler/compiler.ts b/modules/angular2/src/compiler/compiler.ts
@@ -40,10 +40,7 @@ export function compilerProviders(): Array<Type | Provider | any[]> {
     CommandCompiler,
     ChangeDetectionCompiler,
     provide(ChangeDetectorGenConfig,
-            {
-              useValue:
-                  new ChangeDetectorGenConfig(assertionsEnabled(), assertionsEnabled(), false, true)
-            }),
+            {useValue: new ChangeDetectorGenConfig(assertionsEnabled(), false, true)}),
     TemplateCompiler,
     provide(RuntimeCompiler, {useClass: RuntimeCompiler_}),
     provide(Compiler, {useExisting: RuntimeCompiler}),
diff --git a/modules/angular2/src/core/change_detection/abstract_change_detector.ts b/modules/angular2/src/core/change_detection/abstract_change_detector.ts
@@ -1,5 +1,4 @@
-import {isPresent, isBlank, StringWrapper} from 'angular2/src/core/facade/lang';
-import {BaseException} from 'angular2/src/core/facade/exceptions';
+import {assertionsEnabled, isPresent, isBlank, StringWrapper} from 'angular2/src/core/facade/lang';
 import {ListWrapper} from 'angular2/src/core/facade/collection';
 import {ChangeDetectionUtil} from './change_detection_util';
 import {ChangeDetectorRef, ChangeDetectorRef_} from './change_detector_ref';
@@ -76,7 +75,11 @@ export class AbstractChangeDetector<T> implements ChangeDetector {
 
   detectChanges(): void { this.runDetectChanges(false); }
 
-  checkNoChanges(): void { throw new BaseException("Not implemented"); }
+  checkNoChanges(): void {
+    if (assertionsEnabled()) {
+      this.runDetectChanges(true);
+    }
+  }
 
   runDetectChanges(throwOnChange: boolean): void {
     if (this.mode === ChangeDetectionStrategy.Detached ||
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
@@ -1,4 +1,10 @@
-import {Type, isBlank, isPresent, StringWrapper} from 'angular2/src/core/facade/lang';
+import {
+  Type,
+  assertionsEnabled,
+  isBlank,
+  isPresent,
+  StringWrapper
+} from 'angular2/src/core/facade/lang';
 import {BaseException} from 'angular2/src/core/facade/exceptions';
 import {ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/core/facade/collection';
 
@@ -97,8 +103,6 @@ export class ChangeDetectorJITGenerator {
 
       ${this._maybeGenHandleEventInternal()}
 
-      ${this._genCheckNoChanges()}
-
       ${this._maybeGenAfterContentLifecycleCallbacks()}
 
       ${this._maybeGenAfterViewLifecycleCallbacks()}
@@ -434,7 +438,7 @@ export class ChangeDetectorJITGenerator {
 
   /** @internal */
   _genThrowOnChangeCheck(oldValue: string, newValue: string): string {
-    if (this.genConfig.genCheckNoChanges) {
+    if (assertionsEnabled()) {
       return `
         if(throwOnChange) {
           this.throwOnChangeError(${oldValue}, ${newValue});
@@ -445,15 +449,6 @@ export class ChangeDetectorJITGenerator {
     }
   }
 
-  /** @internal */
-  _genCheckNoChanges(): string {
-    if (this.genConfig.genCheckNoChanges) {
-      return `${this.typeName}.prototype.checkNoChanges = function() { this.runDetectChanges(true); }`;
-    } else {
-      return '';
-    }
-  }
-
   /** @internal */
   _genAddToChanges(r: ProtoRecord): string {
     var newValue = this._names.getLocalName(r.selfIndex);
diff --git a/modules/angular2/src/core/change_detection/interfaces.ts b/modules/angular2/src/core/change_detection/interfaces.ts
@@ -39,8 +39,8 @@ export interface ChangeDetector {
 export interface ProtoChangeDetector { instantiate(dispatcher: ChangeDispatcher): ChangeDetector; }
 
 export class ChangeDetectorGenConfig {
-  constructor(public genCheckNoChanges: boolean, public genDebugInfo: boolean,
-              public logBindingUpdate: boolean, public useJit: boolean) {}
+  constructor(public genDebugInfo: boolean, public logBindingUpdate: boolean,
+              public useJit: boolean) {}
 }
 
 export class ChangeDetectorDefinition {
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
@@ -19,7 +19,7 @@ export 'package:angular2/src/core/change_detection/proto_record.dart'
     show ProtoRecord;
 export 'package:angular2/src/core/change_detection/change_detection_util.dart'
     show ChangeDetectionUtil;
-export 'package:angular2/src/core/facade/lang.dart' show looseIdentical;
+export 'package:angular2/src/core/facade/lang.dart' show assertionsEnabled, looseIdentical;
 
 typedef ProtoChangeDetector PregenProtoChangeDetectorFactory(
     ChangeDetectorDefinition definition);
diff --git a/modules/angular2/src/testing/test_injector.ts b/modules/angular2/src/testing/test_injector.ts
@@ -89,8 +89,7 @@ function _getAppBindings() {
 
   return [
     compilerProviders(),
-    provide(ChangeDetectorGenConfig,
-            {useValue: new ChangeDetectorGenConfig(true, true, false, true)}),
+    provide(ChangeDetectorGenConfig, {useValue: new ChangeDetectorGenConfig(true, false, true)}),
     provide(DOCUMENT, {useValue: appDoc}),
     provide(DomRenderer, {useClass: DomRenderer_}),
     provide(Renderer, {useExisting: DomRenderer}),
diff --git a/modules/angular2/test/compiler/change_definition_factory_spec.ts b/modules/angular2/test/compiler/change_definition_factory_spec.ts
@@ -64,7 +64,7 @@ export function main() {
       var protoChangeDetectors =
           createChangeDetectorDefinitions(new CompileTypeMetadata({name: 'SomeComp'}),
                                           ChangeDetectionStrategy.Default,
-                                          new ChangeDetectorGenConfig(true, true, false, false),
+                                          new ChangeDetectorGenConfig(true, false, false),
                                           parser.parse(template, directives, 'TestComp'))
               .map(definition => new DynamicProtoChangeDetector(definition));
       var changeDetector = protoChangeDetectors[protoViewIndex].instantiate(dispatcher);
diff --git a/modules/angular2/test/compiler/change_detector_compiler_spec.ts b/modules/angular2/test/compiler/change_detector_compiler_spec.ts
@@ -80,7 +80,7 @@ export function main() {
       describe('no jit', () => {
         beforeEachBindings(() => [
           provide(ChangeDetectorGenConfig,
-                  {useValue: new ChangeDetectorGenConfig(true, true, false, false)})
+                  {useValue: new ChangeDetectorGenConfig(true, false, false)})
         ]);
         it('should watch element properties', () => {
           expect(detectChanges(compiler, '<div [el-prop]="someProp">'))
@@ -91,7 +91,7 @@ export function main() {
       describe('jit', () => {
         beforeEachBindings(() => [
           provide(ChangeDetectorGenConfig,
-                  {useValue: new ChangeDetectorGenConfig(true, true, false, true)})
+                  {useValue: new ChangeDetectorGenConfig(true, false, true)})
         ]);
         it('should watch element properties', () => {
           expect(detectChanges(compiler, '<div [el-prop]="someProp">'))
diff --git a/modules/angular2/test/core/change_detection/change_detector_config.ts b/modules/angular2/test/core/change_detection/change_detector_config.ts
@@ -65,7 +65,7 @@ export const PROP_NAME = 'propName';
  * In this case, we expect `id` and `expression` to be the same string.
  */
 export function getDefinition(id: string): TestDefinition {
-  var genConfig = new ChangeDetectorGenConfig(true, true, true, true);
+  var genConfig = new ChangeDetectorGenConfig(true, true, true);
   var testDef = null;
   if (StringMapWrapper.contains(_ExpressionWithLocals.availableDefinitions, id)) {
     let val = StringMapWrapper.get(_ExpressionWithLocals.availableDefinitions, id);
@@ -121,7 +121,7 @@ export function getDefinition(id: string): TestDefinition {
                                              [_DirectiveUpdating.recordNoCallbacks], genConfig);
     testDef = new TestDefinition(id, cdDef, null);
   } else if (id == "updateElementProduction") {
-    var genConfig = new ChangeDetectorGenConfig(false, false, false, true);
+    var genConfig = new ChangeDetectorGenConfig(false, false, true);
     var records = _createBindingRecords("name");
     let cdDef = new ChangeDetectorDefinition(id, null, [], records, [], [], genConfig);
     testDef = new TestDefinition(id, cdDef, null);
@@ -167,7 +167,7 @@ class _ExpressionWithLocals {
     var variableBindings = _convertLocalsToVariableBindings(this.locals);
     var bindingRecords = _createBindingRecords(this._expression);
     var directiveRecords = [];
-    var genConfig = new ChangeDetectorGenConfig(true, true, true, true);
+    var genConfig = new ChangeDetectorGenConfig(true, true, true);
     return new ChangeDetectorDefinition('(empty id)', strategy, variableBindings, bindingRecords,
                                         [], directiveRecords, genConfig);
   }
@@ -231,7 +231,7 @@ class _ExpressionWithMode {
                              _createHostEventRecords("(host-event)='false'", dirRecordWithOnPush))
     }
 
-    var genConfig = new ChangeDetectorGenConfig(true, true, true, true);
+    var genConfig = new ChangeDetectorGenConfig(true, true, true);
 
     return new ChangeDetectorDefinition('(empty id)', this._strategy, variableBindings,
                                         bindingRecords, eventRecords, directiveRecords, genConfig);
@@ -260,7 +260,7 @@ class _DirectiveUpdating {
   createChangeDetectorDefinition(): ChangeDetectorDefinition {
     var strategy = null;
     var variableBindings = [];
-    var genConfig = new ChangeDetectorGenConfig(true, true, true, true);
+    var genConfig = new ChangeDetectorGenConfig(true, true, true);
 
     return new ChangeDetectorDefinition('(empty id)', strategy, variableBindings,
                                         this._bindingRecords, [], this._directiveRecords,
diff --git a/modules/angular2/test/core/linker/integration_spec.ts b/modules/angular2/test/core/linker/integration_spec.ts
@@ -1575,7 +1575,7 @@ export function main() {
     describe('logging property updates', () => {
       beforeEachBindings(() => [
         provide(ChangeDetectorGenConfig,
-                {useValue: new ChangeDetectorGenConfig(true, true, true, false)})
+                {useValue: new ChangeDetectorGenConfig(true, true, false)})
       ]);
 
       it('should reflect property values as attributes',
diff --git a/modules/benchmarks/src/change_detection/change_detection_benchmark.ts b/modules/benchmarks/src/change_detection/change_detection_benchmark.ts
@@ -248,7 +248,7 @@ function setUpChangeDetection(protoChangeDetectorFactory: Function, iterations,
   var dispatcher = new DummyDispatcher();
   var parser = new Parser(new Lexer());
 
-  var genConfig = new ChangeDetectorGenConfig(false, false, false, true);
+  var genConfig = new ChangeDetectorGenConfig(false, false, true);
   var parentProto = protoChangeDetectorFactory(
       new ChangeDetectorDefinition('parent', null, [], [], [], [], genConfig));
   var parentCd = parentProto.instantiate(dispatcher);
diff --git a/modules/benchmarks/src/compiler/compiler_benchmark.ts b/modules/benchmarks/src/compiler/compiler_benchmark.ts
@@ -35,8 +35,7 @@ function _createBindings(): Provider[] {
             }),
     // Use DynamicChangeDetector as that is the only one that Dart supports as well
     // so that we can compare the numbers between JS and Dart
-    provide(ChangeDetectorGenConfig,
-            {useValue: new ChangeDetectorGenConfig(false, false, false, false)})
+    provide(ChangeDetectorGenConfig, {useValue: new ChangeDetectorGenConfig(false, false, false)})
   ];
 }
 
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
@@ -123,7 +123,8 @@ class _CodegenState {
 
     var names = new CodegenNameUtil(
         protoRecords, eventBindings, def.directiveRecords, '$genPrefix$_UTIL');
-    var logic = new CodegenLogicUtil(names, '$genPrefix$_UTIL', '$genPrefix$_STATE', def.strategy);
+    var logic = new CodegenLogicUtil(
+        names, '$genPrefix$_UTIL', '$genPrefix$_STATE', def.strategy);
     return new _CodegenState._(
         genPrefix,
         def.id,
@@ -163,8 +164,6 @@ class _CodegenState {
 
         ${_maybeGenHandleEventInternal()}
 
-        ${_genCheckNoChanges()}
-
         ${_maybeGenAfterContentLifecycleCallbacks()}
 
         ${_maybeGenAfterViewLifecycleCallbacks()}
@@ -515,23 +514,11 @@ class _CodegenState {
   }
 
   String _genThrowOnChangeCheck(String oldValue, String newValue) {
-    if (this._genConfig.genCheckNoChanges) {
-      return '''
-        if(throwOnChange) {
-          this.throwOnChangeError(${oldValue}, ${newValue});
-        }
-      ''';
-    } else {
-      return "";
-    }
-  }
-
-  String _genCheckNoChanges() {
-    if (this._genConfig.genCheckNoChanges) {
-      return 'void checkNoChanges() { runDetectChanges(true); }';
-    } else {
-      return '';
-    }
+    return '''
+      if(${_genPrefix}assertionsEnabled() && throwOnChange) {
+        this.throwOnChangeError(${oldValue}, ${newValue});
+      }
+    ''';
   }
 
   String _maybeFirstInBinding(ProtoRecord r) {
diff --git a/modules_dart/transform/lib/src/transform/template_compiler/generator.dart b/modules_dart/transform/lib/src/transform/template_compiler/generator.dart
@@ -28,8 +28,10 @@ import 'compile_data_creator.dart';
 ///
 /// This method assumes a {@link DomAdapter} has been registered.
 Future<Outputs> processTemplates(AssetReader reader, AssetId assetId,
-    {bool reflectPropertiesAsAttributes: false, List<String> ambientDirectives}) async {
-  var viewDefResults = await createCompileData(reader, assetId, ambientDirectives);
+    {bool reflectPropertiesAsAttributes: false,
+    List<String> ambientDirectives}) async {
+  var viewDefResults =
+      await createCompileData(reader, assetId, ambientDirectives);
   if (viewDefResults == null) return null;
   final directiveMetadatas = viewDefResults.ngMeta.types.values;
   if (directiveMetadatas.isNotEmpty) {
@@ -45,7 +47,7 @@ Future<Outputs> processTemplates(AssetReader reader, AssetId assetId,
   var templateCompiler = zone.templateCompiler;
   if (templateCompiler == null) {
     templateCompiler = createTemplateCompiler(reader,
-        changeDetectionConfig: new ChangeDetectorGenConfig(assertionsEnabled(),
+        changeDetectionConfig: new ChangeDetectorGenConfig(
             assertionsEnabled(), reflectPropertiesAsAttributes, false));
   }
 

Original file line number	Diff line number	Diff line change
`@@ -39,8 +39,8 @@ export interface ChangeDetector {`
`39`	`39`	`export interface ProtoChangeDetector { instantiate(dispatcher: ChangeDispatcher): ChangeDetector; }`
`40`	`40`
`41`	`41`	`export class ChangeDetectorGenConfig {`
`42`		`- constructor(public genCheckNoChanges: boolean, public genDebugInfo: boolean,`
`43`		`- public logBindingUpdate: boolean, public useJit: boolean) {}`
	`42`	`+ constructor(public genDebugInfo: boolean, public logBindingUpdate: boolean,`
	`43`	`+ public useJit: boolean) {}`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`export class ChangeDetectorDefinition {`