Closed
Conversation
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
ritave
force-pushed
the
ritave/iso8601
branch
from
325965f to
e1bdde6
Compare
legobeat
changed the title
Contributor
|
The Maybe this is fine for our purposes at the moment but did you also consider the new Temporal API? I feel that would make it more future-proof and reduce reliance on any single implementation.
Libraries: |
hmalik88
reviewed
Oct 16, 2024
| export const InvalidIso8601Date = new Error('Invalid ISO-8601 date'); | ||
|
|
||
| /** | ||
| * Parses ISO-8601 date time string. |
Contributor
There was a problem hiding this comment.
Suggested change
| * Parses ISO-8601 date time string. | |
| * Parses ISO-8601 extended format date time string. |
hmalik88
reviewed
Oct 16, 2024
Comment on lines
+225
to
+232
| assert(year !== undefined, 'Invalid ISO-8601 parser state'); | ||
| month = month ?? '01'; | ||
| date = date ?? '01'; | ||
| hours = hours ?? '00'; | ||
| minutes = minutes ?? '00'; | ||
| seconds = seconds ?? '00'; | ||
| offsetHours = offsetHours ?? '00'; | ||
| offsetMinutes = offsetMinutes ?? '00'; |
Contributor
There was a problem hiding this comment.
Mind leaving a comment about how this is accounting for partial dates as that is something that ISO-8601 allows?
hmalik88
reviewed
Oct 16, 2024
Comment on lines
+57
to
+58
| }; | ||
| const consumeSeparator = (sep: '-' | ':') => { |
Contributor
There was a problem hiding this comment.
Suggested change
| }; | |
| const consumeSeparator = (sep: '-' | ':') => { | |
| }; | |
| const consumeSeparator = (sep: '-' | ':') => { |
hmalik88
reviewed
Oct 16, 2024
Comment on lines
+53
to
+54
| }; | ||
| const skip = (count = 1) => { |
Contributor
There was a problem hiding this comment.
Suggested change
| }; | |
| const skip = (count = 1) => { | |
| }; | |
| const skip = (count = 1) => { |
hmalik88
reviewed
Oct 16, 2024
Comment on lines
+47
to
+48
| }; | ||
| const peek = () => { |
Contributor
There was a problem hiding this comment.
Suggested change
| }; | |
| const peek = () => { | |
| }; | |
| const peek = () => { |
hmalik88
reviewed
Oct 16, 2024
Comment on lines
+179
to
+191
| case ParseDateState.Timezone: | ||
| timezone = consume(); | ||
|
|
||
| if (timezone === 'Z') { | ||
| if (peek() !== END) { | ||
| throw InvalidIso8601Date; | ||
| } | ||
| state = ParseDateState.End; | ||
| } else if (['-', '+'].includes(timezone)) { | ||
| state = ParseDateState.TimezoneHour; | ||
| } else { | ||
| throw InvalidIso8601Date; | ||
| } |
Contributor
There was a problem hiding this comment.
This logic should probably be updated in accordance with MetaMask/SIPs#152
Member
|
No longer needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SIP-28: Background Events requires support for ISO-8601 date and times with timezones.
I found no viable library to support our use-case and thus implemented our own.