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

Parsing issue with timezone since 1.10.2 #1318

Open
j0k3r opened this issue Jan 6, 2021 · 7 comments
Open

Parsing issue with timezone since 1.10.2 #1318

j0k3r opened this issue Jan 6, 2021 · 7 comments

Comments

@j0k3r
Copy link

j0k3r commented Jan 6, 2021

Describe the bug
Running v1.9.8:

console.log(dayjs('20170719T112552+0000'))

/*
    d {
      '$L': 'en',
      '$d': 2017-07-19T09:25:52.000Z,
      '$x': {},
      '$y': 2017,
      '$M': 6,
      '$D': 19,
      '$W': 3,
      '$H': 11,
      '$m': 25,
      '$s': 52,
      '$ms': 0
    }
*/

Running v1.10.2:

console.log(dayjs('20170719T112552+0000'))

/*
    d {
      '$L': 'en',
      '$d': Invalid Date,
      '$x': {},
      '$y': NaN,
      '$M': NaN,
      '$D': NaN,
      '$W': NaN,
      '$H': NaN,
      '$m': NaN,
      '$s': NaN,
      '$ms': NaN
    }
*/

Removing the timezone from the source string and the date is parsed again:

console.log(dayjs('20170719T112552'))

/*
    d {
      '$L': 'en',
      '$d': 2017-07-19T09:25:52.000Z,
      '$x': {},
      '$y': 2017,
      '$M': 6,
      '$D': 19,
      '$W': 3,
      '$H': 11,
      '$m': 25,
      '$s': 52,
      '$ms': 0
    }
*/

Looks like the regex (#1307) still need improvement I guess?

Expected behavior
Date properly parsed.

Information

  • Day.js Version: 1.10.2
  • OS: macOS
  • Browser: node 12
  • Time zone: [e.g. GMT-07:00 DST (Pacific Daylight Time)]
@chengB12
Copy link

chengB12 commented Jan 11, 2021

still open in 1.10.3

v1.9.7
dayjs.tz.setDefault('America/Los_Angeles')
console.log(dayjs.tz('2021-01-01T00:00:00-0800').format()) ==> 2021-01-01T00:00:00-08:00
v1.10.3
console.log(dayjs.tz('2021-01-01T00:00:00-0800').format()) ==> 2021-01-01T08:00:00-08:00

@iamkun
Copy link
Owner

iamkun commented Jan 12, 2021

will fix soon

@iamkun iamkun closed this as completed Jan 12, 2021
@iamkun iamkun reopened this Jan 12, 2021
@marklai1998
Copy link

marklai1998 commented Jan 23, 2021

any update on this issue? I'm still having this issue but only on android device
ios safari and desktop chrome are fine

android chorme 88.0.4324.93 has this issue but not 86.0.4240.198(browserstack)

I narrowed the issue down to the timezone function(remove the time zone and the parsing work again).

ps. downgrade to 1.9.8 doesn't fix the issue

@BePo65
Copy link
Contributor

BePo65 commented May 31, 2022

IMHO the easiest (the right?) solution is to use the customParseFormat plugin with a format of YYYYMMDD[T]HHmmssZ; this will parse the example. If the offset is only optional, then you could use an array with 2 formats: one with the Z format and one without.
Here the code (using dayjs 1.11.2):

import dayjs from 'dayjs'
import customParseFormat from 'dayjs/plugin/customParseFormat'
import timezone from 'dayjs/plugin/timezone'  // just to show the result with offset +00:00

dayjs.extend(customParseFormat)

const result = dayjs('20170719T112552+0000', 'YYYYMMDD[T]HHmmssZ')
console.log(result.isValid()) // true
console.log(result.tz('GMT').format('DD-MM-YYYY HH:mm:ss Z')) // 19-07-2017 11:25:52 +00:00

@j0k3r
Copy link
Author

j0k3r commented May 31, 2022

Thanks @BePo65 that workaround is working. But the bug remain :)

@BePo65
Copy link
Contributor

BePo65 commented May 31, 2022

But the bug remain :)

Of course You are right; in the documentation for parse > string it says

Parse the given string in ISO 8601 format and return a Day.js object instance.

And the offset is part of ISO 8601 (see e.g. wikipedia).

So we need to find someone who has the time to improve the regex for parsing the ISO_8601 date 😁

BePo65 added a commit to BePo65/dayjs that referenced this issue Jun 11, 2022
ISO 8601 date string can have an offset; moment parses this correctly.
Issue fixed; tests added / updated.
BePo65 added a commit to BePo65/dayjs that referenced this issue Jun 11, 2022
ISO 8601 date string can have an offset; moment parses this correctly.
Issue fixed; tests added / updated.
@olekristensen
Copy link

Hi there - is there a timeframe before this is merged?
Any help still needed?

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

6 participants