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

Handle time-range start & end string that contain timezone postfixes like +00:00 #6

Closed
emrysal opened this issue Aug 19, 2021 · 3 comments

Comments

@emrysal
Copy link

emrysal commented Aug 19, 2021

Currently when configuring a time-range it handles converting a ISO8601 time string to a ISO8601 time string without seperators, this works pretty well:

timeRange?.start.replace(/[-:.]/g, '')

// "2018-10-17T20:49:00Z".replace(/[-:.]/g, "")
// "20181017T204900Z" = neat

Based on which Date library or formatting you use however, I believe there are a couple of valid isodate strings that this library should support:

  • 2018-10-17T20:49:00+00:00 (Long form UTC, I believe this should auto-convert to Z) - currently this becomes the invalid string: 20181017T204900+0000.
  • 2018-10-17T20:49:00+08:00 (Local time, must not be used rfc5545) - currently becomes 20181017T204900+0800. My thoughts are this should strip the +0800 part OR maybe even throw an error that the user passed a non-UTC timestamp.

👍 Great library - willing to help contribute the above, let me know what you think.

@natelindev
Copy link
Owner

What I recommend doing is to use Date object, or whatever datetime libraries that have toISOString() function and pass the result here instead of using raw timeString. Since this is not a datetime library, I'm not quite willing to add too much datetime logic.

@emrysal
Copy link
Author

emrysal commented Aug 20, 2021

I can agree with that; however changing the string out for a date would be a breaking change - in addition you'd still need to postprocess toISOString() to get valid input.

// note, regex unmerged for clarity - removes milliseconds and seperator characters.
date.toISOString().replace(/\.[0-9]{3}/, '').replace(/[-:.]/g, '')

// potential implementation:
timeRange?.start.toISOString().replace(/(\.[0-9]{3}|[-:.])/g, '')

@natelindev
Copy link
Owner

All ISO8601 should be supported in v1.1.0 now, and non-ISO8601 format will cause an error be thrown. Since time range is now validated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants