CompositionTransaction and CompositionTransactionNotifier not work as per 367

[Spaceman1861]

I'm submitting a bug report

• Library Version:
  "aurelia-after-attached-plugin": "github:aurelia-ui-toolkits/aurelia-after-attached-plugin@^0.1.3",
  "aurelia-bootstrapper": "npm:aurelia-bootstrapper@^2.3.0",
  "aurelia-cookie": "npm:aurelia-cookie@^1.0.10",
  "aurelia-event-aggregator": "npm:aurelia-event-aggregator@^1.0.1",
  "aurelia-fetch-client": "npm:aurelia-fetch-client@^1.6.0",
  "aurelia-framework": "npm:aurelia-framework@^1.3.0",
  "aurelia-kendoui-bridge": "npm:aurelia-kendoui-bridge@^1.8.0",
  "aurelia-pal": "npm:aurelia-pal@^1.8.0",
  "aurelia-pal-browser": "npm:aurelia-pal-browser@^1.8.0",
  "aurelia-router": "npm:aurelia-router@^1.6.3",
  "aurelia-templating-resources": "npm:aurelia-templating-resources@^1.7.1"

Please tell us about your environment:

• Operating System:
  Windows 10

• Node Version:
  8.11.1

• NPM Version:
  6.4.0

• Aurelia CLI OR JSPM OR Webpack AND Version
  JSPM 0.16.53

• Browser:
  all

• Language:
  TypeScript 3

Current behavior:
Im am trying to setup usage of CompositionTransaction as described in #367.

import { inject,  CompositionTransaction, CompositionTransactionNotifier } from "aurelia-framework";

@inject(CompositionTransaction)
export class Dashboard {
    public compositionTransactionNotifier: CompositionTransactionNotifier;
    public compositionTransaction: CompositionTransaction;

    private timeToWaitBetweenStates: number = 5000;

    constructor(
        compositionTransaction: CompositionTransaction
    ) {
        this.compositionTransaction = compositionTransaction;
        this.compositionTransactionNotifier = this.compositionTransaction.enlist();

        this.log("Constructor called");
        setTimeout(() => {
                this.compositionTransactionNotifier.done();
                this.log("Constructor done");
            },
            this.timeToWaitBetweenStates
        );
    }

    public bind() {
        this.compositionTransactionNotifier = this.compositionTransaction.enlist();

        this.log("Bind called");
        setTimeout(() => {
                this.compositionTransactionNotifier.done();
                this.log("Bind done");
            },
            this.timeToWaitBetweenStates
        );
    }

    public attached() {
        this.compositionTransactionNotifier = this.compositionTransaction.enlist();

        this.log("Attached called");
        setTimeout(() => {
                this.compositionTransactionNotifier.done();
                this.log("Attached done");
            },
            this.timeToWaitBetweenStates
        );
    }

    public detached() {
        this.compositionTransactionNotifier = this.compositionTransaction.enlist();

        this.log("Detached called");
        setTimeout(() => {
                this.compositionTransactionNotifier.done();
                this.log("Detached done");
            },
            this.timeToWaitBetweenStates
        );
    }

    public unbind() {
        this.compositionTransactionNotifier = this.compositionTransaction.enlist();

        this.log("Unbind called");
        setTimeout(() => {
                this.compositionTransactionNotifier.done();
                this.log("Unbind done");
            },
            this.timeToWaitBetweenStates
        );
    }

    public log(text) {
        console.log(
            text,
            this,
            this.compositionTransaction["_compositionCount"]
        );
    }
}

This outputs
14:27:36.102 CvLogger.ts:3 Constructor called (3) [Dashboard, 1]
14:27:36.162 CvLogger.ts:3 Bind called (3) [Dashboard, 2]
14:27:36.258 CvLogger.ts:3 Attached called (3) [Dashboard, 3]
14:27:41.109 CvLogger.ts:3 Constructor done (3) [Dashboard, 2]
14:27:41.167 CvLogger.ts:3 Bind done (3) [Dashboard, 1]
14:27:41.263 CvLogger.ts:3 Attached done (3) [Dashboard, 0]

Expected/desired behavior:
I expect for each lifecycle step to finished 5 seconds after the last eg

14:27:00.000CvLogger.ts:3 Constructor called (3) [Dashboard, 1]
14:32:00.000CvLogger.ts:3 Constructor done (3) [Dashboard, 0]
14:32:00.000CvLogger.ts:3 Bind called (3) [Dashboard, 1]
14:37:00.000CvLogger.ts:3 Bind done (3) [Dashboard, 0]
14:37:00.000CvLogger.ts:3 Attached called (3) [Dashboard, 1]
14:42:00.000CvLogger.ts:3 Attached done (3) [Dashboard, 0,]

• What is the motivation / use case for changing the behavior?
  I wish have data loaded before moving to the next step.

[bigopon]

I don't think CompositionTransaction is meant to be used that way in the constructor. It's a away to wait ensure swap happens after transaction has finished, which means only attached called / done is guaranteed to come after bind done. Now your actual results are a bit unexpected, can you provide a reproduction based on this https://gist.run/?id=0bee57f9e341fabf5360c55073bcf4f9

[Spaceman1861]

I am unable to replicate exactly in the gist im still looking into why.

Can you explain why in #367
You can await the constructor and the bind?

Im probably being dense sorry.

[bigopon]

Awaiting in the constructor should never worked. bind works because it's what the composition engine waits for before it swap views. If it did work, then I'd need a reproduction of it to even know what it's doing

[Spaceman1861]

Ok so i have managed to replicate in the gist looks like it only occurs when I nest additional viewmodels.

Up until that point as you say the bind => attached CompositionTransaction is observed.

Whilst I would prefer to have all the steps between lifecycle state functional I really only need one to work so bind => attached is fine.

Here is the gist.
https://gist.run/?id=e96391f6787b9695bdae3800ffabba54

For me it outputs this:

Dashboard : Constructor done
Dashboard : Bind called
Dashboard : Attached called
Dashboard : Bind done
Dashboard : Attached done
BaseWidget : Constructor called
SubWidget : Bind called
SubWidget : Attached called
SubWidget : Constructor done
SubWidget : Bind done
SubWidget : Attached done
BaseWidget : Constructor called
SuperSubWidget : Bind called
SuperSubWidget : Attached called
SuperSubWidget : Constructor done
SuperSubWidget : Bind done
SuperSubWidget : Attached done

If i modify the app.html to:

<template>
  <require from="./sub-widget"></require>
  
  <div repeat.for="log of logs">
    <div>${log}</div>
  </div>
</template>

I get:

BaseWidget : Constructor called
Dashboard : Constructor done
Dashboard : Bind called
Dashboard : Attached called
Dashboard : Bind done
Dashboard : Attached done

If i modify the app.html to

<template>
  <div repeat.for="log of logs">
    <div>${log}</div>
  </div>
</template>

I get:

BaseWidget : Constructor called
Dashboard : Bind called
Dashboard : Constructor done
Dashboard : Bind done
Dashboard : Attached called
Dashboard : Attached done

I'm probably doing something really dumb thanks for having a look.

[Spaceman1861]

Any chance on an update on this? Its a bit of a blocker for me :(

[EisenbergEffect]

@bigopon Are you able to take a second look?

[bigopon]

@Spaceman1861 I think you are confused by the logs. They are behaving as documented for me. Note that the logs don't reflect the order of lifecycle correctly, as each widget has their own logs.
A little tweak to show the order: https://gist.run/?id=d31fd5ddb639af5a6c48876b593be0b9

Simple playable project for local experiment with some styling to highlight the issue with logs (extract zip and run a http-server in the folder) aurelia-comp-token.zip

[Spaceman1861]

@bigopon I could be crazy but I think your gist is still showing incorrect behavior.

Your gist outputs:

BaseWidget : Constructor called
Dashboard : Constructor done
Dashboard : Bind called
Dashboard : Attached called
Dashboard : Bind done
Dashboard : Attached done
BaseWidget : Constructor called
SubWidget : Bind called
SubWidget : Attached called
SubWidget : Constructor done
SubWidget : Bind done
SubWidget : Attached done
BaseWidget : Constructor called
SuperSubWidget : Bind called
SuperSubWidget : Attached called
SuperSubWidget : Constructor done
SuperSubWidget : Bind done
SuperSubWidget : Attached done

Note:

Dashboard : Constructor done
Dashboard : Bind called
Dashboard : Attached called
Dashboard : Bind done
Dashboard : Attached done

I would expect this to be:

Dashboard : Constructor done
Dashboard : Bind called
Dashboard : Bind done
Dashboard : Attached called
Dashboard : Attached done

Is this an incorrect assumption

[bigopon]

I see what you meant. I think you are correct. Let me check it again.

[bigopon]

I don't think CompositionTransaction is meant to be used that way in the constructor. It's a away to wait ensure swap happens after transaction has finished, which means only attached called / done is guaranteed to come after bind done. Now your actual results are a bit unexpected, can you provide a reproduction based on this https://gist.run/?id=0bee57f9e341fabf5360c55073bcf4f9

The above is what I said, and it's horribly wrong, I didn't understand it enough. My apology.

Here is an updated gist log showing the behavior you'd expect https://gist.run/?id=0de9543f3f53bf73cf891c4f7258a634

So the key point is to enlist composition transaction notifier before bind, and delist it via done() before attached(). If you try to delist via done() in constructor, via a timeout, it's luck base, depends on which comes first: the composition controller creation or the timeout. Maybe you can update your gist to show why you need to pair enlist() and done() in each cycle that way?

[Spaceman1861]

The timeout is just simulating a bunch of awaited ajax requests.

Widget is attached => loads data related to the bind method => then bind fires.

The reason im trying to do it this way is I want all the data loaded before proceeding to the bind step.

I will review your gist as it appears to be functioning as I expect.

It would appear that by removing the usage in the constructor we avoid calling done twice in the constructor and stop the cascaded offset of the notifier.

[Spaceman1861]

I can confirm that if i only use the compositionTransactionNotifier to "Await" the bind step and nothing else it appears to work.

[Spaceman1861]

@bigopon Ok so I attempted to translate only using bind into my app and I found that if the component is conditional its does not adhere to the transaction notifier.

https://gist.run/?id=1beeeae39facd87ad9eec9c6e4c3d0db

I have updated the gist with an example where some arbitrary trigger(in this case a timeout) causes the widget to be bound, it will ignore the transaction notifier.

note:
<sub-widget if.bind="viewmode == 1"></sub-widget>

In my apps case this is not triggered by a timeout but when the state of the data changes a different widget is loaded. The state change is driven by websockets.

[bigopon]

For the updated gist:

• enlist() should happen before bind() life cycle, and bind() will never be called or always be called if _compositionCount is > 0 or <= 0, respectively. Because of this, you need to enlist() in constructor or created() lifecycle.
• CompositionTransaction is meant to used for a composition transaction, which is typically setRoot, or <compose/>. If the app finishes its initial composition, and then some data gets changed, it's a view swapping inside if custom attribute, not really a composition, thus, it won't work with CompositionTransaction.

For your scenario, I think you can easily employ an if and an else to handle it.

[Spaceman1861]

Would you be able to update the gist showing how you would implement it with the if / else i'm not following your suggestion at the moment.

[bigopon]

@Spaceman1861 I was giving that suggestion based on this:

In my apps case this is not triggered by a timeout but when the state of the data changes a different widget is loaded. The state change is driven by websockets.

I would normally hide <sub-widget/> like this:

<sub-widget if.bind='hasData'></sub-widget>

And on data comes from ws or fetch, just turn hasData = true.

The above is what I guessed out of what described. I think you may want to move this topic to Discourse (https://discourse.aurelia.io) or StackOverflow. It's better for future visibility.

[Spaceman1861]

So you are suggesting i move the onus of the data loading to the parent widget?

If this is the case, im not a huge fan of this solution.

I would prefer to have the widget responsible for loading its own data.

I have managed to create a work around tho.
https://gist.run/?id=b02132587e9def755a5d0d6c64f2b3ee

It just means i need to wrap the html in some dummy if binding within the widget.

<template>
  <require from="./super-sub-widget"></require>
  <div if.bind="bindLogicCompleted">
      <div repeat.for="log of logs">
        <div>${log}</div>
      </div>
      
      <super-sub-widget></super-sub-widget>
    
  </div>
</template>

As opposed to

<template>
  <require from="./super-sub-widget"></require>
  <div repeat.for="log of logs">
        <div>${log}</div>
  </div>
      
  <super-sub-widget></super-sub-widget>
</template>

I have done away with using the CompositionTransaction and put a bunch of wrapper functions around the attach / bind code. This is kinda just ignoring the Aurelia life cycle tho.

    bindLogicCompleted = false;
    // Default to completed
    bindLogicPromise = new Promise((resolve) => {
      this.bindLogicCompleted = true;
      resolve();
    });
    
    timeToWaitBetweenStates: number = 500;

    bind() {
        //this.log("Bind method called");
        
        this.bindLogicPromise = new Promise((resolve) => {
            this.log("Bind logic called");
            
            // Simulated data load
            setTimeout(() => {
                
                this.bindCompleted = true;
                resolve();
                
                this.log("Bind logic done");
              },
              this.timeToWaitBetweenStates 
          );
        })
        
        //this.log("Bind method done");
    }

    attached() {
        //this.log("Attached called");
        
        this.bindLogicPromise.then(() => {
          this.log("Attached logic called");
          // Do attached logic
          this.log("Attached logic done");
        })
        
        //this.log("Attached method done");
    }

Thanks for all your input on this.

[bigopon]

@Spaceman1861 There is nothing preventing you from moving data loading to the child element. What I was suggesting was to offload change the logic of delaying attached to a simple visibility toggle, either in parent or child. Glad you figured it out though