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.

Sign up for GitHub

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

Jump to bottom

dayjs.utc() dropped timezone offset support #1239

Closed
jeiea opened this issue Nov 25, 2020 · 7 comments · Fixed by #1295
Closed

dayjs.utc() dropped timezone offset support #1239

jeiea opened this issue Nov 25, 2020 · 7 comments · Fixed by #1295
Labels
☢️Bug Something isn't working

Comments

@jeiea
Copy link

jeiea commented Nov 25, 2020

Describe the bug
dayjs.utc('2020-12-01T09:00:00+0900').toDate().getTime() === new Date('2020-12-01T09:00:00+0900').getTime()

Expected behavior
The above evaluates to true. It's the behaviour of dayjs 1.8.33.

Actual behaviour

> dayjs.utc('2020-12-01T09:00:00+0900').toDate()
2020-12-01T09:00:00.090Z
> new Date('2020-12-01T00:00:00+0900')
2020-11-30T15:00:00.000Z

Information

  • Day.js Version: 1.9.6
  • OS: macOS
  • Browser:
  • Time zone: GMT+0900 (Korean Standard Time)
@iamkun
Copy link
Owner

iamkun commented Nov 26, 2020 

dayjs.utc('2020-12-01T09:00:00+0900').toDate().getTime() 
   === new Date('2020-12-01T09:00:00+0900').getTime()
   === dayjs('2020-12-01T09:00:00+0900').toDate().getTime()

The above three should have the same result. Cause it represents the same time '2020-12-01T09:00:00+0900'

@jeiea
Copy link
Author

jeiea commented Nov 26, 2020

There was my mistake in the code snippet. I'll explain again.

What I said is your invariant is not established. codesandbox
Interestingly, all of them have different result.

// Windows 10 + Chrome 86.0.4240.198
dayjs.utc('2020-12-01T09:00:00+0900').toDate().getTime(): 1606813200090
dayjs    ('2020-12-01T09:00:00+0900').toDate().getTime(): 1606780800090
 new Date('2020-12-01T09:00:00+0900')         .getTime(): 1606780800000

@iamkun iamkun added the ☢️Bug Something isn't working label Nov 30, 2020
@iamkun
Copy link
Owner

iamkun commented Nov 30, 2020

This might be a bug in UTC plugin.

@t-mangoe
Copy link

Hi. I researched about this issue.
In the case of '2020-12-01T09:00:00+0900', +0900 is interpreted as a value of miliseconds.
I checked regex constant to parse date string, and it may be wrong that dot character is not escaped.

dayjs/src/constant.js

Lines 28 to 29 in 17221ec

// regex
export const REGEX_PARSE = /^(\d{4})[-/]?(\d{1,2})?[-/]?(\d{0,2})[^0-9]*(\d{1,2})?:?(\d{1,2})?:?(\d{1,2})?.?(\d+)?$/

@bcurtin144
Copy link
Contributor

@t-mangoe is correct. To be sure the last digit token in REGEX_PARSE only matches for milliseconds, the period must be escaped. Otherwise, it acts as a wildcard.

To offer some more examples, all of these are the same date/time using different ISO 8601 time zone formats:
dayjs("2020-12-31T18:00:00.000-0500")
dayjs("2020-12-31T18:00:00-05:00")
dayjs("2020-12-31T18:00:00-0500")
The first two parse correctly because REGEX_PARSE doesn't match. But the last example does match REGEX_PARSE, with the - character matching the wildcard. So the date is parsed in local time and the first three characters of 0500 become 50ms.

I'm sorry I don't know how to contribute the fix, but the code should read:
export const REGEX_PARSE = /^(\d{4})[-/]?(\d{1,2})?[-/]?(\d{0,2})[^0-9]*(\d{1,2})?:?(\d{1,2})?:?(\d{1,2})?\.?(\d+)?$/

Thank you for the library. It's very useful!

bcurtin144 added a commit to bcurtin144/dayjs that referenced this issue Dec 31, 2020
@bcurtin144
fix: escape last period to match only milliseconds (iamkun#1239)
17724a5
@bcurtin144
Copy link
Contributor

I added a pull request with the change: #1295

bcurtin144 added a commit to bcurtin144/dayjs that referenced this issue Jan 2, 2021
@bcurtin144
add tests for REGEX_PARSE
cfcf088 
includes two tests that would have matched before fix iamkun#1239 but no longer match
iamkun pushed a commit that referenced this issue Jan 3, 2021
@bcurtin144
fix: escape last period to match only milliseconds (#1239) (#1295)
90bb665
@iamkun
Copy link
Owner

iamkun commented Jan 3, 2021

fixed in #1295

@iamkun iamkun closed this as completed Jan 3, 2021
@iamkun iamkun mentioned this issue Jan 3, 2021
Merged
iamkun pushed a commit that referenced this issue Jan 3, 2021
@bcurtin144 @iamkun
fix: escape last period to match only milliseconds (#1239) (#1295)
64037e6
iamkun pushed a commit that referenced this issue Jan 3, 2021
@semantic-release-bot
chore(release): 1.10.0 [skip ci]
b60466e 
# [1.10.0](v1.9.8...v1.10.0) (2021-01-03)

### Bug Fixes

* add ordinal to localeData plugin ([#1266](#1266)) ([fd229fa](fd229fa))
* add preParsePostFormat plugin & update Arabic [ar] locale ([#1255](#1255)) ([f2e4790](f2e4790))
* add type support for plural forms of units ([#1289](#1289)) ([de49bb1](de49bb1))
* escape last period to match only milliseconds ([#1239](#1239)) ([#1295](#1295)) ([64037e6](64037e6))

### Features

* add ES6 Module Support, package.json module point to "esm/index.js" ([#1298](#1298)) ([f63375d](f63375d)), closes [#598](#598) [#313](#313)
This was referenced Mar 16, 2021
Merged
Closed
andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this issue May 10, 2022
@andrewhood125ruhuc
chore(release): 1.10.0 [skip ci]
9699493 
# [1.10.0](iamkun/dayjs@v1.9.8...v1.10.0) (2021-01-03)

### Bug Fixes

* add ordinal to localeData plugin ([#1266](iamkun/dayjs#1266)) ([fd229fa](iamkun/dayjs@fd229fa))
* add preParsePostFormat plugin & update Arabic [ar] locale ([#1255](iamkun/dayjs#1255)) ([f2e4790](iamkun/dayjs@f2e4790))
* add type support for plural forms of units ([#1289](iamkun/dayjs#1289)) ([de49bb1](iamkun/dayjs@de49bb1))
* escape last period to match only milliseconds ([#1239](iamkun/dayjs#1239)) ([#1295](iamkun/dayjs#1295)) ([64037e6](iamkun/dayjs@64037e6))

### Features

* add ES6 Module Support, package.json module point to "esm/index.js" ([#1298](iamkun/dayjs#1298)) ([f63375d](iamkun/dayjs@f63375d)), closes [#598](iamkun/dayjs#598) [#313](iamkun/dayjs#313)
andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this issue May 10, 2022
@andrewhood125ruhuc
chore(release): 1.10.0 [skip ci]
81a01fa 
# [1.10.0](iamkun/dayjs@v1.9.8...v1.10.0) (2021-01-03)

### Bug Fixes

* add ordinal to localeData plugin ([#1266](iamkun/dayjs#1266)) ([fd229fa](iamkun/dayjs@fd229fa))
* add preParsePostFormat plugin & update Arabic [ar] locale ([#1255](iamkun/dayjs#1255)) ([f2e4790](iamkun/dayjs@f2e4790))
* add type support for plural forms of units ([#1289](iamkun/dayjs#1289)) ([de49bb1](iamkun/dayjs@de49bb1))
* escape last period to match only milliseconds ([#1239](iamkun/dayjs#1239)) ([#1295](iamkun/dayjs#1295)) ([64037e6](iamkun/dayjs@64037e6))

### Features

* add ES6 Module Support, package.json module point to "esm/index.js" ([#1298](iamkun/dayjs#1298)) ([f63375d](iamkun/dayjs@f63375d)), closes [#598](iamkun/dayjs#598) [#313](iamkun/dayjs#313)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
☢️Bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: escape last period to match only milliseconds (#1239) bcurtin144/dayjs
4 participants
@jeiea @iamkun @t-mangoe @bcurtin144