Observing Array fires mutated value twice

[ahmedkandel]

Description

I am using a javascript framework which depends on observable-membrane for reactivity. While observing array at pushing new indexes the valueMutated() gets fired twice. So after digging i found that it is fired in the set() trap of ReactiveProxyHandler when pushing new index which is what expected but also is fired directly after that as the array length is changing which is a conseguente of pushing new index.

Steps to Reproduce

Observe an array then push new item to the array.

Expected Results

valueMutated() should be fired once after pushing to an observed array.

Actual Results

valueMutated() is fired twice after pushing to an observed array.

Possible Solution Removing "else if" conditional statement at https://github.com/salesforce/observable-membrane/blob/master/src/reactive-handler.ts#L80 As it was a fix for issue number 236 which may be is related to the multi-package from where observable-membrane was extracted so may be it is not needed anymore.

[pmdartus]

This is the expected behavior of the membrane, it tracks all the mutations on the wrapped object.

The valueMutated hook is invoked twice in this case because a single item is pushed to the array. Generally speaking, invoking Array.protype.push does 1+n operations on the underlying array (where n is the number of arguments passed to the method):

• set the new array length property once
• set the n arguments to the array

Example: live

const membrane = new ObservableMembrane({
    valueMutated(target, key) {
        console.log('mutated ', key);
    },
});

const arr = membrane.getProxy(['a']);
arr.push('b', 'c');

mutated  1
mutated  2
mutated  length