From 63e853dcd7b6924c0ec6b363e3bf5e73f7be81b9 Mon Sep 17 00:00:00 2001 From: Tim Blasi Date: Wed, 28 Oct 2015 10:06:53 -0700 Subject: [PATCH] feat(change_detect): Guard `checkNoChanges` behind `assertionsEnabled` Always generate `checkNoChanges` tests, but guard them behind `assertionsEnabled` tests. Closes #4560 --- modules/angular2/src/compiler/compiler.ts | 5 +--- .../abstract_change_detector.ts | 9 ++++--- .../change_detection_jit_generator.ts | 21 ++++++--------- .../src/core/change_detection/interfaces.ts | 4 +-- .../pregen_proto_change_detector.dart | 2 +- modules/angular2/src/testing/test_injector.ts | 3 +-- .../change_definition_factory_spec.ts | 2 +- .../compiler/change_detector_compiler_spec.ts | 4 +-- .../change_detector_config.ts | 10 +++---- .../test/core/linker/integration_spec.ts | 2 +- .../change_detection_benchmark.ts | 2 +- .../src/compiler/compiler_benchmark.ts | 3 +-- .../change_detector_codegen.dart | 27 +++++-------------- .../template_compiler/generator.dart | 8 +++--- 14 files changed, 42 insertions(+), 60 deletions(-) diff --git a/modules/angular2/src/compiler/compiler.ts b/modules/angular2/src/compiler/compiler.ts index 25fb3e12b5..9df2131143 100644 --- a/modules/angular2/src/compiler/compiler.ts +++ b/modules/angular2/src/compiler/compiler.ts @@ -40,10 +40,7 @@ export function compilerProviders(): Array { 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 index cf17a65d84..f98dab129c 100644 --- 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 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 index 883fadab96..de4a7e00c3 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 @@ -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 index 7bb0c8c196..803d962c56 100644 --- 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 index 1963abed56..dc305f8da1 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 @@ -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 index 5fd40ed333..02aec4b93e 100644 --- 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 index d4f635c8c9..16aa0c9be5 100644 --- 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 index 499425d482..2c9a9b5eb7 100644 --- 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, '
')) @@ -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, '
')) 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 bc9da2bfa2..7be94e8893 100644 --- 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 index 96907f7b48..5d245597bc 100644 --- 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 index d62838f317..9fd8bdbe06 100644 --- 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 index 26647b2fa8..840226297f 100644 --- 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 index 3709838d36..da90524062 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 @@ -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 index b13142c66c..932857d104 100644 --- 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 processTemplates(AssetReader reader, AssetId assetId, - {bool reflectPropertiesAsAttributes: false, List ambientDirectives}) async { - var viewDefResults = await createCompileData(reader, assetId, ambientDirectives); + {bool reflectPropertiesAsAttributes: false, + List 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 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)); }