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

Allow parseJSON to accept strings without a trailing 'Z' symbold and with up to 6 digits in the milliseconds field (#1496) #1499

Merged
merged 1 commit into from
Nov 7, 2019

Conversation

dmytro-gokun
Copy link
Contributor

Here's one thing to think about though.. JS Date does not allow to have fractional milliseconds. So, the way i've implemented it is that extra digits are simply truncated. E.g.:

2019-12-31T23:59:59.999999Z becomes 2019-12-31T23:59:59.999Z

The question arises: do we need implemented a proper rounding here instead of truncation? E.g., with proper rounding:

2019-12-31T23:59:59.999999Z becomes 2020-01-01T00:00:00Z

In my opinion, this is not really needed and we should keep truncating it for the sake of simplicity. But I'd like to hear different opinions.

src/parseJSON/index.js Outdated Show resolved Hide resolved
src/parseJSON/index.js Outdated Show resolved Hide resolved
@dkozickis
Copy link
Contributor

dkozickis commented Oct 28, 2019

Thank you @dmytro-gokun.

I think the big bad Oracle truncates milliseconds, so we can just follow the lead on this one.

@dmytro-gokun
Copy link
Contributor Author

@dkozickis You are right. There is no real need to over-complicate this. If someone really cares about fractional milliseconds and rounding, they should probably do it server-side.

src/parseJSON/index.js Outdated Show resolved Hide resolved
@dkozickis
Copy link
Contributor

Thank you @dmytro-gokun !

@kossnocorp kossnocorp merged commit 7188ea5 into date-fns:master Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants