mount event not fired due to listener for wrong event (mount instead of updated) #2249

Closed
trassmann opened this Issue Feb 6, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@trassmann

Help us to manage our issues by answering the following:

1. Describe your issue:

In 3.1.1 there was a change to the way mount is triggered for looped custom child tags.

See this code in the mount function:

// if it's not a child tag we can trigger its mount event
if (!this.parent) {
  this.trigger('mount');
}
// otherwise we need to wait that the parent "mount" or "updated" event gets triggered
else {
  getImmediateCustomParentTag(this.parent).one(!this.parent.isMounted ? 'mount' : 'updated', function() {
    this$1.trigger('mount');
  });
}

This causes an issue for some of our code, since you add the listener to the immediateCustomParentTag but the check for which event to listen to (mount or updated) is on this.parent. The check however should be on the immediateCustomParentTag:

// if it's not a child tag we can trigger its mount event
if (!this.parent) {
  this.trigger('mount');
}
// otherwise we need to wait that the parent "mount" or "updated" event gets triggered
else {
  var immediateCustomParentTag = getImmediateCustomParentTag(this.parent);
  immediateCustomParentTag.one(!immediateCustomParentTag.isMounted ? 'mount' : 'updated', function() {
    this$1.trigger('mount');
  });
}

Without this change, the child tags in my code never trigger a mount, only a before-mount, even though they are on the DOM. I am not sure if this is an adequate fix or if I'm missing the big picture here (maybe this.parent or this.parent.isMounted is incorrect in some cases?), but this seems to fix all my issues and logically makes sense. If I'm right here, I'm glad to make a PR.

TL;DR Code should be listening for updated on the parent to trigger mount on the child, but instead is listening for mount to trigger child mount on an already mounted parent tag, hence never triggering mount on the child.

4. Which version of Riot does it affect?

  • 3.1.1

5. How would you tag this issue?

  • Question
  • Bug
  • Discussion
  • Feature request
  • Tip
  • Enhancement
  • Performance
@GianlucaGuarini

This comment has been minimized.

Show comment
Hide comment
@GianlucaGuarini

GianlucaGuarini Feb 6, 2017

Member

@trassmann thanks for reporting it

Member

GianlucaGuarini commented Feb 6, 2017

@trassmann thanks for reporting it

@GianlucaGuarini

This comment has been minimized.

Show comment
Hide comment
@GianlucaGuarini

GianlucaGuarini Feb 6, 2017

Member

fixed in riot@3.2.0

Member

GianlucaGuarini commented Feb 6, 2017

fixed in riot@3.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment