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

Fixing timezone issues with the x-Axis dateTicker #912

Closed
wants to merge 1 commit into from

Conversation

saboya
Copy link

@saboya saboya commented Feb 20, 2018

I've noticed dygraphs was choking on my charts for some time now, but assumed it had something to do with my own data at the time. But I decided to investigate recently and discovered a bug in the getDateAxis function.

Based on the (automatically chosen) granularity, this function steps through the timestamps accordingly to generate the desired x-axis labels. It does so by creating an array with fields that correspond to year, month, day, etc. It uses that to instance a new Date object for each step.

However, the start_date that is used for comparison is instaced using new Date(start_time). If you look at the documentation for the Date constructor closely, these do not produce the same date:

Note: Where Date is called as a constructor with more than one argument, the specifed arguments represent local time. If UTC is desired, use new Date(Date.UTC(...)) with the same arguments.

So, this function was called with any timezone different than UTC, it resulted the loop being called a lot more than it needed, based on the granularity.step and timezone difference. In my case.

And the reason nobody noticed this until now is because in the last Dygraphs released, millisecond granularity was introduced with #893. Up until then, those extra calls only accounted for a few thousand more based on the number of seconds between UTC and localtime. However, with millisecond accuracy, those calls quickly account for millions more, which results in pretty significant lag.

And when Dygraphs is called with an empty file, start_date is new Date(0) and start_date.getTime() === 0, but tick_date is basically new Date(1969, 11, 31, 21, 0, 0, 0) and tick_date.getTime() === -3600000 (for my timezone).

This PR fixes the issue by using UTC accessors to instantiate tick_date, which makes it equal to how start_date is instanced.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.195% when pulling 212c452 on saboya:wrong-date-ticks into 6611837 on danvk:master.

saboya added a commit to saboya/react-dygraphs that referenced this pull request Feb 21, 2018
pawelzwronek added a commit to pawelzwronek/dygraphs that referenced this pull request Apr 14, 2018
Fixing timezone issues with the x-Axis dateTicker danvk#912
@saboya saboya force-pushed the wrong-date-ticks branch 2 times, most recently from 2749bd7 to cb35acf Compare May 8, 2018 00:40
pawelzwronek added a commit to pawelzwronek/dygraphs that referenced this pull request May 8, 2018
pawelzwronek added a commit to pawelzwronek/dygraphs that referenced this pull request May 8, 2018
Revert "Fixing timezone issues with the x-Axis dateTicker danvk#912"
@saboya saboya closed this Sep 14, 2018
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

Successfully merging this pull request may close these issues.

None yet

2 participants