ractivejs / ractive

Next-generation DOM manipulation
http://ractive.js.org
MIT License
5.94k stars 396 forks source link

Components in an {{#each}} within an {{#if}} might not get teardown #3263

Closed giovannipiller closed 6 years ago

giovannipiller commented 6 years ago

Description:

I have an {{#each items}} which gets executed only {{#if items.length}}, and inserts multiple components <MyComponent/> in the page. Removing an element from items will trigger teardown for each instance of <MyComponent/> except the last one.

Looks like the {{#if items.length}} is getting in the way.

{{#if items.length}}
    {{#each items}}
        <MyComponent label="{{label}}"/>
    {{/each}}
{{/if}}

Versions affected:

0.10.9 (reproduced also on 0.10.4)

Platforms affected:

All

Reproduction:

JSFiddle. Same JSFiddle, without the {{#if items.length}}

const MyComponent = Ractive.extend({
  template: '<h1>Hello there {{label}}</h1>',
  onteardown() {
    this.parent.add('teardownCounter');
  },
});

// if you change your console context to "result", you can interact with
// your Ractive instance with the glboal "r"
const r = window.r = new Ractive({
  el: '#main',
  template: `
    <h1>Tore down components {{teardownCounter}}. Should be: {{teardownCounterShouldBe}}</h1>

    {{#if items.length}}
      {{#each items}}
        <MyComponent label="{{label}}"/>
      {{/each}}
    {{/if}}
  `,

  components: {
    MyComponent,
  },

  data: {
    // number of components tored down by splicing `items`
    teardownCounter: 0,
    teardownCounterShouldBe: 0,

    items: [{
        label: 'item 1'
      },
      {
        label: 'item 2'
      },
    ]
  },

  oncomplete() {
    setTimeout(() => {
      // will trigger a teardown properly
      this.splice('items', 0, 1)
      this.add('teardownCounterShouldBe')
    }, 1000)

    setTimeout(() => {
      // will not trigger a teardown for MyComponent!!
      this.splice('items', 0, 1)
      this.add('teardownCounterShouldBe')
    }, 2000)
  },
});

Edit 1: this example uses setTimeout(), but the same happens when splicing interactively (ex. clicking a button)

ceremcem commented 6 years ago

Obviously it can be solved with timing tricks: https://jsfiddle.net/cb1sd27u/17/ but I would also expect MyComponent to be unrendered when it is unrendered by {{if ...}}

giovannipiller commented 6 years ago

Yup, my actual code is much different and removes components interactively via a button.

evs-chris commented 6 years ago

I've tracked this down to an issue wherein a shuffled out item never gets fully unbound when its parent fragment unbinds before it can finish shuffling. I think I've got it fixed, but I need to write a few tests, which I'll try to do when my brain is a bit more functional :smile:

evs-chris commented 6 years ago

This should be fixed on edge and v0.10-dev once travis finishes travising. Thanks for the report!

giovannipiller commented 6 years ago

Thanks Chris! While this is fixed on my example, I'm still experiencing the issue on my app. I'm trying to figure out what's going on but, as always, it's in a super deep component structure 😅

I'm working on a fiddle, though the only strange thing I discovered so far is #3265. Could that be somehow related? No idea yet..

My scenario involves decorators within {{#if ...}} and {{#each ...}}, however the simplest cases have been already fixed by d99c5a0.

Will update if I can isolate the problem.