repeat binding fails to maintain ordering of DOM nodes when item is removed and reinserted at a different location in the array

[insidewhy]

Copying the comment by @ben-girardet as it's in the closed issue #233 and I fear that what seems like a very important issue is being buried by being a comment in a closed issue:

// Option 1 (using splice)
this.steps.splice(newPosition, 0, this.steps.splice(previousPosition, 1)[0]);

// Option 2 (using sort)
let index = 0;
for(let step of this.steps) {
  step.position = (index < newPosition) ? index : index+1;
  if(index == previousPosition) {
    step.position = newPosition;
  }
  index++;
}
this.steps.sort((a,b)=>{
  return a.position - b.position
});

Both options have problems when the view gets updated. Sometime the moved item gets duplicated, sometimes the array gets an empty new item, sometimes an item gets missing, ...

Further to this I (@ohjames) have found that it also triggers if I splice the entire array to empty it and then insert all the new items to represent the new order. The item that was moved ends up in the wrong place, duplicated or busted.

[EisenbergEffect]

@jdanyow @martingust We need to look into this immediately. This is highest priority.

[jdanyow]

@ohjames / @ben-girardet we can look into this if you can provide a reproduction of the issue:

https://gist.run/?id=7542e061bc940cde506b

[insidewhy]

I've moved on from Aurelia so won't be providing a reproduction, it is however very trivial to reproduce. Fail to react to this gravely important issue at Aurelia's peril.

[CasiOo]

Option 1 seems to be working fine in this gist
https://gist.run/?id=ce2bb5b6a65610116aa3517007d2eb33

[EisenbergEffect]

I tested both option 1 and option 2 and could not reproduce any issues. The second option did have invalid JavaScript code though (if used with an array of strings). Here's a correct version that can be dropped into the gist above:

export class App {
  steps = [new Num(1), new Num(2), new Num(3), new Num(4), new Num(5)];

  mutate(previousPosition, newPosition) {
    let index = 0;

    for(let step of this.steps) {
      step.position = (index < newPosition) ? index : index+1;
      if(index == previousPosition) {
        step.position = newPosition;
      }
      index++;
    }

    this.steps.sort((a,b)=>{
      return a.position - b.position
    });
  }
}

class Num {
  constructor(value) {
    this.value = value;
  }

  toString() {
    return this.value.toString();
  }
}

[sebastien-roch]

@EisenbergEffect I could reproduce the issue but with the <compose> tag: https://gist.run/?id=fda383ab7b92edbb39ab26c2f1db7bc7

Actually I noticed that the containerless attribute is the culprit here. When not present, it works well.

[EisenbergEffect]

@sebastien-roch So, you are saying that when you have a repeat, with a compose inside of that repeat and there's a containerless attribute on the compose, that's when it breaks?

[EisenbergEffect]

That's something I can go on. With that information I have an idea already.

[sebastien-roch]

@EisenbergEffect at least it's when it breaks for me. I could only reproduce this problem with compose though, don't know if the described issue happen in other cases.
The repeat.for is set on the <compose> tag itself, check out the gist.

[EisenbergEffect]

Yes. I have a suspicion about what might be the cause. I'll test it out later today. Thanks for tracking down that detail!

[EisenbergEffect]

@sebastien-roch Thanks for the info. It's fixed and will go out in the next release in the next day or two.