Contact Manager tutorial: Pub/Sub with EventAggregator issue

Hello.

Currently I am walking through the Contact Manager tutorial of Aurelia v1 again.

I have an issue with the code logic in section Adding Pub/Sub Messaging. It seems that the implemented pub/sub messaging with EventAggregator does not fix all the issues that are described at the start of that section.

When making a change to a contact and then attempting to navigate away but cancelling the confirmation message, the contact details of the “dirty” contact remain visible in the details view (as expected), but the selected item in the contact list does change to the newly clicked contact. (Subsequently clicking on any other contact in the list will keep asking for a confirmation for navigating away, until either the dirty contact itself is selected in the list again, the dirty contact’s changes are saved using the Save button, or the confirmation for navigating away is accepted by pressing its OK button.)

When debugging the behavior in the browser, it seems to me that the select(contact) method in the contact list viewmodel is called several times during this process. First with the correct value (by the EventAggregator’s subscribed callback function), but then once again with the wrong value (due to continued event handling in the contact-list component?)

Or have I missed something else, perhaps?

Bart,

I never tried the contact tutorial, but I think the select method is being called multiple times is because the EventAggregator subscriptions are not being disposed. Try changing the subscription like this:

this.viewSubscription = ea.subscribe(ContactViewed, msg => this.select(msg.contact));
this.updateSubscription = ea.subscribe(ContactUpdated, msg => {

then add this detached method:

detached() {
this.viewSubscription.dispose();
this.updateSubscription.dispose();
}

I haven’t tried this, but I think it will solve the problem.

Rich

1 Like

As another possibility, that might not be code related since you don’t show anything.

When I am debugging and reloading multiple times using HMR I find that the app/browser gets all wacky and starts calling lifecycle stuff multiple times. I have to close that tab and reopen the app in a new tab as refresh doesn’t seem to get it back to normal.

Hi Rich,

Thanks for this information about the disposable subscriptions. I was not aware that such subscriptions are disposable. Since I am convinced that disposable resources should always be disposed when possible (because otherwise they would not need to be disposable in the first place), I followed your advice by storing the subscriptions in private member fields and disposing them in the detached lifecycle hook.

It’s a pity that the component lifecycle does not seem to support a disposed hook that complements the created hook. Using the detached hook for disposing resources without adding the complementing logic for creating those resources in the attached hook feels a bit messy IMHO. It immediately raises the question what will happen if the custom element is manually detached and attached again, or when it is being detached/attached by an if binding command/attribute… However, putting the subscriptions in the attached hook instead of in the constructor or the created hook will introduce other issues, so that does not seem to be a good solution. Unless the subscriptions are created in both the constructor and the attached hook , which will add additional complexity. I will investigate this further.

Anyway, however, the behavior remains exactly the same with this disposal logic in place. So the disposal logic is certainly very useful, but sadly it does not fix the issue at hand.

Hi airboss,

Yes, I already suspected that the refreshing logic of webpack c.q. the browser could be an issue. But the behavior remains the same even when I completely stop webpack’s web server, completely close the browser and start the app from scratch using au run --open .

So for now I tend to think that this issue is not webpack/HMR/browser-related.

Bart,

I always subscribe in the the attached method and have never had any complications. Not disposing a subscription causes 2 problems. One is memory leaks. The other is if you go to a component, leave it and come back again, not only will your current subscription be triggered, but the subscription from the first visit will be triggered as well. If your subscription is supposed to cause a record to be added to a table, for instance, you might see the record added multiple times, because the old subscriptions are still firing.

Rich

Hi Rich,

The specific complication in my case (in the contact manager tutorial) with creating the subscriptions in the attach hook instead of in the constructor or the created hook is that the correct contact will not be selected as the active contact in the contact list when the app is started using an URL for a specific contact (for example http://localhost:8080/contacts/2).

So in my case (in the contact manager tutorial), the contact list component would need to be able to create subscriptions in both the constructor (or created hook) and the attach hook. I created a separate private method for that purpose, which creates the subscriptions if they are not yet created. In the detach hook, I dispose the subscriptions and reset them to undefined or null, so that a subsequent call of the attach hook will recreate the subscriptions.

You are correct about those issues when subscriptions are not correctly disposed. I fully agree with you in that regard. :slight_smile: :+1:

However, I have been thinking about this contact manager app for a while now. It’s a very simple app. It seems that the contact list is instantiated only once inside the App component and is kept alive during the entire application’s lifetime in the browser. So it’s a sort of singleton here. I think that’s also the reason why disposing the subscriptions did not get any attention from the author. But when developing production-worthy components, I personally would still add code to correctly create and dispose such subscriptions, just to be consistent, even when the component will be instantiated only once.

Meanwhile, I have found a possible solution to the issue. I will post it in a follow-up message shortly.

Bart

Hi Rich,

The specific complication in my case (in the contact manager tutorial) with creating the subscriptions in the attach hook instead of in the constructor or the created hook is that the correct contact will not be selected as the active contact in the contact list when the app is started using an URL for a specific contact (for example http://localhost:8080/contacts/2).

So in my case (in the contact manager tutorial), the contact list component would need to be able to create subscriptions in both the constructor (or created hook) and the attach hook. I created a separate private method for that purpose, which creates the subscriptions if they are not yet created. In the detach hook, I dispose the subscriptions and reset them to undefined or null, so that a subsequent call of the attach hook will recreate the subscriptions.

You are correct about those issues when subscriptions are not correctly disposed. I fully agree with you in that regard. :slight_smile: :+1:

However, I have been thinking about this contact manager app for a while now. It’s a very simple app. It seems that the contact list is instantiated only once inside the App component and is kept alive during the entire application’s lifetime in the browser. So it’s a sort of singleton here. I think that’s also the reason why disposing the subscriptions did not get any attention from the author. But when developing production-worthy components, I personally would still add code to correctly create and dispose such subscriptions, just to be consistent, even when the component will be instantiated only once.

Meanwhile, I have found a possible solution to the issue. I will post it in a follow-up message shortly.

Bart

I have found a possible solution to my issue. I don’t like it. It smells odd. So any critical comments are very welcome.

My idea was to somehow avoid executing the system event handling of the clicked item in the contact list when the user cancelled the confirmation to navigate away from an unsaved (dirty) contact.

Currently, the contact details view publishes a ContactViewed message when the user cancels the confirmation for navigating away. In my solution, I introduced a new message called NavigatingAwayFromDirtyContactCancelled. (Yes, I agree that the name is somewhat long, so suggestions for any other self-documenting names are very welcome.)

messages.ts:

export class NavigatingAwayFromDirtyContactCancelled {
}

In the contact details component, I publish this new message instead of the ContactViewed message in the canDeactivate method:

contact-detail.ts:

canDeactivate() {
  if (!areEqual(this.originalContact, this.contact)) {
    const result = confirm('You have unsaved changes. Are you sure you wish to leave?');

    if (!result) {
      this.ea.publish(new NavigatingAwayFromDirtyContactCancelled());
    }

    return result;
  }

  return true;
}

In the contact list component, I introduced a new private boolean field called avoidSelectionChange. The subscription to the new NavigatingAwayFromDirtyContactCancelled simply sets this flag to true.

contact-detail.ts:

private avoidSelectionChange = false;

constructor(private api: WebAPI, ea: EventAggregator) {
  ea.subscribe(NavigatingAwayFromDirtyContactCancelled, () => this.avoidSelectionChange = true);
  ea.subscribe(ContactViewed, msg => this.select(msg.contact));
  ea.subscribe(ContactUpdated, msg => {
    const id = msg.contact.id;
    const found = this.contacts.find(x => x.id == id);
    Object.assign(found, msg.contact);
  });
}

With this logic in place, the select(contact) method can now evaluate the avoidSelectionChange flag to determine what should be done:

contact-detail.ts:

select(contact) {
  // Check if the currently selected contact should not be changed (because navigating away was cancelled by the user).
  if (this.avoidSelectionChange) {
    // Reset the `avoidSelectionChange` flag so that subsequent calls will not be blocked.
    this.avoidSelectionChange = false;
    // Do not continue event handling for the clicked contact in the list.
    return false;
  }

  this.selectedId = contact.id;
  return true;
}

This logic seems to work fine for me, at least in this contact manager tutorial. As I said, I do not consider it to be a nice solution. I hope that more elegant solutions will be possible, either now or in the future.

Event handling can get pretty messy very fast if not covered the right way. Now as an alternative you may look into the same app with using the Aurelia store plugin. https://github.com/zewa666/aurelia-store-examples/tree/master/contacts

If the same issue exists there, we’re up to another issue. Btw also rxjs subscriptions are unsubscribable aka disposable but left out as the original sample didn’t either do it

Hi zewa666,

Yes, I agree with the event handling mess. :slight_smile:

So I looked briefly into your Aurelia store examples repository.

Hmmm. It seems that the contacts app in your repository behaves even worse. :cry:

After opening the application in the browser, I clicked through the five demo contacts to see their details. All fine.

Then I selected the first contact (John Tolkien) and make some changes but did not save them. Then I clicked the last contact (Roger Green) and I got the confirmation dialog saying that I have unsaved changes and asking if I wanted to leave. I clicked the Cancel button.

As a result, not only was the selection changed to Roger Green :face_with_raised_eyebrow:, but after clicking on John Tolkien again, it seemed that my changes were gone as well. :open_mouth: More shockingly, clicking on the second contact (Clive Lewis) showed details about Roger Green! :astonished: Clicking the third contact (Owen Barfield) worked fine. Clicking back on the second contact (Clive Lewis) showed the details of Clive Lewis again. :relieved: Clicking on Roger Green next was fine as well. But then clicking on Charles Williams showed the details of Clive Lewis! :scream:

Finally, when opening a specific contact detail URL, the correct details are shown, but the specified contact will not be marked as the active contact in the list. :neutral_face:

I will certainly look into the Aurelia store plugin, but I am not sure yet if it will fix my issues. :thinking:

Thanks anyway.

Bart

Hi zewa666,

I just posted a reply to your message. But somehow it did not get marked as a reply to your message, it seems. Sorry if I did something wrong.

Bart

The power of proper user testing, as clearly shown not my strength :rofl:

So it seems all is reflected with the cancel, that’s what indicates it’s more of a logical issue of the app, besides those quick untested porting issues specific to the store version.

EDIT:
uff yeah, reading the message displayed when pressing cancel I see that the intention was a totally different one, in staying where you are and ignoring the navigation to the next entry while keeping data around. So yeah my implementation got messed up while porting over the original contacts app.