Navigation Menu

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

getTimeZoneOffset callback passes inconsistent value type #13042

Closed
mediafreakch opened this issue Feb 28, 2020 · 5 comments · Fixed by #13052
Closed

getTimeZoneOffset callback passes inconsistent value type #13042

mediafreakch opened this issue Feb 28, 2020 · 5 comments · Fixed by #13052
Assignees
Labels
Product: TypeScript Issues & enhancements related to TS/TypeScript

Comments

@mediafreakch
Copy link

Expected behaviour

time.getTimezoneOffset should take a parameter timestamp that is milliseconds since January 1 1970 according to docs

Actual behaviour

Effectively timestamp is sometimes the number of milliseconds, but also a Date() object. I think it depends on what kind of "thing" is being transformed (label, axis, value, ...).

Live demo with steps to reproduce

https://jsfiddle.net/tw6gujfv/ (observe in console)

Product version

Highcharts JS v8.0.0 (2019-12-10)

Affected browser(s)

all

@mediafreakch
Copy link
Author

On this occasion, maybe also the return type of the function should be documented correctly: It should be number and not string.

@pawelfus
Copy link
Contributor

pawelfus commented Mar 2, 2020

Thanks for reporting @mediafreakch

FYI @TorsteinHonsi, @bre1470

@pawelfus pawelfus added the Product: TypeScript Issues & enhancements related to TS/TypeScript label Mar 2, 2020
@bre1470 bre1470 self-assigned this Mar 2, 2020
@bre1470
Copy link
Contributor

bre1470 commented Mar 2, 2020

Looks like the documentation is incorrect and we will investigate, why it sometimes provide timestamp instead of Date instance. As timezone information is only available on the Date instance and timestamp is always GMT, it makes more sense to have always a Date instance for this callback.

@mediafreakch
Copy link
Author

mediafreakch commented Mar 2, 2020

@bre1470 We use this callback to get around the limitation that timezones are only supported with Moment.js. We use date-fns-tz (those Date instances have no timezone information, but it works regardless).

We use the following callback to transform to a highcharts compatible format:

export function getTimezoneOffset(timestamp: number | Date): number {
  let serverUTCTime: number

  if (typeof timestamp === 'object') {
    serverUTCTime = timestamp.getTime()
  } else {
    serverUTCTime = timestamp
  }

  const stationUnixTime = utcToZonedTime(serverUTCTime, this.timeZone).getTime()
  const utcUnixTime = utcToZonedTime(serverUTCTime, 'UTC').getTime()

  // for highcharts, we need the reverse-offset: how  much does UTC offset from stationTime?
  // For Date.UTC(2020, 0, 1) Europe/Zurich it is -01:00
  const offsetInMinutes = Math.floor((utcUnixTime - stationUnixTime) / 1000 / 60)
  return offsetInMinutes
}

An example: https://codesandbox.io/s/morning-thunder-9yw3n

@bre1470
Copy link
Contributor

bre1470 commented Mar 2, 2020

@mediafreakch
Thank you for the additional example of implementation! So in your use case a number type would be clearly more beneficial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product: TypeScript Issues & enhancements related to TS/TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants