Multiple composed views rendered at once

[klinki]

Hello,

I have found a bug in data-bind="compose: composition. If you change composed view quickly enough, it can get into situation when it renders multiple views instead of just a last one.

This happens mostly when .html for the template is not yet loaded, so it seems to be some kind of race condition.

Here is simple repro repository: https://github.com/klinki/aurelia-knockout-composedview-bug

(I'd host it on https://codesandbox.io/ but that doesn't seem to be working anymore since it adds <script> tags into templates and stackblitz.io doesn't work for me at the moment either :( )

I tried to play with it little bit but I couldn't solve it completely :( I tried to use following modification of doComposition method in knockout-composition.js file:

    function doComposition(element, unwrappedValue, viewModel) {
        var _this = this;
        var compositionId = (element.compositionId || 0) + 1;
        element.compositionId = compositionId;
        console.log({element, compositionId, state: 'before'});

        this.buildCompositionSettings(unwrappedValue, viewModel).then(function (settings) {
            console.log({element, compositionId, state: 'after'});

            /**
             * 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;
            }

            composeElementInstruction.call(_this, element, settings).then(function () {
                callEvent(element, 'compositionComplete', [element, element.parentElement]);
            });
        });
    }

Unfortunately that still didn't solve the problem :(

I believe problem is in
KnockoutComposition.prototype.register method in ko.bindingHandlers.compose.update function and probably in this part:
if (element.childElementCount > 0). I suppose race condition happens so the value evaluates to 0. But meanwhile, child element gets loaded and when doComposition.call is called, it adds another child view without detaching and removing the old ones. But it is just an idea.

[ckotzbauer]

Hi @klinki, thanks for the detailed description and the repro repo. I will have a look within the next week.

[ckotzbauer]

I released your suggested fix as 2.4.2. I ended up with this solution as I had no better one. Of course, it's hacky, but I think it does the thing.

Just out of interest: How long do you (and your company) plan on using this plugin (and associated Knockout/Durandal)? It was never intended as a permanent solution, but only as a help for migration to aureia. Since both frameworks have not seen any further development for years, maintenance is becoming increasingly difficult.

[klinki]

Honestly, if I had enough development and test capacity we would be migrated to Aurelia already.
Problem is it is a backoffice application which itself is in some sort of maintenance mode and not very actively developed.

Sometimes I have to fix some bug or add some functionality to it and I decided to move to aurelia with aurelia-knockout
because some routing issues were unsolvable with DurandalJS.

There has been lot of other work with higher priority and I'm barely finishing DurandalJS migration to aurelia with aurelia-knockout now (it is still living on separate git branch and it just moved from test to staging environment).
Luckily, we have more testers capacity now so I'm hoping I could start to gradually migrate screens from aurelia-knockout to pure aurelia.

Anyway, thanks for looking into it. As this issue happens only on small part of application I guess I will move that part into aurelia and get rid of this issue once for all.

[ckotzbauer]

Great, thanks for your feedback.