fix(util.convert): epoch times stored as strings convert correctly, unit test fixup, parse iso dates in local tz #699
Conversation
This file contains 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
Previously stated "Unable to convert to Date" Now states "Unable to convert to moment"
Previously, "Date" & "Moment" had identical switch branches, but each return statement in "Date" had `moment(...).toDate()`. Now, Date branch is just `convert(object, "Moment").toDate()`. Also, each now throw TypeErrors, & Date branch catches "TypeError" & rethrows its own.
…nverting to moment
yotamberk
previously approved these changes
Oct 3, 2020
as per review comment
|
Should be ready to pull @yotamberk |
yotamberk
approved these changes
Oct 5, 2020
|
🎉 This PR is included in version 7.3.10 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #698
Fixes #697
Fixes #663
Squash preferred.
Please feel free to delete examples/timeline/testOffset.html before merging, there's no need to request I do it, I really don't mind.
ISO dates in local tz:
ISO date strings with no time/timezone are now parsed to midnight that date, in local time #663 .
This is consistent with handling of ISO datetime strings that include time, but omit timezone (also treated as local time).
This difference is demonstrated in new file:
examples/timeline/testOffset.html.This file is intended to demonstrate this bug fix only - Please feel free to delete examples/timeline/testOffset.html before merging.
While writing tests for this, I realized that previous test cases for parsers were not correctly capturing when parsing failed #697 .
Unit test fixup + treat numeric strings as epoch timestamps
This exposed another bug - the broken testcases implied that the following should be equivalent:
util.convert("111111", "Date)util.convert(111111, "Date")Once fixed, they exposed a (potential) bug, that
util.convert("111111", "Date)yields and invalid date ( #698 ).This is now handled
Small changes - refactor, and more specific error throwing / handling
Additional refactor for DRY in
util.convert-util.convert(object, "Date")now callsutil.convert(object, "Moment").toDate(). (Previously duplicated logic.for type in
["Date", "Moment"]:util.convert(object, type)will return aTypeError, ("Cannot convert object of type " + getType(object) + " to type " + type) , rather than a more generalError`.