Skip to content

TimeZone - a plugin with time zone support for parsing and formatting strings - #325

Closed
prantlf wants to merge 8 commits into
iamkun:masterfrom
prantlf:format-timezone
Closed

TimeZone - a plugin with time zone support for parsing and formatting strings#325
prantlf wants to merge 8 commits into
iamkun:masterfrom
prantlf:format-timezone

Conversation

@prantlf

@prantlf prantlf commented Sep 4, 2018

Copy link
Copy Markdown
Contributor

Day.js Time Zone Plugin https://day.js.org/docs/en/timezone/timezone







Attempts to implement #323 by introducing the TimeZone plugin.

It uses the IANA time zone database, packed to a JSON format by moment-time-zone and with a simple interface to it provided by timezone-support.

Although the Intl time zone support has been introduced, there are still browsers or platforms, which do not support it and would not mind "paying for" the support by including more JavaScript code. The size of the time zone supporting library without the Day.js plugin: source 185 kB, minified 182 kB, gzipped 24 kB.

I can imagine adding support for remembering the time zone for future calls to .format() as discussed at #46.

Time zone conversions are possible already without an additional API. Jut use UTC date from one Day.js object and format it the target time zone using the other (or the same) Day.js object.

@prantlf

prantlf commented Sep 4, 2018

Copy link
Copy Markdown
Contributor Author

The build fails only on Node.js 6, because of the usage of async keyword in build/index.js. @iamkun, do you want to drop the Node.js 6 support, or use only JavaScript features supported by Node.js 6?

$ npm run build
> dayjs@0.0.0-development build /home/travis/build/iamkun/dayjs
> cross-env BABEL_ENV=build node build && npm run size
/home/travis/build/iamkun/dayjs/build/index.js:13
async function build(option) {
      ^^^^^^^^
SyntaxError: Unexpected token function

@codecov-io

codecov-io commented Sep 5, 2018

Copy link
Copy Markdown

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #325   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          32     33    +1     
  Lines         386    441   +55     
  Branches       53     63   +10     
=====================================
+ Hits          386    441   +55
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø) ⬆️
src/plugin/timeZone/index.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b14bdd1...505b201. Read the comment docs.

@iamkun

iamkun commented Sep 5, 2018

Copy link
Copy Markdown
Owner

we build dayjs only on CI (node 10)

@prantlf

prantlf commented Sep 5, 2018

Copy link
Copy Markdown
Contributor Author

@iamkun , this is your configuration in .travis.yml:

node_js:
  - '10'
  - '8'
  - '6'

I opened PR #327, which drops the Node.js 6 line, for your consideration. Thanks!

Add test for parsing a string with time zone.
@prantlf

prantlf commented Sep 6, 2018

Copy link
Copy Markdown
Contributor Author

Oh, I am sorry, @iamkun. I implemented other workaround for not having the built files for testing.

I will need to figure out the code coverage yet, which does not show me any report.

@iamkun

iamkun commented Sep 6, 2018

Copy link
Copy Markdown
Owner

@prantlf you may could run npm run test on your local machine.

@prantlf

prantlf commented Sep 6, 2018

Copy link
Copy Markdown
Contributor Author

Yes, that helped, thanks! :-) I think, that I confused codecov by rebasing the PR branch to the current master and forcing the re-push.

* Use the timezone-support module to get a lightweight time zone support.
* Construct Dayjs from a zone-less string and an explit time zone.
* Format a string with Dayjs converted to other time zone.
It would be possible to bundle the timezone-support module to the timeZone plugin. It is a single-file dependency.
Upgrade timezone-support to 1.3.0 suporting dayOfWeek.
@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #325   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          32     33    +1     
  Lines         386    440   +54     
  Branches       53     63   +10     
=====================================
+ Hits          386    440   +54
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø) ⬆️
src/plugin/timeZone/index.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b14bdd1...19f3f70. Read the comment docs.

@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #325   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          32     33    +1     
  Lines         386    442   +56     
  Branches       53     63   +10     
=====================================
+ Hits          386    442   +56
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø) ⬆️
src/plugin/timeZone/index.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b14bdd1...19f3f70. Read the comment docs.

3 similar comments
@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #325   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          32     33    +1     
  Lines         386    442   +56     
  Branches       53     63   +10     
=====================================
+ Hits          386    442   +56
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø) ⬆️
src/plugin/timeZone/index.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b14bdd1...19f3f70. Read the comment docs.

@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #325   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          32     33    +1     
  Lines         386    442   +56     
  Branches       53     63   +10     
=====================================
+ Hits          386    442   +56
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø) ⬆️
src/plugin/timeZone/index.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b14bdd1...19f3f70. Read the comment docs.

@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #325   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          32     33    +1     
  Lines         386    442   +56     
  Branches       53     63   +10     
=====================================
+ Hits          386    442   +56
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø) ⬆️
src/plugin/timeZone/index.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b14bdd1...19f3f70. Read the comment docs.

@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #325   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          32     33    +1     
  Lines         386    442   +56     
  Branches       53     63   +10     
=====================================
+ Hits          386    442   +56
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø) ⬆️
src/plugin/timeZone/index.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b14bdd1...a8742af. Read the comment docs.

2 similar comments
@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #325   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          32     33    +1     
  Lines         386    442   +56     
  Branches       53     63   +10     
=====================================
+ Hits          386    442   +56
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø) ⬆️
src/plugin/timeZone/index.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b14bdd1...a8742af. Read the comment docs.

@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #325   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          32     33    +1     
  Lines         386    442   +56     
  Branches       53     63   +10     
=====================================
+ Hits          386    442   +56
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø) ⬆️
src/plugin/timeZone/index.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b14bdd1...a8742af. Read the comment docs.

@codecov-io

codecov-io commented Sep 18, 2018

Copy link
Copy Markdown

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #325   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          32     44   +12     
  Lines         386    481   +95     
  Branches       53     65   +12     
=====================================
+ Hits          386    481   +95
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø) ⬆️
src/plugin/timeZone/index.js 100% <100%> (ø)
src/locale/es.js 100% <0%> (ø) ⬆️
src/locale/de.js 100% <0%> (ø) ⬆️
src/locale/fr.js 100% <0%> (ø) ⬆️
src/locale/it.js 100% <0%> (ø) ⬆️
src/locale/ja.js 100% <0%> (ø) ⬆️
src/locale/ru.js 100% <0%> (ø) ⬆️
src/locale/es-es.js 100% <0%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b14bdd1...341d3ba. Read the comment docs.

@prantlf

prantlf commented Sep 18, 2018

Copy link
Copy Markdown
Contributor Author

@pedro-mass, true, I ignored the day of the week in the original submission. I added it by the latest update. All properties of the days object should be covered now.

@pedro-mass

Copy link
Copy Markdown

@prantlf This looks awesome, thanks for the hard work and contribution!
Your timezone-support library also seems pretty interesting. Keep up the good work 👏

@prantlf

prantlf commented Sep 29, 2018

Copy link
Copy Markdown
Contributor Author

Thank you! Let's see how it goes on :-)

I was thinking about introducing an interface compatible with Moment.js:

// Alternative to the constructor:
// dayjs('2018-09-29 21:34', { timeZone: 'Europe/Berlin' })
dayjs.tz('2018-09-29 21:34', 'Europe/Berlin')

// Convert an existing Day.js instance to other time zone
dayjs().tz('Europe/Berlin')

The alternative to the constructor means no problem to the further usage. You might question if it is worth adding, because you need to pass the time zone as a parameter anyway, unlike the static method dayjs.utc(...). Another thing might be the lack of the instance method dayjs().tz(...), because it may not be wanted because of its problems. Anyway, no technical no problems, just some artificial subjective opinions ;-)

The time zone conversion for a Day.js instance may confuse the end-user. Day.js stores a Date instance internally and caches the date and time parts in this.$* properties used by getting and formatting methods. The Date object does not support arbitrary time zones. How to store a date in a particular time zone?

  1. Overhaul Day.js not to use the Date object. The converting method toDate() would return the native Date object in the local time, possibly in a different time zone, than the Day.js instance. This would break the plugins, which depend on this.$d.
  2. Add a new object to store the time-zone-aware date. This would make the Day.js instance more complicated and need some synchronization with this.$d to prevent confusion.
  3. Do not store the time zone information in Day.js instances. Use it only in the constructor/parse method to convert the input to UTC and store it in the Day.js instance, and in the formatting method to output a date string in a request ed time zone. This would not allow to carry the original time zone information together with the date value.

The first approach would be interesting to prototype. It could turn up lightweight and quick. I chose the third approach, because it was not intrusive, it was feasible as a plugin and the interface could be kept if the first approach won later.

However, The third approach does store the time zone information publicly in a Day.js instance, which means, that converting one Day.js instance to another one in an arbitrary time zone is not possible. When I could not add the instance method tz, I did not find adding the static method tz a good idea.

@aralroca

aralroca commented Oct 2, 2018

Copy link
Copy Markdown

@prantlf Thanks for that great contribution! The interface compatible with Moment.js would be very great. 👏

@prantlf

prantlf commented Oct 7, 2018

Copy link
Copy Markdown
Contributor Author

@aralroca, I'm afraid, that I went for the option 3 from my comment above, which doesn't allow implementing the interface fully compatible with Moment.js... :'-{

I could implement the static tz method parsing the input in the specified time zone. However, the instance method tz cannot be implemented with the the option 3, because this option doesn't store the time zone in the dayjs object.

The trouble with the static tz from the option 3 is, that it converts the date to the local time zone, which is the only one supported by the native Date object.

const date = dayjs.tz('2018-09-29 21:34', 'Europe/Berlin')
// Some code here, or the date may be passed to a distant function
const output = date.format('DD/MM/YYYY')

What would you expect from such dayjs object? Would you expect the output in the original time zone, or in the local time, as it is done now?

@petermikitsh

Copy link
Copy Markdown

Hi, I'm inquiring to the status of this PR. Would be incredibly useful to have timezone support in dayjs. Thanks.

Repository owner deleted a comment from pedro-mass Feb 24, 2019
@danielb2

Copy link
Copy Markdown

curious about status too

@petermikitsh

Copy link
Copy Markdown

@danielb2 You might not need dayjs. Intl.DateTimeFormat may give you what you need.

https://caniuse.com/#search=datetimeformat

For older browsers, there's a polyfill (npm module intl):

https://bundlephobia.com/result?p=intl@1.2.5

@bissolli

Copy link
Copy Markdown

@iamkun @prantlf, any news on it? Would be amazing having this plugin.

@trylovetom

Copy link
Copy Markdown

Any Update? I can't wait for this plugin.

@iamkun

iamkun commented Jul 29, 2019

Copy link
Copy Markdown
Owner

This pr introduced a heavy dependency which is not really suitable to dayjs. We will introduce a time zone plugin base on Intl.DateTimeFormat.

Day.js Time Zone Plugin https://day.js.org/docs/en/timezone/timezone

@iamkun iamkun closed this Jul 29, 2019
@trylovetom

Copy link
Copy Markdown

@iamkun so you will adopt #608 ?

@iamkun

iamkun commented Aug 4, 2020

Copy link
Copy Markdown
Owner

Comment thread docs/en/Plugin.md

| Format | Output | Description |
| ------ | ------ | ----------------------- |
| `z` | CEST | Time zone abbreviation |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamkun any chance to implement this as part of the timezone plugin?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akoskm I have the same need and draft a PR here #1069

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

Successfully merging this pull request may close these issues.