diff --git a/modules/angular2/src/change_detection/change_detection_jit_generator.es6 b/modules/angular2/src/change_detection/change_detection_jit_generator.es6 index 9ba4a302be..2f63bb529b 100644 --- a/modules/angular2/src/change_detection/change_detection_jit_generator.es6 +++ b/modules/angular2/src/change_detection/change_detection_jit_generator.es6 @@ -30,9 +30,9 @@ import { * this.dispatcher = dispatcher; * this.protos = protos; * - * this.context = null; - * this.address0 = null; - * this.city1 = null; + * this.context = ChangeDetectionUtil.unitialized(); + * this.address0 = ChangeDetectionUtil.unitialized(); + * this.city1 = ChangeDetectionUtil.unitialized(); * } * ChangeDetector0.prototype = Object.create(AbstractChangeDetector.prototype); * @@ -70,10 +70,20 @@ import { * } * * - * ChangeDetector0.prototype.setContext = function(context) { + * ChangeDetector0.prototype.hydrate = function(context) { * this.context = context; * } * + * ChangeDetector0.prototype.dehydrate = function(context) { + * this.context = ChangeDetectionUtil.unitialized(); + * this.address0 = ChangeDetectionUtil.unitialized(); + * this.city1 = ChangeDetectionUtil.unitialized(); + * } + * + * ChangeDetector0.prototype.hydrated = function() { + * return this.context !== ChangeDetectionUtil.unitialized(); + * } + * * return ChangeDetector0; * * @@ -119,11 +129,22 @@ ${type}.prototype = Object.create(${ABSTRACT_CHANGE_DETECTOR}.prototype); `; } -function setContextTemplate(type:string):string { +function pipeOnDestroyTemplate(pipeNames:List) { + return pipeNames.map((p) => `${p}.onDestroy()`).join("\n"); +} + +function hydrateTemplate(type:string, fieldsDefinitions:string, pipeOnDestroy:string):string { return ` -${type}.prototype.setContext = function(context) { +${type}.prototype.hydrate = function(context) { this.context = context; } +${type}.prototype.dehydrate = function() { + ${pipeOnDestroy} + ${fieldsDefinitions} +} +${type}.prototype.hydrated = function() { + return this.context !== ${UTIL}.unitialized(); +} `; } @@ -162,7 +183,10 @@ if (${CHANGES_LOCAL} && ${CHANGES_LOCAL}.length > 0) { function pipeCheckTemplate(context:string, pipe:string, pipeType:string, value:string, change:string, addRecord:string, notify:string):string{ return ` -if (${pipe} === ${UTIL}.unitialized() || !${pipe}.supports(${context})) { +if (${pipe} === ${UTIL}.unitialized()) { + ${pipe} = ${PIPE_REGISTRY_ACCESSOR}.get('${pipeType}', ${context}); +} else if (!${pipe}.supports(${context})) { + ${pipe}.onDestroy(); ${pipe} = ${PIPE_REGISTRY_ACCESSOR}.get('${pipeType}', ${context}); } @@ -281,25 +305,34 @@ export class ChangeDetectorJITGenerator { } generate():Function { - var text = typeTemplate(this.typeName, this.genConstructor(), this.genDetectChanges(), this.genSetContext()); + var text = typeTemplate(this.typeName, this.genConstructor(), this.genDetectChanges(), this.genHydrate()); return new Function('AbstractChangeDetector', 'ChangeDetectionUtil', 'ContextWithVariableBindings', 'protos', text)(AbstractChangeDetector, ChangeDetectionUtil, ContextWithVariableBindings, this.records); } genConstructor():string { - var fields = []; - fields = fields.concat(this.fieldNames); - - this.records.forEach((r) => { - if (r.mode === RECORD_TYPE_PIPE) { - fields.push(this.pipeNames[r.selfIndex]); - } - }); - - return constructorTemplate(this.typeName, fieldDefinitionsTemplate(fields)); + return constructorTemplate(this.typeName, this.genFieldDefinitions()); } - genSetContext():string { - return setContextTemplate(this.typeName); + genHydrate():string { + return hydrateTemplate(this.typeName, this.genFieldDefinitions(), + pipeOnDestroyTemplate(this.getnonNullPipeNames())); + } + + genFieldDefinitions() { + var fields = []; + fields = fields.concat(this.fieldNames); + fields = fields.concat(this.getnonNullPipeNames()); + return fieldDefinitionsTemplate(fields); + } + + getnonNullPipeNames():List { + var pipes = []; + this.records.forEach((r) => { + if (r.mode === RECORD_TYPE_PIPE) { + pipes.push(this.pipeNames[r.selfIndex]); + } + }); + return pipes; } genDetectChanges():string { diff --git a/modules/angular2/src/change_detection/dynamic_change_detector.js b/modules/angular2/src/change_detection/dynamic_change_detector.js index 051fb027fb..2243864afc 100644 --- a/modules/angular2/src/change_detection/dynamic_change_detector.js +++ b/modules/angular2/src/change_detection/dynamic_change_detector.js @@ -43,15 +43,36 @@ export class DynamicChangeDetector extends AbstractChangeDetector { this.prevContexts = ListWrapper.createFixedSize(protoRecords.length + 1); this.changes = ListWrapper.createFixedSize(protoRecords.length + 1); + ListWrapper.fill(this.values, uninitialized); + ListWrapper.fill(this.pipes, null); + ListWrapper.fill(this.prevContexts, uninitialized); + ListWrapper.fill(this.changes, false); + this.protos = protoRecords; } - setContext(context:any) { + hydrate(context:any) { + this.values[0] = context; + } + + dehydrate() { + this._destroyPipes(); ListWrapper.fill(this.values, uninitialized); ListWrapper.fill(this.changes, false); ListWrapper.fill(this.pipes, null); ListWrapper.fill(this.prevContexts, uninitialized); - this.values[0] = context; + } + + _destroyPipes() { + for(var i = 0; i < this.pipes.length; ++i) { + if (isPresent(this.pipes[i])) { + this.pipes[i].onDestroy(); + } + } + } + + hydrated():boolean { + return this.values[0] !== uninitialized; } detectChangesInRecords(throwOnChange:boolean) { @@ -184,11 +205,13 @@ export class DynamicChangeDetector extends AbstractChangeDetector { var storedPipe = this._readPipe(proto); if (isPresent(storedPipe) && storedPipe.supports(context)) { return storedPipe; - } else { - var pipe = this.pipeRegistry.get(proto.name, context); - this._writePipe(proto, pipe); - return pipe; } + if (isPresent(storedPipe)) { + storedPipe.onDestroy(); + } + var pipe = this.pipeRegistry.get(proto.name, context); + this._writePipe(proto, pipe); + return pipe; } _readContext(proto:ProtoRecord) { diff --git a/modules/angular2/src/change_detection/interfaces.js b/modules/angular2/src/change_detection/interfaces.js index ce8888d0b6..5f7bbed50d 100644 --- a/modules/angular2/src/change_detection/interfaces.js +++ b/modules/angular2/src/change_detection/interfaces.js @@ -55,7 +55,8 @@ export class ChangeDetector { addChild(cd:ChangeDetector) {} removeChild(cd:ChangeDetector) {} remove() {} - setContext(context:any) {} + hydrate(context:any) {} + dehydrate() {} markPathToRootAsCheckOnce() {} detectChanges() {} diff --git a/modules/angular2/src/change_detection/pipes/pipe.js b/modules/angular2/src/change_detection/pipes/pipe.js index 002d1827f4..c1213ce0af 100644 --- a/modules/angular2/src/change_detection/pipes/pipe.js +++ b/modules/angular2/src/change_detection/pipes/pipe.js @@ -2,5 +2,6 @@ export var NO_CHANGE = new Object(); export class Pipe { supports(obj):boolean {return false;} + onDestroy() {} transform(value:any):any {return null;} } \ No newline at end of file diff --git a/modules/angular2/src/core/compiler/view.js b/modules/angular2/src/core/compiler/view.js index ba61bf0279..4c0c5d44cc 100644 --- a/modules/angular2/src/core/compiler/view.js +++ b/modules/angular2/src/core/compiler/view.js @@ -97,7 +97,7 @@ export class View { // TODO(tbosch): if we have a contextWithLocals we actually only need to // set the contextWithLocals once. Would it be faster to always use a contextWithLocals // even if we don't have locals and not update the recordRange here? - this.changeDetector.setContext(this.context); + this.changeDetector.hydrate(this.context); } _dehydrateContext() { @@ -105,6 +105,7 @@ export class View { this.contextWithLocals.clearValues(); } this.context = null; + this.changeDetector.dehydrate(); } /** diff --git a/modules/angular2/test/change_detection/change_detection_spec.js b/modules/angular2/test/change_detection/change_detection_spec.js index b3a7d2920c..124cd56664 100644 --- a/modules/angular2/test/change_detection/change_detection_spec.js +++ b/modules/angular2/test/change_detection/change_detection_spec.js @@ -7,7 +7,7 @@ import {Parser} from 'angular2/src/change_detection/parser/parser'; import {Lexer} from 'angular2/src/change_detection/parser/lexer'; import {ChangeDispatcher, DynamicChangeDetector, ChangeDetectionError, ContextWithVariableBindings, - PipeRegistry, NO_CHANGE, CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED} from 'angular2/change_detection'; + PipeRegistry, Pipe, NO_CHANGE, CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED} from 'angular2/change_detection'; import {ChangeDetectionUtil} from 'angular2/src/change_detection/change_detection_util'; @@ -33,7 +33,7 @@ export function main() { pcd.addAst(ast(exp), memo, memo); var dispatcher = new TestDispatcher(); var cd = pcd.instantiate(dispatcher); - cd.setContext(context); + cd.hydrate(context); return {"changeDetector" : cd, "dispatcher" : dispatcher}; } @@ -183,7 +183,7 @@ export function main() { var dispatcher = new TestDispatcher(); var cd = pcd.instantiate(dispatcher); - cd.setContext(new TestData("value")); + cd.hydrate(new TestData("value")); cd.detectChanges(); @@ -264,7 +264,7 @@ export function main() { dispatcher.logValue('InvokeC'); return 'c' }; - cd.setContext(tr); + cd.hydrate(tr); cd.detectChanges(); @@ -280,7 +280,7 @@ export function main() { var dispatcher = new TestDispatcher(); var cd = pcd.instantiate(dispatcher); - cd.setContext(new TestData('value')); + cd.hydrate(new TestData('value')); expect(() => { cd.checkNoChanges(); @@ -295,7 +295,7 @@ export function main() { pcd.addAst(ast('invalidProp', 'someComponent'), "a", 1); var cd = pcd.instantiate(new TestDispatcher()); - cd.setContext(null); + cd.hydrate(null); try { cd.detectChanges(); @@ -442,6 +442,35 @@ export function main() { }); }); + describe("hydration", () => { + it("should be able to rehydrate a change detector", () => { + var c = createChangeDetector("memo", "name"); + var cd = c["changeDetector"]; + + cd.hydrate("some context"); + expect(cd.hydrated()).toBe(true); + + cd.dehydrate(); + expect(cd.hydrated()).toBe(false); + + cd.hydrate("other context"); + expect(cd.hydrated()).toBe(true); + }); + + it("should destroy all active pipes during dehyration", () => { + var pipe = new OncePipe(); + var registry = new FakePipeRegistry('pipe', () => pipe); + var c = createChangeDetector("memo", "name | pipe", new Person('bob'), registry); + var cd = c["changeDetector"]; + + cd.detectChanges(); + + cd.dehydrate(); + + expect(pipe.destroyCalled).toBe(true); + }); + }); + describe("pipes", () => { it("should support pipes", () => { var registry = new FakePipeRegistry('pipe', () => new CountingPipe()); @@ -477,6 +506,21 @@ export function main() { expect(registry.numberOfLookups).toEqual(2); }); + + it("should invoke onDestroy on a pipe before switching to another one", () => { + var pipe = new OncePipe(); + var registry = new FakePipeRegistry('pipe', () => pipe); + var ctx = new Person("Megatron"); + + var c = createChangeDetector("memo", "name | pipe", ctx, registry); + var cd = c["changeDetector"]; + + cd.detectChanges(); + ctx.name = "Optimus Prime"; + cd.detectChanges(); + + expect(pipe.destroyCalled).toEqual(true); + }); }); it("should do nothing when returns NO_CHANGE", () => { @@ -502,10 +546,11 @@ export function main() { }); } -class CountingPipe { +class CountingPipe extends Pipe { state:number; constructor() { + super(); this.state = 0; } @@ -518,23 +563,31 @@ class CountingPipe { } } -class OncePipe { +class OncePipe extends Pipe { called:boolean; + destroyCalled:boolean; + constructor() { + super(); this.called = false;; + this.destroyCalled = false; } supports(newValue) { return !this.called; } + onDestroy() { + this.destroyCalled = true; + } + transform(value) { this.called = true; return value; } } -class IdentityPipe { +class IdentityPipe extends Pipe { state:any; supports(newValue) { diff --git a/modules/angular2/test/core/compiler/view_spec.js b/modules/angular2/test/core/compiler/view_spec.js index 0443b7675c..0582443083 100644 --- a/modules/angular2/test/core/compiler/view_spec.js +++ b/modules/angular2/test/core/compiler/view_spec.js @@ -76,6 +76,15 @@ export function main() { expect(view.hydrated()).toBe(false); }); + it('should hydrate and dehydrate the change detector', () => { + var ctx = new Object(); + view.hydrate(null, null, ctx); + expect(view.changeDetector.hydrated()).toBe(true); + + view.dehydrate(); + expect(view.changeDetector.hydrated()).toBe(false); + }); + it('should use the view pool to reuse views', () => { var pv = new ProtoView(el('
'), new DynamicProtoChangeDetector(null), null); var fakeView = new FakeView(); diff --git a/modules/benchmarks/src/change_detection/change_detection_benchmark.js b/modules/benchmarks/src/change_detection/change_detection_benchmark.js index 1fc86c47c5..65b19e785f 100644 --- a/modules/benchmarks/src/change_detection/change_detection_benchmark.js +++ b/modules/benchmarks/src/change_detection/change_detection_benchmark.js @@ -129,7 +129,7 @@ function setUpChangeDetection(changeDetection:ChangeDetection, iterations) { obj.setField(j, i); } var cd = proto.instantiate(dispatcher); - cd.setContext(obj); + cd.hydrate(obj); parentCd.addChild(cd); } return parentCd;