Use with typed arrays

[bairathirahul]

Hi developers,

I am facing an issue after upgrading from v2.6.0 to v2.7.0. The charts are not working with typed arrays. Here is a JSFiddle example

https://jsfiddle.net/fwb415nw/1/

[jcopperfield]

There are 2 problems

1. The example contains an error.
   The line: new Float32Array(12, 19, 3, 5, 2, 3)
   should be: new Float32Array([12, 19, 3, 5, 2, 3])
2. The helper.each function doesn't recognize typed array.

    Array.isArray(new Float32Array())
    false

    Array.isArray(new Array())
    true

A rudimentary fix could be adding a check for typed arrays.

isTypedArray: function(value) {
    var type = Object.prototype.toString.call(value);
    return type.substr(0, 7) === '[object' && type.substr(-6) === 'Array]';
},

each: function(loopable, fn, thisArg, reverse) {
    var i, len, keys;
    if (helpers.isArray(loopable) || helpers.isTypedArray(loopable)) {

Or maybe it would be better to adopt the JQuery implementation isArrayLike.

[jcopperfield]

@etimberg any thoughts on a preferred implementation for type arrays?
jQuery isArrayLike or extra check for isTypedArray?

[etimberg]

I have no preference for with solution. Both work for me

[benmccann]

I like your method for checking for typed array better than jquery's, but think it makes sense to encapsulate inside isArray instead of having to always call both isArray and isTypedArray

	isArray: function(value) {
		// typed array
		var type = Object.prototype.toString.call(value);
		return type.substr(0, 7) === '[object' && type.substr(-6) === 'Array]';
		if (Array.isArray) {
			return Array.isArray(value);
		}
		if (Object.prototype.toString.call(value) === '[object Array]') {
			return true;
		}
	}

[jcopperfield]

@benmccann I'm afraid your code won't work, as it would return Array.isArray when it is defined, and only execute the on browsers where it isn't natively present. Then again I don't think there are many browsers that won't support it and would still be successfully create a ChartJS chart.

Browser compatibility Array.isArray

Feature	Chrome	Edge	Firefox	Internet Explorer	Opera	Safari
Basic support	5	Yes	4	9	10.5	5

I still think it would be better to implement a separate function

/**
* Returns true if `value` is a typed array, else returns false.
* @param {*} value - The value to test.
* @returns {Boolean}
* @function
*/
isTypedArray: function(value) {
  var type = Object.prototype.toString.call(value);
  return type.substr(1, 6) === 'object' && type.substr(-6, 5) === 'Array' && value.byteLength !== undefined;
}

[simonbrunel]

I would start with isArrayLike since I think we will always test both (helpers.isArray(iterable) || helpers.isTypedArray(iterable)), which is better for readability and minification. I would add isTypedArray only if at one point we need to test typed array only.

[benmccann]

@jcopperfield yeah, I was a bit hasty. I've updated the suggestion to account for the issue you mentioned

[simonbrunel]

@benmccann it still doesn't work because Array.isArray will certainly be defined and thus will always return Array.isArray(value);. Also changing the behavior of helpers.isArray could be a breaking change if someone used it in a way not compatible with typed array (not sure how though).

Safer to define a new method.

[benmccann]

you're right. I've updated again

We have a couple dozen places where we call isArray. We may need to update all of them to call the new function and leave the old isArray unused

I'm not in love with the name isArrayLike since these actually are arrays and not array-like objects. I think isArray is a much better name. It's just that the previous implementation was buggy

[simonbrunel]

I'm not fan either of the isArrayLike name but I think it's better to have one method that do both checks instead of having to write both each time in the code (similar to if (isNullOrUndef(value)) instead of if (value === null || value === undefined)). It also faster to check both at the same time.

If you can guarantee that changing the behavior of helpers.isArray will not introduce breaking changes, then I would do the same and keep using isArray, however I'm not sure it's the case. I'm not familiar with typed array so don't know what would be side effects of returning true for isArray() on a typed array.