Cannot set property '$this' of undefined.

[Kanaye]

Found in the shopping example.
Steps to reproduce:

• open the shopping-example
• click under "CATEGORIES" on "UNDERWEAR"
• click under "BRANDS" on "RACH"
• click under "CATEGORIES" on "ALL"

I have no time to spend on this today but I can look into this later this week.

[Kanaye]

Short update as I will have a short break (hopefully just a view hours) on working on this:
This bug is very strange:
In some cases blocks is assigning the (in my opinion) wrong child contexts to context.childs.
e.g. here the context.childs of the div with the data-query each(pages) equals (sometimes, including pageload) the context.childs of the div with the data-query each(products.view).

[Kanaye]

Okay not as strange as I thought.
The bug occurs cause children rendered with VirtualElement.renderChildren() will share there parents context and therefore the context.childs property.
To be more clear:

<!-- some data-query using renderChildren e.g. view -->
<div data-query="view(Somview)">
   <!-- these queries are sharing their context.childs -->
    <div data-query="each(first)">{{$this}}</div>
    <div data-query="each(second)">{{$this}}</div>
</div>

If now one array observable gets emptied (e.g. first.reset()) this query will empty the context.childs array.
If now the other gets updated (e.g. second.reset(['some stuff'])) the VirtualElement.updateChildren function will try to acces the context in the context.childs array for the updated items element.
But as the context.childs array is empty it will fail.

Maybe all elements should get there own context, but this could/will lead to other problems.

Let me think about a solution as this is more complex.

[astoilkov]

Wow. This actually seems extremely complex issue. I don't have an idea how it could be fixed. If you have hard times figuring this out we could take a look at it together. Let met know.

[Kanaye]

Sorry, I was really busy the last 3 months.
I hope I have more time to spend on jsblocks now.

[Kanaye]

I might have parts of a solution for this.

We could remove the context.childs array.
As we need the context to be the same object accross some elements we can't clone it (or something like that).
I'm thinking about just pushing a new context for each child of an each-query and recalculating the $index for new items.
It seems to work fine (but I haven't tested much yet) with one exception:
I have to think about how to update the index of existing elements that don't get rerendered, but it sholdn't be that big of a deal.

I will post here if I have any updates.

[astoilkov]

Another idea. You could also take a look at the code before the context.childs was added: https://github.com/astoilkov/jsblocks/tree/8d3b0cb051a9ed847c6d0a2e165f192500173ab4

[Kanaye]

Intresting enough, that's the first thing I had in mind to solve this.
Adding a index-array to array-observables with an observable containing the index.
That avoids to have to mess with the contexts.
But I'm not sure that I like that solution. Let me think about that.

[Kanaye]

I worked on this Issue today but stumbled accross one problem, while testing within the shopping-example.
DomQuery.createElementObservableDependencies() calls itself in some case.
But it set's the DomQueries _context to null.
This causes dom-queries of following elements in the caller-createElementObsservableDependencies-function to get executed without or with a wrong context.
Removing it doesn't let the tests fail and the shopping-example seems to work fine.
But I didn't had the time to test everything or to search for cases where this might be needed.
Do you remember why you added that line ?

[astoilkov]

Yeah. I kinda remember. First I wrote the method without the clearing of the this._context. Then I removed it because I was thinking there is no need for it and then I found a scenario where it wasn't working without it and I returned it. It seems that I returned the code when I was doing performance improvements.

It is possible another fix to have resolved this so I encourage you to remove it test it more and then push it.

[Kanaye]

I created an public branch with my current version of a fix for this. But I want to test a little bit more before I open a PR.

To prevent bugs from beeing recreated we should consider to created an "issue" category in the tests with test cases for issue that are to complex or involve other components so they don't fit in the other categories.

What do you think ?

[astoilkov]

Yeah. I think this is a good idea. You could try it out and open a PR and I will review the final idea behind the tests. :)