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

customParseFormat strict mode bug #929

Open
sheffieldnikki opened this issue Jun 11, 2020 · 15 comments · May be fixed by #1467
Open

customParseFormat strict mode bug #929

sheffieldnikki opened this issue Jun 11, 2020 · 15 comments · May be fixed by #1467
Labels
☢️Bug Something isn't working

Comments

@sheffieldnikki
Copy link

sheffieldnikki commented Jun 11, 2020

Describe the bug
Using the format string "YYYY-MM-DD HH:mm:ss ZZ", parsing certain strings returns Invalid Date when it should return a date:

var format = "YYYY-MM-DD HH:mm:ss ZZ";
var ok = dayjs("2018-03-01 00:01:00 +0000", format, true);
var bad = dayjs("2018-04-01 00:01:00 +0000", format, true);

where bad is:

{
  "$D": 1
  "$H": 1
  "$L": "en"
  "$M": 3
  "$W": 0
  "$d": Invalid Date
  "$m": 1
  "$ms": 0
  "$s": 0
  "$u": undefined
  "$y": 2018
}

The parsing is successful if strict mode is not used. The problem seems to be in the customParseFormat.js line:

      if (isStrict && date !== this.format(format)) {

where the date that the plugin has parsed is not matching the default output from dayjs.format(). In the above case:

"2018-04-01 00:01:00 +0000" !== "2018-04-01 01:01:00 +0100"

but the same bug will effect parsing any date where the timezone doesn't match whatever local timezone/dst dayjs uses. eg. "2018-03-01 00:01:00 +0600", "2018-04-01 00:01:00 -0200", etc.

Expected behavior
bad should be a valid date object.

Information

  • Day.js Version: v1.8.28
  • OS: Windows 7
  • Browser: Firefox 77, Chrome 83
  • Time zone: UTC+00:00 DST (British Summer Time)
@z0al
Copy link

z0al commented Jul 9, 2020

We have the same problem. I created this codesandbox example to reproduce the issue

@iamkun iamkun added the ☢️Bug Something isn't working label Jul 10, 2020
@waspar
Copy link

waspar commented Aug 7, 2020

R.I.P 👌

@DDoerner
Copy link

I've looked into the code and so far I see only two options for how this bug could be fixed:

  1. The timezone information that is extracted by makeParser() could be passed back to the main method in addition to the parsed Date object. It can then be used to format the final result with a custom timezone before the string comparison is made. This however would mean to either require the timezone plugin for the customParseFormat plugin to work or to recreate this functionality in customParseFormat, which might be overkill
  2. Rework how strict parsing in customParseFormat works by giving each segment expression information on how to enforce a strict format

@iamkun
Copy link
Owner

iamkun commented Sep 17, 2020

Bug confirmed. Strict mode does not work good with parsing timezone (ZZ)

I'm looking for a way to solve it.

@AllainPL
Copy link

AllainPL commented Nov 2, 2020

I am trying to migrate some code from moment to dayjs, and this strict check is a big problem for me :(, any idea how to solve it?

@ivansieder
Copy link

@iamkun is there any known workaround available right now? Or do you eventually have an estimated time until when this can be looked into? Thanks in advance

@ivansieder
Copy link

ivansieder commented Apr 21, 2021

@iamkun I've created a pull request which tries to fix this issue (#1467). The following has been done to fix it: The parseFormattedInput function returns a UTC object for dates with timezones. When using dayjs().format() it usually formats the date with the current timezone. This fix manually removes the current timezone's offset and manually adds the originally parsed offset. Then the timezone part (+/-xx:xx or +-xxxx) will be removed from both strings and the timestamp without the timezone can be compared correctly.

@BreakBB
Copy link

BreakBB commented Sep 10, 2021

Any updates on this? Looks like the PR #1467 has conflicts which need to be resolved.

Kinda strange that dayjs can't handle YYYY-MM-DDTHH:mm:ss.SSSZ as format in strict mode, which is the default format from what I read in the docs.

@ivansieder
Copy link

@BreakBB thank you for the hint. I've merged it now with dev, updated the code and the tests are passing. Now waiting for further feedback from @iamkun 👍

@ivansieder
Copy link

@iamkun is there anything else we can do to help get this pull request merged?

@Lonli-Lokli
Copy link

@iamkun Just a friendly reminder :)

@IonVillarreal
Copy link

any news about the problem?

@kangwooc
Copy link

Is there any progress for this?

@medev21
Copy link

medev21 commented Oct 17, 2022

any news about the timezone parsing issue? It will be great to use it properly as I'm migrating from luxon to dayjs

@wolak041
Copy link

any news? 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☢️Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.