nx-js / observer-util

Transparent reactivity with 100% language coverage. Made with ❤️ and ES6 Proxies.
MIT License
1.2k stars 94 forks source link

Bug or user error? Copying observable to other position in array converts it to the raw item #49

Open kwaclaw opened 4 years ago

kwaclaw commented 4 years ago

In this code, "this.items" is an observable array, that contains items that are observables themselves. When I run this code:

 moveItem(from, to) {
    if (from === to) return;

    // this algorithm keeps the array length constant
    const itemToMove = this.items[from];
    if (to > from) {
      this.items.copyWithin(from, from + 1, to + 1);
    } else if (to < from) {
      this.items.copyWithin(to + 1, to, from);
    }
    this.items[to] = itemToMove;
  }

then all the items that are affected by the copy operations, that is, items affected by copyWithin() and also items affected by plain assignments, like in this.items[to] = itemToMove where itemToMove is a proxy, get replaced by their raw items inside the array. So what started oput as an array of proxies ends up as a mixed array of proxies and raw items.

I found one workaround by operating on the raw array first, but it is not elegant:

moveItem(from, to) {
    if (from === to) return;

    const items = raw(this.items);

    // this algorithm keeps the array length constant
    const itemToMove = items[from];
    if (to > from) {
      items.copyWithin(from, from + 1, to + 1);
    } else if (to < from) {
      items.copyWithin(to + 1, to, from);
    }
    items[to] = itemToMove;

    // we made changes on the raw array, because copyWithin and assignments applied to the proxy
    // 'this.items' strip the copied/assigned array elements of any proxies that might wrap them.
    // So we need to trigger a reaction explicity on this.items by clearing and re-assigning to it.
    this.items = [];
    this.items = items;
  }

Am I missing something? Is there a better way to do this?