chartjs/chartjs-adapter-moment

[BUG] Deprecation Warning - from moment.js when using time axes

tiholic opened this issue ยท 8 comments

Expected Behavior

Current Behavior

Deprecation warning: value provided is not in a recognized RFC2822 or ISO format. moment construction falls back to js Date(), which is not reliable across all browsers and versions. Non RFC2822/ISO date formats are discouraged and will be removed in an upcoming major release. Please refer to http://momentjs.com/guides/#/warnings/js-date/ for more info.
Arguments: 
[0] _isAMomentObject: true, _isUTC: false, _useUTC: false, _l: undefined, _i: 2018.01.01, _f: undefined, _strict: undefined, _locale: [object Object]
Error
    at Function.createFromInputFallback (http://127.0.0.1:8000/static/documerge/node_modules/moment/moment.js?v=1523389320944:320:98)
    at configFromString (http://127.0.0.1:8000/static/documerge/node_modules/moment/moment.js?v=1523389320944:2368:15)
    at configFromInput (http://127.0.0.1:8000/static/documerge/node_modules/moment/moment.js?v=1523389320944:2594:13)
    at prepareConfig (http://127.0.0.1:8000/static/documerge/node_modules/moment/moment.js?v=1523389320944:2577:13)
    at createFromConfig (http://127.0.0.1:8000/static/documerge/node_modules/moment/moment.js?v=1523389320944:2544:44)
    at createLocalOrUTC (http://127.0.0.1:8000/static/documerge/node_modules/moment/moment.js?v=1523389320944:2631:16)
    at createLocal (http://127.0.0.1:8000/static/documerge/node_modules/moment/moment.js?v=1523389320944:2635:16)
    at hooks (http://127.0.0.1:8000/static/documerge/node_modules/moment/moment.js?v=1523389320944:12:29)
    at momentify (http://127.0.0.1:8000/static/documerge/node_modules/chart.js/dist/Chart.js?v=1523389320944:13795:11)
    at parse (http://127.0.0.1:8000/static/documerge/node_modules/chart.js/dist/Chart.js?v=1523389320944:13817:14)
warn @ moment.js?v=1523389320944:293
(anonymous) @ moment.js?v=1523389320944:320
configFromString @ moment.js?v=1523389320944:2368
configFromInput @ moment.js?v=1523389320944:2594
prepareConfig @ moment.js?v=1523389320944:2577
createFromConfig @ moment.js?v=1523389320944:2544
createLocalOrUTC @ moment.js?v=1523389320944:2631
createLocal @ moment.js?v=1523389320944:2635
hooks @ moment.js?v=1523389320944:12
momentify @ Chart.js?v=1523389320944:13795
parse @ Chart.js?v=1523389320944:13817
determineDataLimits @ Chart.js?v=1523389320944:14149
update @ Chart.js?v=1523389320944:7230
update @ Chart.js?v=1523389320944:14109
getMinimumBoxSize @ Chart.js?v=1523389320944:6416
each @ Chart.js?v=1523389320944:9899
update @ Chart.js?v=1523389320944:6430
updateLayout @ Chart.js?v=1523389320944:4259
update @ Chart.js?v=1523389320944:4214
construct @ Chart.js?v=1523389320944:3944
Chart @ Chart.js?v=1523389320944:6215

image

Possible Solution

Moment.js might deprecate the functionality. Unless provided date format along with date. like moment(value, format) in #13795 form the above screenshot

Steps to Reproduce (for bugs)

  1. Configure to time axis
{
type: "line",
data: {
            // labels: ["Used", "Remaining"],
            datasets: [{
                label: 'Test Merges',
                xAxisID: "time",
                fill: true,
                lineTension: 0,
                data: [
                    {x: '01-01-2018', y:10},
                    {x: '02-01-2018', y:5},
                    {x: '02-05-2018', y:15}
                ]
            }]
        },
config: {
            scales: {
               xAxes: [{
                    id: 'time',
                    type: 'time',
                    time: {
                        tooltipFormat: "DD MMMM, YYYY"
                    }
                }]
            }
        };
    }
}

Environment

  • Chart.js version: "^2.7.2",
  • Chrome 65.0.3325.181

@simonbrunel @benmccann thoughts on how to handle this? One solution might be to try and if moment errors out, throw something else.

I think a try ... catch here would be an important performance hit since it's called very often (I already removed exception handling for color parsing). What moment recommend as alternative for this deprecation?

As mentioned in the Issue description, format needs to be passed long with value. As they are not supposed to assume a date format for the value being passed, as per the RFC2822 or ISO format.

///So, 
moment(value); //moment("04-12-2018");
//must be changed to
moment(value, format); //moment("04-12-2018", "MM-DD-YYYY");

That's line 13791 and if Chart.js users didn't provide a format, then we still need to fallback to moment(value) so we can't just drop support for moment(value) until next major release.

maybe we can just assume ISO8601 .... it makes the deprecation warning go away but it doesn't necessarily solve the problem

I moved this from the main repo, now that the moment adapter code is split out and optional

I think it would be best to document this and suggest providing the format or parser option to the time scale.

Or we could add a lot of dead weight by doing some checks on first data point etc, with try/catch and specific errors.

Specifying the format or parser option is the best way to go here IMO