-
Notifications
You must be signed in to change notification settings - Fork 151
Replace unclear fromStandardDate() #1290
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
base: 6.x
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 ⌚ 🙅
🙏 ☮️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My brain was close to melting while trying to understand this PR 🫠
I think example code would go a long way. Something like this (maybe split the examples and spread them across the functions' docs)
// will do what it says on the tin
Neo4jDate.fromStandardDateLocal(new Date(2020, 01, 01))
Neo4jDate.fromStandardDateUTC(new Date(Date.UTC(2020, 01, 01)))
// Depending on the host's locale, might end up with a different calendar date
// than 2020-01-01
Neo4jDate.fromStandardDateLocal(new Date(Date.UTC(2020, 01, 01)))
Neo4jDate.fromStandardDateUTC(new Date(2020, 01, 01))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional: I think it might be very much worthwhile to run these tests for different time zones/locales. In particular the ones that are far off of UTC (and maybe also UTC itself).
https://stackoverflow.com/questions/8083410/how-can-i-set-the-default-timezone-in-node-js
* fromStandardDateLocal(new Date("2010-10-10T00:00:00")) // This may cause issues as JS Dates are created at local time by default | ||
* fromStandardDateLocal(new Date("2010-10-10T00:00:00Z")) // Will create a date at 2010-10-10 as this date is created at UTC with the trailing "Z" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* fromStandardDateLocal(new Date("2010-10-10T00:00:00")) // This may cause issues as JS Dates are created at local time by default | |
* fromStandardDateLocal(new Date("2010-10-10T00:00:00Z")) // Will create a date at 2010-10-10 as this date is created at UTC with the trailing "Z" | |
* fromStandardDateUTC(new Date("2010-10-10T00:00:00")) // This may cause issues as JS Dates are created at local time by default | |
* fromStandardDateUTC(new Date("2010-10-10T00:00:00Z")) // Will create a date at 2010-10-10 as this date is created at UTC with the trailing "Z" |
The
Neo4j.Date
functionsfromStandardDate()
andtoStandardDate()
are not inverse functions of each other, asfromStandardDate()
takes the date that the StandardDate would correspond to in local time, andtoStandardDate()
creates a Date set at Midnight UTC.This PR deprecates the old
fromStandardDate()
and replaces it with a drop-in replacementfromStandardDateLocal()
which functions the same andfromStandardDateUTC()
which takes the date as interpreted in UTC.