Report a single high-level mutation when adding/removing non-last array elements

[christianliebel]

Adding/removing non-last array elements leads to a chain of update changes, before the actual delete/add change is reported on the last array element. I can understand this behaviour from the proxy’s perspective (and I know that you try to be close to the Proxy behaviour), however as a developer I’d prefer to get the high-level perspective of which array element was actually affected.

Library version tested against: 0.0.8

Repro:

const ObservableSlim = require('observable-slim');
const proxy = ObservableSlim.create([0, 1, 2, 3]);

ObservableSlim.observe(proxy, changes => changes
  .forEach(change => console.log(change.currentPath, change.type)));

console.log(proxy);

proxy.splice(0, 1); // delete “0” from index 0
console.log(proxy);

proxy.splice(0, 0, 0); // add “0” at index 0
console.log(proxy);

Result:

[ 0, 1, 2, 3 ]
0 update
1 update
2 update
3 delete
length update
[ 1, 2, 3 ]
3 add
2 update
1 update
0 update
[ 0, 1, 2, 3 ]

Expected:
The library should either report the actual affected index for delete/add changes on arrays or introduce a new (additional) change type addElement/deleteElement that reports the actual index. Interestingly, there seems to be no change fired on the length property when you add items to an array (I tried it with both splice and push).

[ElliotNB]

I can understand this behaviour from the proxy’s perspective (and I know that you try to be close to the Proxy behaviour), however as a developer I’d prefer to get the high-level perspective of which array element was actually affected.

There was another user who had a similar enhancement request a couple weeks back -- essentially to provide a more user-friendly and concise description of what mutation just occurred. Unfortunately there are a few things that make this difficult if not impossible without some major sacrifices to other parts the library functionality.

As far as I know, there we two ways you could accomplish the desired behavior. You could either:

1. Wrap the Array mutation methods so you can capture the push, splice, etc invocation before the value is mutated.
2. Permanently enable domDelay so that changes are reported in batches and if the set of array mutations matches the pattern for a push, splice or anything else, then emit a custom event for that array mutation rather than the batch of raw mutations.

The first option isn't really viable because wrapping Array mutation methods is just asking for a conflict with other libraries and the ES5 Proxy polyfill already wraps the array mutation methods (i.e., things would get complicated and slow if we double wrapped the Array mutation methods).

The second option seems possible, but it will have it's own challenges. I'm not certain if all browsers implementation of Proxy reports change events with a push, splice, etc in the same order. The second option would also prevent us from fulfilling another enhancement request where we allow users to intercept changes. I'm thinking it might be best if the second option were enabled as a special mode that's turned off by default. Either way, it'll take quite a bit of time and effort to put together that functionality and verify it's working across all major browsers and the polyfill.

[ElliotNB]

Interestingly, there seems to be no change fired on the length property when you add items to an array (I tried it with both splice and push).

The issue you caught is a very interesting one. It's caused by an oddity with how the Proxy intercepts changes. In some cases, you cannot intercept length mutations.

For example, if you perform a push on an Array, the set handler gets invoked twice -- once for appending the new item and another time for modifying the length. However, when you attempt to retrieve the previous length via receiver[property] -- the new Array length is already set so there is no opportunity to retrieve the previous value. Because the value doesn't actually change when the set handler is invoked, is why the library wasn't reporting out the length mutation.

I'm going ahead with a little workaround enhancement/bug fix to resolve this issue so length changes are always reported accurately.

@christianliebel Thank you for reporting these issues! These are some excellent catches.

[ElliotNB]

@christianliebel I just pushed out the fix for the length issue you reported and I'll be publishing 0.0.9 on npm shortly.

The enhancement you've requested will take some time to finish, but I'm hoping in the meantime that accurate reporting on Array length mutations, will help you sufficiently detect push, splice, etc events.

[christianliebel]

@ElliotNB Awesome, thanks a lot. I’ll try detecting array additions/deletions from the stack of changes, but I’d agree that an opt-in, library-provided method would be preferable.

Thank you for reporting these issues! These are some excellent catches.

You’re welcome! 🤘 Thanks for your fast help and support.

[joharzmn]

@christianliebel I am also trying to get additions/deletions from changes in proxyArray. Any luck in detecting additiuons/deletions?

[christianliebel]

@joharzmn Yes, I’ve enabled the domDelay functionality and process the update batches, unfortunately, I can’t share the code. There’s a certain pattern of changes when entries are added to or deleted from an array. Then, I replace this pattern of changes by a single event.

[ElliotNB]

@christianliebel Have you noticed any differences between how Chrome, Firefox, Edge, etc report Proxy events for splice mutations? Any differences in the order of the steps?

I've been meaning to get this enhancement built out...

[christianliebel]

@ElliotNB No, I haven’t specifically tested the Proxy behavior on different browsers. But in the context of the application, the following workaround seems to work across browsers.

@joharzmn This is my workaround:

/**
 * This is a workaround that tries to squash a chain of change events to a single high-level mutation event.
 *
 * Example: Deleting an array entry
 * const arr = [0, 1, 2]
 * arr.splice(0, 1); // arr = [0, 1]
 *
 * Leads to this chain of events:
 * { type: 'update', property: '1', newValue: 2, oldValue: 1 }
 * { type: 'delete', property: '2', newValue: null, oldValue: 2 }
 * { type: 'update', property: 'length', newValue: 2, oldValue: 3 }
 *
 * However, the application is only interested in high-level events:
 * { type: 'delete', property: '1', newValue: null, oldValue: 1 }
 *
 * @param changes
 */
export function squashDeleteMutationChanges(changes) {
  let activeArray = null;
  const processedChanges = [];

  function squashChanges(end) {
    processedChanges.push({...changes[end], type: 'delete', newValue: null});
  }

  changes.reverse();
  changes.forEach((change, index) => {
    const {type, property, target, newValue, previousValue} = change;
    if (type === 'update' && property === 'length' && Array.isArray(target) && newValue < previousValue) {
      if (activeArray) {
        squashChanges(index - 1);
      }
      activeArray = {reference: target, start: index};
    } else if (activeArray && !isNaN(property) && target === activeArray.reference) {
      activeArray.end = index;
    } else {
      activeArray = null;
      processedChanges.push(change);
    }
  });

  if (activeArray) {
    squashChanges(activeArray.end);
  }

  processedChanges.reverse();
  return processedChanges;
}