fix: handle race-condition from async composition-settings

Signed-off-by: Christian Kotzbauer <git@ckotzbauer.de>
aurelia-contrib · Aug 18, 2022 · 92698d9 · 92698d9
1 parent 13d459a
commit 92698d9
Show file tree

Hide file tree

Showing 7 changed files with 147 additions and 29 deletions.
diff --git a/dist/amd/knockout-composition.js b/dist/amd/knockout-composition.js
@@ -7,6 +7,7 @@ var __decorate = (this && this.__decorate) || function (decorators, target, key,
 define(["require", "exports", "knockout", "aurelia-dependency-injection", "aurelia-loader", "aurelia-templating"], function (require, exports, ko, aurelia_dependency_injection_1, aurelia_loader_1, aurelia_templating_1) {
     "use strict";
     Object.defineProperty(exports, "__esModule", { value: true });
+    exports.KnockoutComposition = void 0;
     function endsWith(s, suffix) {
         return s.indexOf(suffix, s.length - suffix.length) !== -1;
     }
@@ -41,10 +42,26 @@ define(["require", "exports", "knockout", "aurelia-dependency-injection", "aurel
     }
     function doComposition(element, unwrappedValue, viewModel) {
         var _this = this;
-        this.buildCompositionSettings(unwrappedValue, viewModel).then(function (settings) {
-            composeElementInstruction.call(_this, element, settings).then(function () {
-                callEvent(element, 'compositionComplete', [element, element.parentElement]);
-            });
+        var compositionId = (element.compositionId || 0) + 1;
+        element.compositionId = compositionId;
+        return this.buildCompositionSettings(unwrappedValue, viewModel)
+            .then(function (settings) {
+            /**
+             * This should fixes rare race condition which happens for example in tabbed view.
+             * Race condition happens when user rapidly clicks multiple tabs (one after another) and views are not
+             * loaded yet.
+             *
+             * As result, Promises are loading the .html file for views on background and waiting.
+             * Then, when they resolve, all tabs are injected into view at once, instead of using just the last one.
+             *
+             * This fixes that issue and only last view is used (last view has highest compositionId).
+             */
+            if (element.compositionId > compositionId) {
+                console.log('Race condition detected');
+                return;
+            }
+            return composeElementInstruction.call(_this, element, settings)
+                .then(function () { return callEvent(element, 'compositionComplete', [element, element.parentElement]); });
         });
     }
     function composeElementInstruction(element, instruction) {

diff --git a/dist/commonjs/knockout-composition.js b/dist/commonjs/knockout-composition.js
@@ -6,6 +6,7 @@ var __decorate = (this && this.__decorate) || function (decorators, target, key,
     return c > 3 && r && Object.defineProperty(target, key, r), r;
 };
 Object.defineProperty(exports, "__esModule", { value: true });
+exports.KnockoutComposition = void 0;
 var ko = require("knockout");
 var aurelia_dependency_injection_1 = require("aurelia-dependency-injection");
 var aurelia_loader_1 = require("aurelia-loader");
@@ -44,10 +45,26 @@ function callEvent(element, eventName, args) {
 }
 function doComposition(element, unwrappedValue, viewModel) {
     var _this = this;
-    this.buildCompositionSettings(unwrappedValue, viewModel).then(function (settings) {
-        composeElementInstruction.call(_this, element, settings).then(function () {
-            callEvent(element, 'compositionComplete', [element, element.parentElement]);
-        });
+    var compositionId = (element.compositionId || 0) + 1;
+    element.compositionId = compositionId;
+    return this.buildCompositionSettings(unwrappedValue, viewModel)
+        .then(function (settings) {
+        /**
+         * This should fixes rare race condition which happens for example in tabbed view.
+         * Race condition happens when user rapidly clicks multiple tabs (one after another) and views are not
+         * loaded yet.
+         *
+         * As result, Promises are loading the .html file for views on background and waiting.
+         * Then, when they resolve, all tabs are injected into view at once, instead of using just the last one.
+         *
+         * This fixes that issue and only last view is used (last view has highest compositionId).
+         */
+        if (element.compositionId > compositionId) {
+            console.log('Race condition detected');
+            return;
+        }
+        return composeElementInstruction.call(_this, element, settings)
+            .then(function () { return callEvent(element, 'compositionComplete', [element, element.parentElement]); });
     });
 }
 function composeElementInstruction(element, instruction) {

diff --git a/dist/es2015/knockout-composition.js b/dist/es2015/knockout-composition.js
@@ -41,10 +41,26 @@ function callEvent(element, eventName, args) {
     }
 }
 function doComposition(element, unwrappedValue, viewModel) {
-    this.buildCompositionSettings(unwrappedValue, viewModel).then((settings) => {
-        composeElementInstruction.call(this, element, settings).then(() => {
-            callEvent(element, 'compositionComplete', [element, element.parentElement]);
-        });
+    const compositionId = (element.compositionId || 0) + 1;
+    element.compositionId = compositionId;
+    return this.buildCompositionSettings(unwrappedValue, viewModel)
+        .then((settings) => {
+        /**
+         * This should fixes rare race condition which happens for example in tabbed view.
+         * Race condition happens when user rapidly clicks multiple tabs (one after another) and views are not
+         * loaded yet.
+         *
+         * As result, Promises are loading the .html file for views on background and waiting.
+         * Then, when they resolve, all tabs are injected into view at once, instead of using just the last one.
+         *
+         * This fixes that issue and only last view is used (last view has highest compositionId).
+         */
+        if (element.compositionId > compositionId) {
+            console.log('Race condition detected');
+            return;
+        }
+        return composeElementInstruction.call(this, element, settings)
+            .then(() => callEvent(element, 'compositionComplete', [element, element.parentElement]));
     });
 }
 function composeElementInstruction(element, instruction) {

diff --git a/dist/es2017/knockout-composition.js b/dist/es2017/knockout-composition.js
@@ -41,10 +41,26 @@ function callEvent(element, eventName, args) {
     }
 }
 function doComposition(element, unwrappedValue, viewModel) {
-    this.buildCompositionSettings(unwrappedValue, viewModel).then((settings) => {
-        composeElementInstruction.call(this, element, settings).then(() => {
-            callEvent(element, 'compositionComplete', [element, element.parentElement]);
-        });
+    const compositionId = (element.compositionId || 0) + 1;
+    element.compositionId = compositionId;
+    return this.buildCompositionSettings(unwrappedValue, viewModel)
+        .then((settings) => {
+        /**
+         * This should fixes rare race condition which happens for example in tabbed view.
+         * Race condition happens when user rapidly clicks multiple tabs (one after another) and views are not
+         * loaded yet.
+         *
+         * As result, Promises are loading the .html file for views on background and waiting.
+         * Then, when they resolve, all tabs are injected into view at once, instead of using just the last one.
+         *
+         * This fixes that issue and only last view is used (last view has highest compositionId).
+         */
+        if (element.compositionId > compositionId) {
+            console.log('Race condition detected');
+            return;
+        }
+        return composeElementInstruction.call(this, element, settings)
+            .then(() => callEvent(element, 'compositionComplete', [element, element.parentElement]));
     });
 }
 function composeElementInstruction(element, instruction) {

diff --git a/dist/native-modules/knockout-composition.js b/dist/native-modules/knockout-composition.js
@@ -42,10 +42,26 @@ function callEvent(element, eventName, args) {
 }
 function doComposition(element, unwrappedValue, viewModel) {
     var _this = this;
-    this.buildCompositionSettings(unwrappedValue, viewModel).then(function (settings) {
-        composeElementInstruction.call(_this, element, settings).then(function () {
-            callEvent(element, 'compositionComplete', [element, element.parentElement]);
-        });
+    var compositionId = (element.compositionId || 0) + 1;
+    element.compositionId = compositionId;
+    return this.buildCompositionSettings(unwrappedValue, viewModel)
+        .then(function (settings) {
+        /**
+         * This should fixes rare race condition which happens for example in tabbed view.
+         * Race condition happens when user rapidly clicks multiple tabs (one after another) and views are not
+         * loaded yet.
+         *
+         * As result, Promises are loading the .html file for views on background and waiting.
+         * Then, when they resolve, all tabs are injected into view at once, instead of using just the last one.
+         *
+         * This fixes that issue and only last view is used (last view has highest compositionId).
+         */
+        if (element.compositionId > compositionId) {
+            console.log('Race condition detected');
+            return;
+        }
+        return composeElementInstruction.call(_this, element, settings)
+            .then(function () { return callEvent(element, 'compositionComplete', [element, element.parentElement]); });
     });
 }
 function composeElementInstruction(element, instruction) {

diff --git a/dist/system/knockout-composition.js b/dist/system/knockout-composition.js
@@ -42,10 +42,26 @@ System.register(["knockout", "aurelia-dependency-injection", "aurelia-loader", "
     }
     function doComposition(element, unwrappedValue, viewModel) {
         var _this = this;
-        this.buildCompositionSettings(unwrappedValue, viewModel).then(function (settings) {
-            composeElementInstruction.call(_this, element, settings).then(function () {
-                callEvent(element, 'compositionComplete', [element, element.parentElement]);
-            });
+        var compositionId = (element.compositionId || 0) + 1;
+        element.compositionId = compositionId;
+        return this.buildCompositionSettings(unwrappedValue, viewModel)
+            .then(function (settings) {
+            /**
+             * This should fixes rare race condition which happens for example in tabbed view.
+             * Race condition happens when user rapidly clicks multiple tabs (one after another) and views are not
+             * loaded yet.
+             *
+             * As result, Promises are loading the .html file for views on background and waiting.
+             * Then, when they resolve, all tabs are injected into view at once, instead of using just the last one.
+             *
+             * This fixes that issue and only last view is used (last view has highest compositionId).
+             */
+            if (element.compositionId > compositionId) {
+                console.log('Race condition detected');
+                return;
+            }
+            return composeElementInstruction.call(_this, element, settings)
+                .then(function () { return callEvent(element, 'compositionComplete', [element, element.parentElement]); });
         });
     }
     function composeElementInstruction(element, instruction) {

diff --git a/src/knockout-composition.ts b/src/knockout-composition.ts
@@ -3,6 +3,9 @@ import {Container, inject} from 'aurelia-dependency-injection';
 import {Loader} from 'aurelia-loader';
 import {ViewSlot, CompositionEngine} from 'aurelia-templating';
 
+interface ComposableElement extends Element {
+  compositionId: number;
+}
 
 function endsWith(s: string, suffix: string): boolean {
   return s.indexOf(suffix, s.length - suffix.length) !== -1;
@@ -43,12 +46,29 @@ function callEvent(element: Element, eventName: string, args: any): void {
   }
 }
 
-function doComposition(element: Element, unwrappedValue: any, viewModel: any): void {
-  this.buildCompositionSettings(unwrappedValue, viewModel).then((settings: any): void => {
-    composeElementInstruction.call(this, element, settings).then((): void => {
-      callEvent(element, 'compositionComplete', [element, element.parentElement]);
+function doComposition(element: ComposableElement, unwrappedValue: any, viewModel: any): void {
+  const compositionId = (element.compositionId || 0) + 1;
+  element.compositionId = compositionId;
+  return this.buildCompositionSettings(unwrappedValue, viewModel)
+    .then((settings: any): void => {
+      /**
+       * This should fixes rare race condition which happens for example in tabbed view.
+       * Race condition happens when user rapidly clicks multiple tabs (one after another) and views are not 
+       * loaded yet.
+       * 
+       * As result, Promises are loading the .html file for views on background and waiting.
+       * Then, when they resolve, all tabs are injected into view at once, instead of using just the last one.
+       * 
+       * This fixes that issue and only last view is used (last view has highest compositionId).
+       */
+      if (element.compositionId > compositionId) {
+        console.log('Race condition detected');
+        return;
+      }
+
+      return composeElementInstruction.call(this, element, settings)
+        .then((): void => callEvent(element, 'compositionComplete', [element, element.parentElement]))
     });
-  });
 }
 
 function composeElementInstruction(element: Element, instruction: any): Promise<void> {