New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refs clobbered into array with mounted sub-tags #2329

Closed
fabien opened this Issue Apr 26, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@fabien
Contributor

fabien commented Apr 26, 2017

  1. Describe your issue:

I'm trying to mount a tag dynamically from code within a tag, like this:

var options = { parent: this };
riot.mount(this.refs.nested, 'nested-tag', options);

As you can see, I'm using a ref to target the DOM element to use. While this works perfectly fine, I have noticed an interesting side-effect of ensuring a proper parent is set for this tag.

After mounting, this.refs.nested no longer returns the DOM element, but an array with the DOM element first, and the mounted tag instance second. Since I'm still using jQuery, this behavior went unnoticed for a while, because $(this.refs.nested) would simply ignore the tag within that array.

I know this is not the standard way to mount tags dynamically, as the declarative data-is approach can be used as well, but I prefer to do it in code, and pass additional options as shown.

Without passing the parent this issue goes away, but I'd like to preserve the proper hierarchy.

  1. Can you reproduce the issue?

http://plnkr.co/edit/FZ4UT5BzGamTjLfrj4wi?p=preview (logged to console)

  1. On which browser/OS does the issue appear?

Chrome Mac, latest

  1. Which version of Riot does it affect?

v. 3.4.3 (latest) and earlier

  1. How would you tag this issue?
  • Bug

@fabien fabien changed the title from Refs clobbered with manual mounted sub-tags to Refs clobbered into array with mounted sub-tags Apr 26, 2017

@fabien

This comment has been minimized.

Show comment
Hide comment
@fabien

fabien Apr 26, 2017

Contributor

I think it's related to the code around this line:

https://github.com/riot/riot/blob/master/lib/browser/tag/ref.js#L29

Contributor

fabien commented Apr 26, 2017

I think it's related to the code around this line:

https://github.com/riot/riot/blob/master/lib/browser/tag/ref.js#L29

@fabien

This comment has been minimized.

Show comment
Hide comment
@fabien

fabien Apr 30, 2017

Contributor

Great, thanks!

Contributor

fabien commented Apr 30, 2017

Great, thanks!

@fabien

This comment has been minimized.

Show comment
Hide comment
@fabien

fabien Apr 30, 2017

Contributor

@GianlucaGuarini quick follow-up question: since the ref attribute is removed now, I assume this also means data-ref will no longer be an attribute. Should I stop relying on this as a selector for css styling?

Contributor

fabien commented Apr 30, 2017

@GianlucaGuarini quick follow-up question: since the ref attribute is removed now, I assume this also means data-ref will no longer be an attribute. Should I stop relying on this as a selector for css styling?

@GianlucaGuarini

This comment has been minimized.

Show comment
Hide comment
@GianlucaGuarini

GianlucaGuarini May 1, 2017

Member

yes please use data-is instead

Member

GianlucaGuarini commented May 1, 2017

yes please use data-is instead

@fabien

This comment has been minimized.

Show comment
Hide comment
@fabien

fabien May 9, 2017

Contributor

In hindsight, I think the attribute should be preserved (data-is is something different altogether, I was specifically referring to data-ref and its use as a regular attribute). Perhaps this explains #2338 too?

Contributor

fabien commented May 9, 2017

In hindsight, I think the attribute should be preserved (data-is is something different altogether, I was specifically referring to data-ref and its use as a regular attribute). Perhaps this explains #2338 too?

@bminer

This comment has been minimized.

Show comment
Hide comment
@bminer

bminer May 18, 2017

Contributor

See also issue #2348. data-ref and ref are no longer removed.

Contributor

bminer commented May 18, 2017

See also issue #2348. data-ref and ref are no longer removed.

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