Tags mounted in a loop inherit all properties from their parent.. Why? #1697

Closed
johnsq opened this Issue Mar 21, 2016 · 7 comments

Projects

None yet

4 participants

@johnsq
johnsq commented Mar 21, 2016

In Riot (current version and previous), when a single tag is added, like.. <item></item>, viewing this in the console, within that tag, shows a standard list of properties (isMounted, _riot_id etc). However, when you add any number of tags via an 'each' loop like.. <item each={item in items}></item> every property (including functions) from the parent tag are added to this list. What is the reasoning for cloning all of this data into every child?

see here..

riot-tag-loop-question

It seems weird. The only extra properties you would expect to to find in each child are the properties of the 'item' you are explicitly adding, but in fact the 'item' you are explicitly adding actually ends up being referenced in each child three times (once in item again in _item.item and finally in the clone of items, the parent array of which it originally came). Is this a bug?

@GianlucaGuarini
Member

It's because many users expect to have the access to properties of the parent ( without using parent.prop), technically the handling of these properties is done here.
Basically riot users expect this behavior as default but I also not 100% sure it's a good idea, we probably could remove this in riot 3.0.0 #1694

@GianlucaGuarini
Member

This pull request is from another user that requires exactly the opposite feature

@GianlucaGuarini GianlucaGuarini referenced this issue Mar 22, 2016
Closed

Riot 3.0.0 roadmap #1694

14 of 16 tasks complete
@johnsq
johnsq commented Mar 22, 2016

Thanks for your response. Just to be clear, my main concern with this is regarding performance. Concerned that tags in long lists are processing and holding a lot of duplicate data that they don't need. Do you think that this issue does impact performance in any significant way?

@GianlucaGuarini
Member

I don't think it has a big impact on the riot performances ( test using current riot release using no-reorder ). But I also would prefer to change this behavior, I am not the only one who take this kind of decisions here also the other core contributors should agree on this

@sylvainpolletvillard
Contributor

Is there any reason this "inheritance" mechanism is done by cloning data and mixing objects, rather than looking up in the parent chain until a value is found, just like lexical scoping ? I am thinking of something like :

var tag = this;
while(!(prop in tag) && tag.parent){
    tag=tag.parent;
}
return tag[prop];

I found this feature useful, because forgetting parent. is a common mistake and relatively difficult to figure out. But mixing objects is definitely a bad idea, and makes things very confusing while debugging.

@GianlucaGuarini GianlucaGuarini added this to the 3.0.0 milestone Apr 26, 2016
@GianlucaGuarini
Member

This issue has been added to our TODO list and we will fix it in riot@3.0.0 #1694

@senica
senica commented May 28, 2016 edited

I'll add some commentary here as I was directed to this issue from gitter. I'm not sure that my issue is directly related to this, but it's close enough. I have a tag that recursively calls itself. There are two issues with this and auto-inherited properties.

  1. If you are relying on an api, and the api does not give you full object sets, you may end up with a case where the parent tag has the correct data, but the child wrongly inherits some property from the parent. One of the things I love about riot is it does not make a lot of assumptions for you like other frameworks do. This creates a little more programming, but much better control which I personally love. I feel this is one of those cases where you should be forced to reference the parent if you needed data from it.
  2. I ran into an issue in which is seems that the parent is modifying the children properties incorrectly. Below is a conversation from Gitter. See https://jsfiddle.net/senica/x15rw8a3/ , which does not work and this one https://jsfiddle.net/senica/x15rw8a3/12/ which does. Calling this.update, seems to overwrite the children property with the parent property....? The only change is the hide function towards the bottom

It seems that passing in attributes to the update function causes this as it works with the below code.

this.hide = function(e){
      e.stopPropagation();
      e.preventUpdate = true;
      this.visible = !this.visible;
      this.update();
    }

as seen here https://jsfiddle.net/senica/x15rw8a3/14/
Same functionality occurs on the alpha channel, btw

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