[ Line Chart ] Line chart does not handle datapoints created with the Number constructor

[MatthieuRivaud]

Data points created with the Number constructor (ex: new Number(...)) : typeof (new Number()) yields "object", thus Chart.controllers.line.updateElement and Chart.Scale.getRightValue treat those values as { x: ..., y: ... } objects and skip them (because x and y are undefined).

If I hack those functions to consider ... instanceof Number the same as typeof ... === 'number', the data points are correctly displayed.

Should we try to handle this kind of (edge?) case ? Other chart types do seem to accept Number objects (tested a radar chart).

[etimberg]

I think we should gracefully handle this case. I'm not sure why anyone would create a number using new Number but since it's valid JS we should at least try and support it if it is easy.

One thing we may also want to test is using Typed arrays for the data to the chart. I don't think we do any specific to check that the data array is an array (isArray will return false for a typed array) but we should add a test to prevent any regressions.

[MatthieuRivaud]

Re: new Number() : I encountered the case with code that handled number sanitation on string representations of given numbers, and passed that to the Number constructor. (Javascript code generated from XSL). I replaced the call by just Number(...) (which gives a typeof = 'number' instead of an object).

Re: Typed arrays : I don't know if we have to specifically handle TypedArray data objects (as in transforming them into real arrays), but it could be nice to be able to take any typed array view as data. Most of the Array methods are also available on typed array views (or can be called with Array.prototype.XXX.apply), so it should be possible to work on typed arrays (as long as we don't try to add or delete items). With well-chosen helpers, it may not be too much of a pain.

Even though, it might be better to just ask for an Array object, or forcefully convert to Array from the start to avoid strange regressions. I don't really see a use case with so much data that copying into another array would be a no go.

[simonbrunel]

Should be fixed by #5752.