Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tiholic opened this issue Apr 10, 2018 · 8 comments
Open

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

tiholic opened this issue Apr 10, 2018 · 8 comments

Comments

@tiholic
Copy link

tiholic commented Apr 10, 2018

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
@etimberg
Copy link
Member

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

@simonbrunel
Copy link
Member

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?

@tiholic
Copy link
Author

tiholic commented Apr 12, 2018

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");

@simonbrunel
Copy link
Member

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.

@etimberg
Copy link
Member

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

@etimberg etimberg transferred this issue from chartjs/Chart.js Feb 21, 2021
@etimberg
Copy link
Member

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

@kurkle
Copy link
Member

kurkle commented Mar 9, 2021

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.

@etimberg
Copy link
Member

etimberg commented Mar 9, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants