AttrNodes on a ContainerView dont get destroyed when moving or removing the ContainerView

[ashleysommer]

I isolated this problem after spending a few weeks debugging issue #10601.

In Renderer_remove, the renderer decides whether to destroy or simply remove child objects depending on if the View is a ContainerView (ie, has a _childViewsMorph object attached). If it is not a ContainerView, the renderer marks all of the children to be destroyed, if it is a ContainerView, the children are removed without being destroyed.

When a regular View object is removed, all children including the AttrNodes are marked isDestroying to denote them destroyed. The problem occurs when a ContainerView is removed from the renderer, the AttrNodes don't get marked isDestroying.

When the same ContainerView is added back into the renderer, it renderer creates new AttrNodes for the bound attributes, and adds them to the _attrNodes array along with the old ones. This is correct behaviour. When rendering the view, the AttrNode render() function simply skips out if isDestroying is true, in order to skip the old AttrNodes and to only render the newly created ones.

In this case, because isDestroying is false, the renderer tries to render two versions of the same AttrNode, and it crashes because _morph is null on the older one.

I believe the simplest solution is to modify Render_remove to check if an _attrNodes array exists on the view, loop through all of the objects in the _attrNodes array and mark them all isDestroyed.

Edit: I am testing on 1.12.0-beta1. This is after PR #10795 and it is not caused by or fixed by that code.

[mixonic]

@flubba86 In the Glimmer PR existing AttrNodes code is completely rewritten. We've been focused on trying to ship that in 1.13 (landing it to master in the next week or so). This is the long-term fix.

After this PR the childView() function should be returning both attrNodes and views into the remove code. You've been looking at this on the stable branch? Or with 1.11.3?

[ashleysommer]

@mixonic I am using the Beta from the builds page, 1.12.0-beta1.
PR #10795 is applied.
You are right, the childView() function does indeed return all attrNodes and views.
The problem, as described in the original post, is that because the View I am removing is a ContainerView, (it has a _childViewsMorph object attached), the AttrNodes are placed in the removeQueue, not the destroyQueue, so they are not destroyed when they are removed.

When the ContainerView is inserted back in, it recreates all of the AttrNodes. This causes two identical AddrNodes to be rendered, but the old one does not have a _morph object so it crashes the renderer.

I fixed my issue by placing an additional check in Renderer_remove which manaully places all of the AttrNodes into the destroyQueue to ensure they are destroyed.

[rwjblue]

ContainerView is deprecated in Ember 1.13 and removed in Ember 2.0.0. Closing....