I have written a simple JavaScript function to make an array reactive (i.e. observable). Is there anything I could improve? What the code does is take an array, a callback and a this
value as arguments, then whenever the array gets updated or a value gets added to it, it runs the callback with the specified this
value. It works similar to the obsolete Array.observe()
.
function arrayObserve(array, callback, thisArg){
var result = [];
result.push.apply(result, array);
function getterAndSetter() {
if (Array.isArray(array))
var currentArray = result;
else
var currentArray = Object.keys(result);
console.log(result);
currentArray.forEach( function(prop){
if (Array.isArray(result))
var currentProp = result.lastIndexOf(prop);
else
var currentProp = prop;
Object.defineProperty(array, currentProp, {
configurable: true,
enumerable: true,
set: function(val) {
result[currentProp] = val;
callback.call(thisArg, result);
},
get: function() {
return result[currentProp];
}
});
//}
if (typeof result[currentProp] == 'object') {
arrayObserve(result[currentProp], callback, result);
}
});
}
getterAndSetter();
['pop','push','reverse','shift','unshift','splice','sort','map','filter'].forEach( function(method){
Object.defineProperty(array, method, {
configurable: false,
enumerable: false,
writable: true,
value: function () {
var noReturnMethods = ['push', 'pop', 'reverse', 'shift', 'unshift'];
if (noReturnMethods.indexOf(method) > -1) {
Array.prototype[method].apply(result, arguments);
} else {
var results = Array.prototype[method].apply(result, arguments);
result = results;
}
callback.call(thisArg, result);
getterAndSetter();
return result;
}
});
});
return result;
}
2 Answers 2
With this block:
if (typeof result[currentProp] == 'object') { arrayObserve(result[currentProp], callback, result); }
It appears that the code is handling nested arrays. Is that supposed to work with nested objects (A.K.A. associative arrays)? If not, then perhaps it would be better to use Array.isArray()
like the start of getterAndSetter()
.
The if
-else
logic within getterAndSetter()
doesn't put the statements in a block (i.e. in curly brackets - {
and }
), whereas the if
-else
logic within the value
function does put the statements in blocks. For the sake of readability (especially for others) it is best to be consistent with curly brackets style.
In the else
block of the value function, when a return method is called:
var results = Array.prototype[method].apply(result, arguments); result = results;
is results
used to avoid side-effects of the method (e.g. splice)? If not, then there isn't really a need to use results
- just re-assign result
with the return value from calling apply
on the method.
Perhaps you are already aware of this, but if I add a new element to an array using bracket notation after arrayObserve
has already been called with the array as the first parameter, then the callback is not called - should that be the case?
var arr = [3, 5, [1]];
arrayObserve(arr, console.log.bind(null,'callback - arguments:'));
arr[3] = 4;
Edit
One other thing to consider is using Array.from()
when creating result
, unless a shallow-copy won't work. That could simplify the following first two lines of arrayObserve()
:
var result = []; result.push.apply(result, array);
To this:
var result = Array.from(array);
If a deep-copy is desired, then the trusty JSON methods can be utilized:
var result = JSON.parse(JSON.stringify(array));
-
1\$\begingroup\$ I have accepted this answer because it helped me the most, thank you for the answer. \$\endgroup\$Jacques Marais– Jacques Marais2018年07月10日 16:55:38 +00:00Commented Jul 10, 2018 at 16:55
Some minor nit-picks about your code.
Object.defineProperty(array, method, { configurable: false, enumerable: false, writable: true, value: function () { // ...
Here, configurable
and enumerable
default to false
anyway. You needn't set them explicitly.
if (noReturnMethods.indexOf(method) > -1) { Array.prototype[method].apply(result, arguments); }
With ES6, you can write the if
condition more concisely as noReturnMethods.includes(method)
.
You could improve readability by storing ['pop','push','reverse','shift','unshift','splice','sort','map','filter']
in a variable. You could probably do
var noReturnMethods = ['push', 'pop', 'reverse', 'shift', 'unshift'];
var returnMethods = ['splice','sort','map','filter'];
var methods = [];
methods = methods.push.apply(noReturnMethods, returnMethods);
... to avoid duplication of the names of methods.
With ES6, you could do methods = [...noReturnMethods, ...returnMethods]
.
Assuming your environment supports ES6, you should probably make use of arrow functions and the let
and const
keywords.
-
2\$\begingroup\$ Thanks for the answer. My environment doesn't necessarily support ES6, but will keep those in mind. If it did, I would probably use Proxies to achieve this. \$\endgroup\$Jacques Marais– Jacques Marais2018年07月08日 19:56:09 +00:00Commented Jul 8, 2018 at 19:56