Repository: aurelia/templating-resources (ts)

Issue:
[Feature request] Make caching of compose configurable
<!--
BUGS: Please use this template.

SUPPORT REQUESTS/QUESTIONS: If you have a support request or question please
submit them to StackOverflow using the tags aurelia and aurelia-framework
http://stackoverflow.com/questions/tagged/aurelia
or the Aurelia Gitter https://gitter.im/aurelia/discuss

Blue Spire offers paid support agreements. Further information regarding paid support
may be obtained by emailing support@bluespire.com

Future support requests submitted here will be closed.

The HTML comments below are for your reference, and are not displayed
when your issue is submitted, feel free to leave them.

Choose one of the two headings, delete the other.
-->
**I'm submitting a feature request**

* **Library Version:**
1.11.0


**Please tell us about your environment:**
* **Operating System:**
Windows 10

* **Node Version:**
10.15.0
<!--
Minimum supported Node version is latest Node 4.x LTS
run `node -v`
-->

* **NPM Version:**
6.4.1
<!--
Minimum supported NPM version is 3.x
run `npm -v`
-->

* **JSPM OR Webpack AND Version**
webpack 4.35.0
<!--
If using JSPM
run `jspm -v`
If using Webpack
run `webpack --help | grep webpack`
-->

* **Browser:**
all 

* **Language:**
TypeScript 3.5.2


**Current behavior:**
The view-model and view used in `compose` is currently cached, that is whenever the model changes, the view-model and view is never recreated and just reactivated with the new model. This makes sense for most of the cases. However, this causes problem for some cases. For example, when validation is used in a custom element. When the model is changed, old validation rules still gets triggered. Some workarounds in this case can be to use an explicit `signal` behavior on `compose` to force recreation of VM, or to reset the validation rules somehow (I tried the later, but without success). For more details on the use case, please refer [this](https://discourse.aurelia.io/t/how-recreate-view-and-vm-for-compose/2685).

**Expected/desired behavior:**
<!--
If the current behavior is a bug, please provide the steps to reproduce and, if possible, a minimal demo of the
problem along with a runnable gist, if possible.
To create a runnable gist, go to https://gist.run/?id=7542e061bc940cde506b&sha=6821c521a6c7bae6f59a36fb8628ecd1032b2d10
Then click "Fork to Public Gist".
Create your gist, then finally click "Update Gist." Include a link to the gist.run below.
-->

* **What is the expected behavior?**
Naive expectation is that the VM and View will be recreated, considering all custom elements are transient. However, in this case I would suggest for a `@bindable cache:boolean` flag which can be used in [`processChanges`](https://github.com/aurelia/templating-resources/blob/master/src/compose.ts#L225) to force the recreation. This is similar to the [`cache` property of `if.bind`](https://github.com/aurelia/templating-resources/pull/364). 

* **What is the motivation / use case for changing the behavior?**
Any workaround will take considerable time on my part due to non-trivial refactoring, whereas the solution with the configurable `cache` looks very much easily doable.

If you are interested for a PR, I would like to give it a try.

Pull request:
feat(compose): bindable activation-strategy
This PR 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.

Fixes #381

Patch:
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 aea6900..1c6f57e 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/>` element
+ *
+ * @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,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)) {
-    // 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,
@@ -248,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
@@ -274,3 +298,15 @@ function requestUpdate(composer: Compose) {
     processChanges(composer);
   });
 }
+
+function needsReInitialization(composer: Compose, changes: any) {
+  let activationStrategy = composer.activationStrategy;
+  const vm = composer.currentViewModel;
+  if (vm && typeof vm.determineActivationStrategy === 'function') {
+    activationStrategy = vm.determineActivationStrategy();
+  }
+
+  return 'view' in changes
+    || 'viewModel' in changes
+    || activationStrategy === ActivationStrategy.Replace;
+}


Test patch:
diff --git a/test/compose.integration.spec.ts b/test/compose.integration.spec.ts
index ac67f91..e209725 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,102 @@ describe('compose.integration.spec.ts', () => {
     component.dispose();
   });
 
+  it('works with determineActivationStrategy() - replace', async () => {
+    const { component, compose } = await bootstrapCompose(
+      `<compose view-model.bind="viewModel"></compose>`,
+      {
+        viewModel: class {
+          // w/o the get view strategy, the initial composition fails, which results to undefined currentViewModel
+          getViewStrategy() {
+            return new InlineViewStrategy('<template></template>');
+          }
+
+          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(
+      `<compose view-model.bind="viewModel"></compose>`,
+      {
+        viewModel: class {
+          activate() {
+            activationCount++;
+          }
+
+          // w/o the get view strategy, the initial composition fails, which results to undefined currentViewModel
+          getViewStrategy() {
+            return new InlineViewStrategy('<template></template>');
+          }
+
+          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();
+    });
+  });
+
+  [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(
+        `<compose view-model.bind="viewModel"></compose>`,
+        {
+          viewModel: class {
+            activate() {
+              activationCount++;
+            }
+
+            // w/o the get view strategy, the initial composition fails, which results to undefined currentViewModel
+            getViewStrategy() {
+              return new InlineViewStrategy('<template></template>');
+            }
+
+            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(
diff --git a/test/compose.spec.ts b/test/compose.spec.ts
index 280bbbe..9c78625 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,30 @@ 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();
+      });
+    });
+
+    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', () => {
@@ -167,6 +191,38 @@ 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('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 => {
@@ -199,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);