From 40788838137d97a448b0f71599f6cfb8ef02a5b6 Mon Sep 17 00:00:00 2001 From: Sayan751 Date: Wed, 26 Jun 2019 23:02:45 +0200 Subject: [PATCH 1/4] feat(compose): @bindable activation-strategy This commit introduces @bindable activation-strategy for compose. The default strategy is 'invoke-lifecycle', which is also the current strategy. The strategy 'replace' forces re-creation of bound view and view-model on model change. This is helpful, where the re-instantiation of view and view-model is needed on model change. For example, when validation is used in a custom element; on model change, old validation rules still gets triggered if the view-model is not reinstantiated. Fixes #381 --- src/compose.ts | 28 ++++++++++++++++++++++++++-- test/compose.spec.ts | 13 ++++++++++++- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/compose.ts b/src/compose.ts index aea6900..db21781 100644 --- a/src/compose.ts +++ b/src/compose.ts @@ -3,7 +3,22 @@ import { DOM } from 'aurelia-pal'; import { TaskQueue } from 'aurelia-task-queue'; import { bindable, CompositionContext, CompositionEngine, customElement, noView, View, ViewResources, ViewSlot } from 'aurelia-templating'; - +/** + * Available activation strategies for the view and view-model bound to compose + * + * @export + * @enum {string} + */ +export enum ActivationStrategy { + /** + * Default activation strategy; the 'activate' lifecycle hook will be invoked when the model changes. + */ + InvokeLifecycle = 'invoke-lifecycle', + /** + * The view/view-model will be recreated, when the "model" changes. + */ + Replace = 'replace' +} /** * Used to compose a new view / view-model template or bind to an existing instance. @@ -39,6 +54,15 @@ export class Compose { */ @bindable viewModel: any; + /** + * Strategy to activate the view-model. Default is "invoke-lifecycle". + * Bind "replace" to recreate the view/view-model when the model changes. + * + * @property activationStrategy + * @type {ActivationStrategy} + */ + @bindable activationStrategy: ActivationStrategy = ActivationStrategy.InvokeLifecycle; + /** * SwapOrder to control the swapping order of the custom element's view. * @@ -226,7 +250,7 @@ function processChanges(composer: Compose) { const changes = composer.changes; composer.changes = Object.create(null); - if (!('view' in changes) && !('viewModel' in changes) && ('model' in changes)) { + if (!('view' in changes) && !('viewModel' in changes) && ('model' in changes) && composer.activationStrategy !== ActivationStrategy.Replace) { // just try to activate the current view model composer.pendingTask = tryActivateViewModel(composer.currentViewModel, changes.model); if (!composer.pendingTask) { return; } diff --git a/test/compose.spec.ts b/test/compose.spec.ts index 280bbbe..3de590c 100644 --- a/test/compose.spec.ts +++ b/test/compose.spec.ts @@ -1,6 +1,6 @@ import './setup'; import { TaskQueue } from 'aurelia-task-queue'; -import { Compose } from '../src/compose'; +import { Compose, ActivationStrategy } from '../src/compose'; import * as LogManager from 'aurelia-logging'; import { View } from 'aurelia-framework'; @@ -156,6 +156,17 @@ describe('Compose', () => { done(); }); }); + + it('when "model" changes and the activation-strategy is set to "replace"', done => { + const model = {}; + sut.activationStrategy = ActivationStrategy.Replace; + updateBindable('model', model); + taskQueue.queueMicroTask(() => { + expect(compositionEngineMock.compose).toHaveBeenCalledTimes(1); + expect(compositionEngineMock.compose).toHaveBeenCalledWith(jasmine.objectContaining({ model })); + done(); + }); + }); }); describe('does not trigger composition', () => { From 3aa3020da12c4b5e330f838542edd7b285ccb993 Mon Sep 17 00:00:00 2001 From: Sayan751 Date: Thu, 4 Jul 2019 21:17:03 +0200 Subject: [PATCH 2/4] feat(compose): activation strategy hook in VM This commit adds the support to determine the activation strategy of the bound view-model by invoking the determineActivationStrategy hook in the view-model, when implemented. The activation strategy from the view-model always overrides the one in the instance of `compose`. Addendum for #381 --- package-lock.json | 43 +++++++++++++++------ src/compose.ts | 11 +++++- test/compose.integration.spec.ts | 66 +++++++++++++++++++++++++++++++- test/compose.spec.ts | 35 +++++++++++++++++ 4 files changed, 140 insertions(+), 15 deletions(-) diff --git a/package-lock.json b/package-lock.json index 33bf6de..98ff218 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "aurelia-templating-resources", - "version": "1.9.1", + "version": "1.11.0", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -3504,7 +3504,8 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "aproba": { "version": "1.2.0", @@ -3525,12 +3526,14 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, + "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -3545,17 +3548,20 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "core-util-is": { "version": "1.0.2", @@ -3672,7 +3678,8 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "ini": { "version": "1.3.5", @@ -3684,6 +3691,7 @@ "version": "1.0.0", "bundled": true, "dev": true, + "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -3698,6 +3706,7 @@ "version": "3.0.4", "bundled": true, "dev": true, + "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -3705,12 +3714,14 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "minipass": { "version": "2.3.5", "bundled": true, "dev": true, + "optional": true, "requires": { "safe-buffer": "^5.1.2", "yallist": "^3.0.0" @@ -3729,6 +3740,7 @@ "version": "0.5.1", "bundled": true, "dev": true, + "optional": true, "requires": { "minimist": "0.0.8" } @@ -3809,7 +3821,8 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "object-assign": { "version": "4.1.1", @@ -3821,6 +3834,7 @@ "version": "1.4.0", "bundled": true, "dev": true, + "optional": true, "requires": { "wrappy": "1" } @@ -3906,7 +3920,8 @@ "safe-buffer": { "version": "5.1.2", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "safer-buffer": { "version": "2.1.2", @@ -3942,6 +3957,7 @@ "version": "1.0.2", "bundled": true, "dev": true, + "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -3961,6 +3977,7 @@ "version": "3.0.1", "bundled": true, "dev": true, + "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -4004,12 +4021,14 @@ "wrappy": { "version": "1.0.2", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "yallist": { "version": "3.0.3", "bundled": true, - "dev": true + "dev": true, + "optional": true } } }, diff --git a/src/compose.ts b/src/compose.ts index db21781..e0df2a1 100644 --- a/src/compose.ts +++ b/src/compose.ts @@ -250,7 +250,7 @@ function processChanges(composer: Compose) { const changes = composer.changes; composer.changes = Object.create(null); - if (!('view' in changes) && !('viewModel' in changes) && ('model' in changes) && composer.activationStrategy !== ActivationStrategy.Replace) { + if (!('view' in changes) && !('viewModel' in changes) && ('model' in changes) && determineActivationStrategy(composer) !== ActivationStrategy.Replace) { // just try to activate the current view model composer.pendingTask = tryActivateViewModel(composer.currentViewModel, changes.model); if (!composer.pendingTask) { return; } @@ -298,3 +298,12 @@ function requestUpdate(composer: Compose) { processChanges(composer); }); } + +function determineActivationStrategy(composer: Compose) { + let activationStrategy = composer.activationStrategy; + const vm = composer.currentViewModel; + if (vm && typeof vm.determineActivationStrategy === 'function') { + activationStrategy = vm.determineActivationStrategy(); + } + return activationStrategy; +} diff --git a/test/compose.integration.spec.ts b/test/compose.integration.spec.ts index ac67f91..eb1f9b8 100644 --- a/test/compose.integration.spec.ts +++ b/test/compose.integration.spec.ts @@ -1,8 +1,8 @@ import './setup'; import { StageComponent, ComponentTester } from 'aurelia-testing'; import { bootstrap } from 'aurelia-bootstrapper'; -import { Compose } from '../src/compose'; -import { InlineViewStrategy, useView } from 'aurelia-framework'; +import { Compose, ActivationStrategy } from '../src/compose'; +import { InlineViewStrategy, useView, TaskQueue } from 'aurelia-framework'; describe('compose.integration.spec.ts', () => { @@ -81,6 +81,68 @@ describe('compose.integration.spec.ts', () => { component.dispose(); }); + it('works with determineActivationStrategy() - replace', async () => { + const { component, compose } = await bootstrapCompose( + ``, + { + viewModel: class { + // w/o the get view strategy, the initial composition fails, which results to undefined currentViewModel + getViewStrategy() { + return new InlineViewStrategy(''); + } + + determineActivationStrategy() { + return ActivationStrategy.Replace; + } + } + } + ); + + const taskQueue = new TaskQueue(); + spyOn(compose.compositionEngine, 'compose').and.callThrough(); + const oldModel = compose.model; + compose.modelChanged({ a: 1 }, oldModel); + + taskQueue.queueMicroTask(() => { + expect(compose.compositionEngine.compose).toHaveBeenCalledTimes(1); + component.dispose(); + }); + + }); + + it('works with determineActivationStrategy() - invoke-lifecycle', async () => { + let activationCount = 0; + const { component, compose } = await bootstrapCompose( + ``, + { + viewModel: class { + activate() { + activationCount++; + } + + // w/o the get view strategy, the initial composition fails, which results to undefined currentViewModel + getViewStrategy() { + return new InlineViewStrategy(''); + } + + determineActivationStrategy() { + return ActivationStrategy.InvokeLifecycle; + } + } + } + ); + + const taskQueue = new TaskQueue(); + + const oldModel = compose.model; + compose.modelChanged({}, oldModel); + + taskQueue.queueMicroTask(() => { + expect(activationCount).toBe(2, 'activation count === 2'); + component.dispose(); + }); + }); + describe('scope traversing', () => { it('traverses scope by default', async () => { const { component } = await bootstrapCompose( diff --git a/test/compose.spec.ts b/test/compose.spec.ts index 3de590c..3d90b04 100644 --- a/test/compose.spec.ts +++ b/test/compose.spec.ts @@ -167,6 +167,19 @@ describe('Compose', () => { done(); }); }); + + it('when "model" changes and the "determineActivationStrategy" hook in view-model returns "replace"', done => { + const model = {}; + sut.currentViewModel = { + determineActivationStrategy() { return ActivationStrategy.Replace; } + }; + updateBindable('model', model); + taskQueue.queueMicroTask(() => { + expect(compositionEngineMock.compose).toHaveBeenCalledTimes(1); + expect(compositionEngineMock.compose).toHaveBeenCalledWith(jasmine.objectContaining({ model })); + done(); + }); + }); }); describe('does not trigger composition', () => { @@ -178,6 +191,28 @@ describe('Compose', () => { done(); }); }); + + it('when "model" changes and the "determineActivationStrategy" hook in view-model returns "invoke-lifecycle"', done => { + const model = {}; + sut.currentViewModel = { + determineActivationStrategy() { return ActivationStrategy.InvokeLifecycle; } + }; + updateBindable('model', model); + taskQueue.queueMicroTask(() => { + expect(compositionEngineMock.compose).not.toHaveBeenCalled(); + done(); + }); + }); + + it('when "model" changes and the "determineActivationStrategy" hook is not implemented in view-model', done => { + const model = {}; + sut.currentViewModel = {}; + updateBindable('model', model); + taskQueue.queueMicroTask(() => { + expect(compositionEngineMock.compose).not.toHaveBeenCalled(); + done(); + }); + }); }); it('aggregates changes from single drain of the micro task queue', done => { From 2bb7890402d935d08dc9aa9f22a444229575ef34 Mon Sep 17 00:00:00 2001 From: Sayan751 Date: Wed, 10 Jul 2019 19:06:43 +0200 Subject: [PATCH 3/4] chore(compose): post review changes - Refactored activation strategy implmentation - Added negative tests - Minor changes in docs Issue #381 --- src/compose.ts | 19 ++++++++++-------- test/compose.integration.spec.ts | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/src/compose.ts b/src/compose.ts index e0df2a1..1c6f57e 100644 --- a/src/compose.ts +++ b/src/compose.ts @@ -4,7 +4,7 @@ import { TaskQueue } from 'aurelia-task-queue'; import { bindable, CompositionContext, CompositionEngine, customElement, noView, View, ViewResources, ViewSlot } from 'aurelia-templating'; /** - * Available activation strategies for the view and view-model bound to compose + * Available activation strategies for the view and view-model bound to `` element * * @export * @enum {string} @@ -250,11 +250,7 @@ function processChanges(composer: Compose) { const changes = composer.changes; composer.changes = Object.create(null); - if (!('view' in changes) && !('viewModel' in changes) && ('model' in changes) && determineActivationStrategy(composer) !== ActivationStrategy.Replace) { - // just try to activate the current view model - composer.pendingTask = tryActivateViewModel(composer.currentViewModel, changes.model); - if (!composer.pendingTask) { return; } - } else { + if (needsReInitialization(composer, changes)) { // init context let instruction = { view: composer.view, @@ -272,6 +268,10 @@ function processChanges(composer: Compose) { composer.currentController = controller; composer.currentViewModel = controller ? controller.viewModel : null; }); + } else { + // just try to activate the current view model + composer.pendingTask = tryActivateViewModel(composer.currentViewModel, changes.model); + if (!composer.pendingTask) { return; } } composer.pendingTask = composer.pendingTask @@ -299,11 +299,14 @@ function requestUpdate(composer: Compose) { }); } -function determineActivationStrategy(composer: Compose) { +function needsReInitialization(composer: Compose, changes: any) { let activationStrategy = composer.activationStrategy; const vm = composer.currentViewModel; if (vm && typeof vm.determineActivationStrategy === 'function') { activationStrategy = vm.determineActivationStrategy(); } - return activationStrategy; + + return 'view' in changes + || 'viewModel' in changes + || activationStrategy === ActivationStrategy.Replace; } diff --git a/test/compose.integration.spec.ts b/test/compose.integration.spec.ts index eb1f9b8..e209725 100644 --- a/test/compose.integration.spec.ts +++ b/test/compose.integration.spec.ts @@ -143,6 +143,40 @@ describe('compose.integration.spec.ts', () => { }); }); + [1, true, 'chaos', new Date(1900, 1, 1), {}].map((strategy) => + it(`applies invoke-lifecycle strategy when determineActivationStrategy() returns unknown value such as ${strategy}`, async () => { + let activationCount = 0; + const { component, compose } = await bootstrapCompose( + ``, + { + viewModel: class { + activate() { + activationCount++; + } + + // w/o the get view strategy, the initial composition fails, which results to undefined currentViewModel + getViewStrategy() { + return new InlineViewStrategy(''); + } + + determineActivationStrategy() { + return strategy; + } + } + } + ); + + const taskQueue = new TaskQueue(); + + const oldModel = compose.model; + compose.modelChanged({}, oldModel); + + taskQueue.queueMicroTask(() => { + expect(activationCount).toBe(2, 'activation count === 2'); + component.dispose(); + }); + })); + describe('scope traversing', () => { it('traverses scope by default', async () => { const { component } = await bootstrapCompose( From ead0b5c0af039f49b835c8bca3afa7cb4c77dca4 Mon Sep 17 00:00:00 2001 From: Sayan751 Date: Thu, 11 Jul 2019 18:02:11 +0200 Subject: [PATCH 4/4] chore(compose): test cases with unknown strategy This commit adds one more tests for unsupported value for activation strategy. The tests assert that for unsupported values, default strategy is used. --- test/compose.spec.ts | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/compose.spec.ts b/test/compose.spec.ts index 3d90b04..9c78625 100644 --- a/test/compose.spec.ts +++ b/test/compose.spec.ts @@ -213,6 +213,16 @@ describe('Compose', () => { done(); }); }); + + it('when "model" changes and the activation-strategy is set to unknown value', done => { + const model = {}; + sut.activationStrategy = Math.random().toString() as ActivationStrategy; + updateBindable('model', model); + taskQueue.queueMicroTask(() => { + expect(compositionEngineMock.compose).not.toHaveBeenCalled(); + done(); + }); + }); }); it('aggregates changes from single drain of the micro task queue', done => { @@ -245,6 +255,19 @@ describe('Compose', () => { }); it('activates the "currentViewModel" if there is no change requiring composition', done => { + const model = 42; + sut.activationStrategy = Math.random().toString() as ActivationStrategy; + sut.currentViewModel = jasmine.createSpyObj('currentViewModelSpy', ['activate']); + updateBindable('model', model); + taskQueue.queueMicroTask(() => { + expect(compositionEngineMock.compose).not.toHaveBeenCalled(); + expect(sut.currentViewModel.activate).toHaveBeenCalledTimes(1); + expect(sut.currentViewModel.activate).toHaveBeenCalledWith(model); + done(); + }); + }); + + it('activates the "currentViewModel" if the activation strategy is unknown', done => { const model = 42; sut.currentViewModel = jasmine.createSpyObj('currentViewModelSpy', ['activate']); updateBindable('model', model);