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

UTC date parsing issue #796

Closed
raajnadar opened this issue Aug 20, 2022 · 12 comments
Closed

UTC date parsing issue #796

raajnadar opened this issue Aug 20, 2022 · 12 comments
Labels
enhancement New feature or request

Comments

@raajnadar
Copy link

Problem

Hermes

new Date("Friday, 02-Aug-2019 07:14:03 UTC");
Date { NaN }

JSC

new Date("Friday, 02-Aug-2019 07:14:03 UTC");
2019-08-02T07:14:03.000Z

Chrome browser dev tools

new Date("Friday, 02-Aug-2019 07:14:03 UTC"));
Fri Aug 02 2019 12:44:03 GMT+0530 (India Standard Time)

Solution

Add support for this date format

Additional Context

It looks similar to this issue ticket #219

@raajnadar raajnadar added the enhancement New feature or request label Aug 20, 2022
@raajnadar
Copy link
Author

Can we get some attention to this issue? Or else I won’t be able to upgrade to Expo SDK 47 which uses RN 0.70

@jpporto
Copy link
Contributor

jpporto commented Oct 10, 2022

Hi,

Thanks for your feedback, and apologies for the delayed response. This is in the teams radar, but even if we were to add this support today, RN 0.70 has already shipped, and thus this support won't be available in that version.

John Paul

@raajnadar
Copy link
Author

I thought this can be sent in a patch release, once this is solved in Hermes, We can ask in the react native releases discussion, if not I am ok to wait for some time, as only one project is blocked due to this, and AFAIK RN v0.70 Hermes is the default so it cannot be turned off, currently I have turned off Hermes to use the v0.69

@jpporto
Copy link
Contributor

jpporto commented Oct 10, 2022

Hi,

Hermes being the default doesn't mean it is required. You can still disable it in your project (see this). Also, note that Hermes is only the default for new projects: Existing projects should be using their engine of choice. If not, reach out to the RN folks as this is probably a bug.

John Paul

@fbmal7
Copy link
Contributor

fbmal7 commented Oct 14, 2022

Hi @raajnadar we put together a fix which should be landing soon.

won’t be able to upgrade to Expo SDK 47

If you can, could you elaborate a bit on this point? It would be good to know how broadly this inconsistency between JSC and Hermes could currently be affecting users.

facebook-github-bot pushed a commit that referenced this issue Oct 14, 2022
Summary: This is addressing a gap between Hermes and V8 regarding date parsing, raised in #796 . There is whitespace allowed between M D Y. However, we should also support allowing a dash to be present as the first character in those gaps.

Reviewed By: jpporto

Differential Revision: D40368889

fbshipit-source-id: 5a9529300a221609ebb5c59dc44c4cf97a153700
@raajnadar
Copy link
Author

@fbmal7 I had a misunderstanding, I thought RN 0.70+ required Hermes to be enabled, after clearly reading the docs I found it is only default for the new apps.

Expo SDK 47 will add support for RN 0.70.3 in the Expo Go app, so I thought I won't be able to upgrade, but I was wrong

@fbmal7
Copy link
Contributor

fbmal7 commented Oct 17, 2022

Okay, but either way at one point this date behavior was causing some incompatibility, correct? I was just interested in learning more about how this was preventing you from using Hermes. But the fix has been shipped in a26472f :)

@fbmal7 fbmal7 closed this as completed Oct 17, 2022
@raajnadar
Copy link
Author

Actually, when I enabled Hermes it was leading to an app crash, so I turned off the Hermes and build the apk

And it crashed only on the apk or aab, not in the dev environment so it was tricky to debug the issue

@fbmal7
Copy link
Contributor

fbmal7 commented Oct 18, 2022

So then, how did you determine that Expo was depending on Hermes supporting this Date format?

@raajnadar
Copy link
Author

I strictly follow the Expo SDK version to upgrade my React Native version, in Expo SDK 47, RN 0.70 is planned, previously I had confusion about Hermes being the default which is clear now.

I use Expo Go for development

The app crashed when Hermes was enabled. I had to run the expo in prod mode to get the crash logs. It crashed because I was passing the date to the Date Fns library’s parseISO() method

@fbmal7
Copy link
Contributor

fbmal7 commented Oct 20, 2022

So you were providing the Date yourself? I thought you were saying Expo itself produced Dates in that format, thus making Expo and Hermes incompatible for some version of Expo.

@raajnadar
Copy link
Author

No, the date was returned by my PHP server, not sure why the backend dev was sending this format, we usually get ISO format which works very well with Hermes.

To be clear Expo is my dev tool, I am using Hermes in every application that is run using Expo Go, we found this date issue only in 1 project which was sending a different date format, with ISO there are no issues.

facebook-github-bot pushed a commit that referenced this issue Nov 18, 2022
Summary:
This is addressing a gap between Hermes and V8 regarding date parsing, raised in #796 . There is whitespace allowed between M D Y. However, we should also support allowing a dash to be present as the first character in those gaps.

Original Author: fbmal7
Original Git: f30f5210e007191972731bece675d18e56c64143

Original Reviewed By: jpporto

Original Revision: D40368889

Reviewed By: tmikov

Differential Revision: D41240843

fbshipit-source-id: 474e4216057b1246443c0e4d8d8efb9a78d9f903
avp added a commit to avp/hermes that referenced this issue Nov 22, 2022
Summary:
This is addressing a gap between Hermes and V8 regarding date parsing, raised in facebook#796 . There is whitespace allowed between M D Y. However, we should also support allowing a dash to be present as the first character in those gaps.

Original Author: fbmal7
Original Git: a26472f

Original Reviewed By: jpporto

Original Revision: D40368889

Reviewed By: tmikov

Differential Revision: D41240843

fbshipit-source-id: 474e4216057b1246443c0e4d8d8efb9a78d9f903
neildhar pushed a commit to neildhar/hermes that referenced this issue Nov 22, 2022
Summary:
This is addressing a gap between Hermes and V8 regarding date parsing, raised in facebook#796 . There is whitespace allowed between M D Y. However, we should also support allowing a dash to be present as the first character in those gaps.

Original Author: fbmal7
Original Git: f30f5210e007191972731bece675d18e56c64143

Original Reviewed By: jpporto

Original Revision: D40368889

Reviewed By: tmikov

Differential Revision: D41240843

fbshipit-source-id: 474e4216057b1246443c0e4d8d8efb9a78d9f903
neildhar pushed a commit to neildhar/hermes that referenced this issue Nov 23, 2022
Summary:
This is addressing a gap between Hermes and V8 regarding date parsing, raised in facebook#796 . There is whitespace allowed between M D Y. However, we should also support allowing a dash to be present as the first character in those gaps.

Original Author: fbmal7
Original Git: a26472f

Original Reviewed By: jpporto

Original Revision: D40368889

Reviewed By: tmikov

Differential Revision: D41240843

fbshipit-source-id: 474e4216057b1246443c0e4d8d8efb9a78d9f903
neildhar pushed a commit to neildhar/hermes that referenced this issue Nov 23, 2022
Summary:
This is addressing a gap between Hermes and V8 regarding date parsing, raised in facebook#796 . There is whitespace allowed between M D Y. However, we should also support allowing a dash to be present as the first character in those gaps.

Original Author: fbmal7
Original Git: f30f5210e007191972731bece675d18e56c64143

Original Reviewed By: jpporto

Original Revision: D40368889

Reviewed By: tmikov

Differential Revision: D41240843

fbshipit-source-id: 474e4216057b1246443c0e4d8d8efb9a78d9f903
neildhar pushed a commit to neildhar/hermes that referenced this issue Nov 23, 2022
Summary:
This is addressing a gap between Hermes and V8 regarding date parsing, raised in facebook#796 . There is whitespace allowed between M D Y. However, we should also support allowing a dash to be present as the first character in those gaps.

Original Author: fbmal7
Original Git: a26472f

Original Reviewed By: jpporto

Original Revision: D40368889

Reviewed By: tmikov

Differential Revision: D41240843

fbshipit-source-id: 474e4216057b1246443c0e4d8d8efb9a78d9f903
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants