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

Format date seems to use incorrect offset #6

Closed
dynajoe opened this issue Apr 12, 2016 · 14 comments
Closed

Format date seems to use incorrect offset #6

dynajoe opened this issue Apr 12, 2016 · 14 comments
Labels

Comments

@dynajoe
Copy link
Contributor

dynajoe commented Apr 12, 2016

Here's a test that fails for me. I'm currently in CDT.

convertingDates : Test
convertingDates =
  suite
    "Converting a date to ISO String"
    [ test
        "output is exactly the same as iso input"
        (assertEqual
          (Ok "2016-03-22T17:30:00.000Z")
          (Date.fromString "2016-03-22T17:30:00.000Z" `Result.andThen` (Ok << dateToISO))
        )
    ]

The output is 2016-03-22T18:30:00.000Z instead of 2016-03-22T17:30:00.000Z.

@rluiten
Copy link
Owner

rluiten commented Apr 12, 2016

Hi, that's an interesting failure.

In the mean time what is the timezone of your javascript vm ?
I don't know if it is related but it might be.

I would like the result of the following two commands please.
new Date()
and
(new Date()).getTimezoneOffset()

I hope to look at this further tonight after work.

@dynajoe
Copy link
Contributor Author

dynajoe commented Apr 12, 2016

new Date()
Tue Apr 12 2016 17:21:58 GMT-0500 (CDT)
(new Date()).getTimezoneOffset()
300

@rluiten
Copy link
Owner

rluiten commented Apr 13, 2016

I added a copy of your test into my tests.

I created dateToISO as follows.

import Date.Extra.Format as Format

dateToISO = Format.utcIsoString

Its not failing for me...
Scratching my head at the moment.

What is your dateToISO function could it be the problem ?

I added your test into the test suite, and its part of version 4.0.0 which was created to shift names spaces.

@dynajoe
Copy link
Contributor Author

dynajoe commented Apr 13, 2016

oops!

module Util.Extra (..) where

import String
import Date exposing (Date)
import Date.Format as Format exposing (format, formatUtc, isoMsecOffsetFormat)
import Date.Config.Config_en_au exposing (config)


dateToISO : Date -> String
dateToISO date =
  formatUtc config isoMsecOffsetFormat date

@rluiten
Copy link
Owner

rluiten commented Apr 13, 2016

Ok, yours works though it does not output this format "2016-03-22T17:30:00.000Z".

It outputs "2016-03-22T17:30:00.000+0000", however I do not see the 18 hour value.

Can you try dateToISO = Format.utcIsoString as i mentioned above.

I am still curious where that 18 is coming from.

@dynajoe
Copy link
Contributor Author

dynajoe commented Apr 13, 2016

Ahh yes, when constructing the test I didn't even look at the way the offset was formatted. Either way, the 18 is indeed odd. The test still fails:

     test
        "output is exactly the same as iso input"
        (assertEqual
          (Ok "2016-03-22T17:30:00.000+0000")
          (Date.fromString "2016-03-22T17:30:00.000Z" `Result.andThen` (Ok << dateToISO))
        )

@dynajoe
Copy link
Contributor Author

dynajoe commented Apr 13, 2016

This also fails:

, test
        "output is exactly the same as iso input"
        (assertEqual
          (Ok "2016-03-22T17:30:00.000Z")
          (Date.fromString "2016-03-22T17:30:00.000Z" `Result.andThen` (Ok << Format.utcIsoString))
        )

@rluiten
Copy link
Owner

rluiten commented Apr 13, 2016

I would focus on the hour discrepancy first if i saw it to, i don't think id notice the time zone difference at end of string either.

I am trying to think of something else (anything else) that might cause that hour difference.

Time to grasp at straws... (asking for any thing that might vaguely matter)

What javascript engine is running the code is it a browser which ? is it node version ?
What platform are you on windows/osx/linux ?
What do these return in your javascript environment ?
(new Date("2016-03-22T17:30:00.000Z"))
(new Date("2016-03-22T17:30:00.000Z")).getHours()

Is it possible to share a project or maybe an extracted piece of your project in isolation to let me try and get a sclose to your soruce base as possible ?

@dynajoe
Copy link
Contributor Author

dynajoe commented Apr 13, 2016

I can screen share

@rluiten
Copy link
Owner

rluiten commented Apr 13, 2016

I am at work at the moment, cant do anything till tonight at least 9 hours away.

rluiten added a commit that referenced this issue Apr 15, 2016
…hmark

test(convertingDates): adjusted still no closer to resolving issue #6
@rluiten
Copy link
Owner

rluiten commented Apr 17, 2016

Hi, I am still intereted in the result of the following in your javascript environment.
(new Date("2016-03-22T17:30:00.000Z"))
(new Date("2016-03-22T17:30:00.000Z")).getHours()

@tesk9
Copy link

tesk9 commented Apr 22, 2016

Hey! I think the problem is that Date.Extra.Create's getTimezoneOffset doesn't incorporate Daylight Savings Time.

The JavaScript spec for getTimezoneOffset explicitly covers daylight savings--it's part of how local time is calculated.

Running in Node, PST obvs

> (new Date(2016, 1)).getTimezoneOffset()
480
> (new Date(2016, 4)).getTimezoneOffset()
420

@rluiten
Copy link
Owner

rluiten commented Apr 22, 2016

Now that is a interesting observation, not one I had considered till now.
I will have a look at it on the weekend,

I have memories of trying to detect the current (as in relation to the date and time we are working with) timezone offset to compensate, but I could have made a mistake or just did something wrong. I get the offset by derivation so it might have a bug or it might not be easy to fix. I have no way to just ask for it from the underlying javascript vm at the moment.

@rluiten
Copy link
Owner

rluiten commented Apr 26, 2016

I have just released 5.0.0 quite a lot of work around improving handling of timezone offsets particularly negative offsets and transitions to and from daylight saving. As well as better handling of daylight saving in date math.

I believe I fixed two areas that may be related to the behaviour you observed @joeandaverde so its probably worth checking.

Thankyou @tesk9 for suggesting daylight saving offsets were a candidate area for examination it was a deep rabbit hole :)

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

No branches or pull requests

3 participants