breaks on {{#if}} direct inside {{#vertical-collection}}

[luxzeitlos]

This template:

<section>
  {{#vertical-collection data
    minHeight=10
    containerSelector="body"
    alwaysRemeasure=true
    as |item|
  }}
    <section>
      Item {{item.id}}
    </section>
    {{#if item.detail}}
      <section>
        Detail
      </section>
    {{/if}}
  {{/vertical-collection}}
</section>

with this data:

data: Em.computed({
  get() {
    const arr = [];
    for(let i = 0; i < 1000; i++) {
      arr.pushObject({
        detail: true,
        id: i
      });
    }

    return arr;
  }
})

breaks with

Error: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.
at Object.clear (ember.debug.js:45707)
at UpdatableBlockTracker.reset (ember.debug.js:46044)
at TryOpcode.handleException (ember.debug.js:54858)
at UpdatingVMFrame.handleException (ember.debug.js:55040)
at UpdatingVM._throw [as throw] (ember.debug.js:54753)
at Assert.evaluate (ember.debug.js:50122)
at UpdatingVM.execute (ember.debug.js:54740)
at RenderResult.rerender (ember.debug.js:54679)
at RootState._this.render (ember.debug.js:12161)
at runInTransaction (ember.debug.js:23478)

I'm not sure if this should be supported, but it AFAIK worked in smoke-and-mirrors.

[scottmessinger]

I ran into the same error when I had an each as the only thing in the vertical-collection block. Given I was having the same error as using an if, I figured it might be because there's a tagless component so I wrapped the each in a div and things worked. Any ideas on how to fix it? Or where to look to patch it?

[pzuraq]

This is related to #24, it has to do with the way we are moving DOM around to avoid thrashing Glimmer. We may be able to avoid these issues if we remove the .virtual-component-renderer div and just render the VCs directly to the main component element, but that could also cause more issues - we'll have to play around with it.

It may be a hard requirement to have a stable root element in place, similar to ember-wormhole, until the Custom Components RFC lands. I'm hoping that will enable us to extend Glimmer in a better way than we are currently.

[luxzeitlos]

Should we treat this as a documentation issue then for now?

[pzuraq]

I'm going to try to solve it for beta 2 next week, but if that doesn't work then yes we should.

[runspired]

@pzuraq with the virtual bounds this should not be an issue. We may have implemented the bounds moving logic incorrectly.

[pzuraq]

Yeah, that's what I'm thinking. I think we need to go back to the original design you implemented, where the virtual bounds were used for the VC each instead of a div container, that's what's tripping Glimmer up. I'll be able to take a look at it first thing next week.

[pzuraq]

@scottmessinger @luxferresum I should be getting beta.2 out this week or early next with this patch in! Let me know if the issue is fixed on your end, I think this should be solid

[luxzeitlos]

I can confirm that it's now working on master.
For now this is a "works on my machine", but I will push it to testing as soon beta.2 is out.

[HenryVonfire]

I'm having the same error with 1.0.0-beta.13 while trying to change the collection of items while filtering them @pzuraq @runspired

Is vertical-collection meant to work only with non-mutable collections?