From 92698d9a5d109a6caefb732d83648cb8723c2433 Mon Sep 17 00:00:00 2001 From: Christian Kotzbauer Date: Thu, 18 Aug 2022 20:49:01 +0200 Subject: [PATCH] fix: handle race-condition from async composition-settings Signed-off-by: Christian Kotzbauer --- dist/amd/knockout-composition.js | 25 ++++++++++++++--- dist/commonjs/knockout-composition.js | 25 ++++++++++++++--- dist/es2015/knockout-composition.js | 24 ++++++++++++++--- dist/es2017/knockout-composition.js | 24 ++++++++++++++--- dist/native-modules/knockout-composition.js | 24 ++++++++++++++--- dist/system/knockout-composition.js | 24 ++++++++++++++--- src/knockout-composition.ts | 30 +++++++++++++++++---- 7 files changed, 147 insertions(+), 29 deletions(-) diff --git a/dist/amd/knockout-composition.js b/dist/amd/knockout-composition.js index ff6e2a0..c532293 100644 --- 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 index 1fa73f4..087e042 100644 --- 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 index 357ae3a..ebfc54a 100644 --- 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 index 357ae3a..ebfc54a 100644 --- 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 index c66e307..e9cdf35 100644 --- 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 index 55807a8..7560dde 100644 --- 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 index c50332f..75ce928 100644 --- 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 {