chore(compose): post review changes

- Refactored activation strategy implmentation - Added negative tests - Minor changes in docs Issue aurelia#381
Sayan751 · Jul 10, 2019 · 2bb7890 · 2bb7890
1 parent 3aa3020
commit 2bb7890
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 8 deletions.
diff --git 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 `<compose/>` 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
@@ -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(
+        `<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(