-
Notifications
You must be signed in to change notification settings - Fork 121
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
fix: switch to momentjs to handle timezones #436
Conversation
to be merged after #430 |
Codecov Report
@@ Coverage Diff @@
## master #436 +/- ##
==========================================
+ Coverage 97.87% 97.88% +<.01%
==========================================
Files 40 41 +1
Lines 2921 2925 +4
Branches 706 708 +2
==========================================
+ Hits 2859 2863 +4
Misses 55 55
Partials 7 7
Continue to review full report at Codecov.
|
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.
This makes sense! Looks good and I like date_time.ts - helped clarify #430 for me 😅
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.
Luxon use Init.DateTimeFormatter for handling timezones, but this is browser dependent. Moving to momentjs will allow us to be compliant to Kibana
d75e78c
to
2504e9a
Compare
jenkins test this please error on downloading nodejs
|
retest |
jenkins test this please |
1 similar comment
jenkins test this please |
## [13.5.7](v13.5.6...v13.5.7) (2019-10-23) ### Bug Fixes * switch to momentjs to handle timezones ([#436](#436)) ([a9f98c8](a9f98c8))
🎉 This PR is included in version 13.5.7 🎉 The release is available on: Your semantic-release bot 📦🚀 |
## [13.5.7](elastic/elastic-charts@v13.5.6...v13.5.7) (2019-10-23) ### Bug Fixes * switch to momentjs to handle timezones ([opensearch-project#436](elastic/elastic-charts#436)) ([cd15932](elastic/elastic-charts@cd15932))
Summary
The current used time library,
Luxon
is usingIntl.DateTimeFormatter
as the main source for handling time zones. That library depends on browsers implementation. IE11 doesn't have any timezone and it will need a polyfill.This PR replace
luxon
withmomentjs
andmoment-timezone
. I'm leavingluxon
as a devDependency on the tests and on stories. We can remove it in a future phase.momentjs
andmoment-timezone
are configured aspeerDependency
since Kibana is already using that library.I've checked that fix cross-linking the build with Kibana. I've tested various timezones in Discover chart.
fix #428
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.src/index.ts
(and stories only import from../src
except for test data & storybook)