locales/tz

[janat08]

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

---

#498

• reverts to using locale file if its added, you'd basically want to add it first before setting a locale, because I'm not sure that it will be overwritten
• dd is two letters long when using intl, it doesn't natively support the option
• won't respect format order when using intl, or repeat items
• both tz and offset can be added with options
• doesn't support fallback locales [en-us,en-gb] won't work in intl
• summary format options like LTS/LLL aren't implemented as I don't know if they're even used
• it defaults to using libraries default if not format is provided in en locale

[codecov-io]

Codecov Report

Merging #608 into dev will decrease coverage by 9.73%.
The diff coverage is 86.36%.

@@           Coverage Diff            @@
##            dev     #608      +/-   ##
========================================
- Coverage   100%   90.26%   -9.74%     
========================================
  Files       154      157       +3     
  Lines       986     1438     +452     
  Branches    141      236      +95     
========================================
+ Hits        986     1298     +312     
- Misses        0      109     +109     
- Partials      0       31      +31

Impacted Files	Coverage Δ
src/plugin/timezones/timezonesTestData.js	60.98% <ø> (ø)
src/plugin/timezones/polyfillLoader.js	100% <100%> (ø)
src/index.js	91.81% <72.85%> (-8.19%)	⬇️
src/plugin/timezones/index.js	97.22% <97.22%> (ø)

---

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 e9a2c28...812a7ef. Read the comment docs.

[janat08]

This node doesn't have complete ICU bundled, so it can't translate texts. (reason why test fails)

[janat08]

I've also applied for bounty on #46, even if at large they expect API that of moment, which I wouldn't mind getting into. There's already another pull request for adding timezone support though, without the intl usage.

[janat08]

Removed dependency on date-fns-tz in production and the build somehow got bigger from 3.35 to 3.4, from original 2.52. The api is more moment like. I didn't mess with utc plugin or how it's used, nor changed api to basically assume by default that new instances are in utc if not specified otherwise.

[corysimmons]

Even if this doesn't get dayjs all the way there, it looks like a really good start.

I'm glad you removed the date-fns dep. It's weird the build got bigger. If you just copy/paste the module from date-fns it should be the same. Even if it gets bigger I think I'd prefer it without the dep.

@iamkun This looks like a good start to timezone support. 👏

[janat08]

I write trash tests, and then I write mindbending code, that isn't following desired api anyhow. Spectacular.

[janat08]

It's done except for skipped tests, and above notes. One of the bigger issues is that time modifications-additions/subtractions- won't take into account dst, and dst calculation and detection aren't non-trivial. Calling tz with the same timezone would do it (this.timeZone), or calling the constructor method, or modifying offsetModifier if the above doesn't work- which is in minutes with the example in constructor method, for using offset as way to modify time.

[janat08]

Btw in terms of backwards DST it will straight up skip an hour, so adding 2 hours just before backwards DST will add 3 instead.

[janat08]

I believe caching/calculating DST in methods that output date like toDate()/format- which already does- would be ideal, instead of always running DST checks. (by way of modifying offsetModifier, with that in mind $utc method already takes that into account).

[janat08]

Another thing to mention is that the plugin itself uses add/subtract with the assumption that it's not dst aware

[iamkun]

Thanks for this huge PR. Seems this PR changed core bundle too much to implement this tz feature?

[janat08]

It adds moment tz data for tests. I don't think it's tree shaking the stuff used in tests.

[janat08]

Or it might be adding the polyfill that shouldn't load by default unless you specify locale for which you haven't loaded dayjs data.

[janat08]

Before I added moment data it was 3.4, and it's still 3.4 Removing polyfill call brings it to 3.28. Removing test packages and data doesn't help. Removing intl locales functionality brings it down to 2.78 without the polyfill, so I suppose it's just massive.

[iamkun]

All right. Thanks. Need some time reviewing this large PR.

[janat08]

The original size being 2.52.

[janat08]

I just remembered that I haven't updated d.ts. I don't know how to make the distinction the there's tz method on prototype and instance.

[focux]

I'd like to see this PR implemented, I love dayjs but the requirements of my company have changed and we need to support different timezones so I'd have to go back to momentjs.

[janat08]

I believe npm lets you install packages from specific commits/branches. Be warned that this pr doesn't include interoperability with some of other dayjs methods (they won't recognize dst).

[trylovetom]

@janat08 @iamkun Any Update ? I can't wait for this amazing PR.

[janat08]

I wouldn't mind implementing the ineroperability with rest of library so long as I know what iamkun would be happy with-implementation wise, or splitting this pr down into two.

[janat08]

The localization of text could probably be its own plugin. I don't know how to stop the polyfill from being bundled even if telling rollup.js about it.

[iamkun]

@janat08 Finally got time review this PR. Thanks. The reason why I haven't merge this PR is it changed our main code a lot and the plugin itself is a little bit complicated( but timezone is complicated). I'm looking for a way to implement tz support in a simple way, or just split this plugin into a separate repo.

[iamkun]

After careful reviewing this pr, we decided to pending it because,

1. too much change on the main bundle file
2. Some DST test result error e.g.

dayjs.tz('2017-03-11 02:00:00', 'America/Los_Angeles').add(1, 'day').format()
// 2017-03-12T02:00:00-08:00 (result , wrong)
// 2017-03-12T02:00:00-07:00 ( should be this)

[janat08]

Yes, I said that I havent gone about running dst checks when the methods like add are run. I also suggested extracting the code into separate packages.

[janat08]

Should I get to it as I have nothing better to do currently. Or more accurately I could use the money.

[iamkun]

@janat08 Thanks anyway. I'm currently digging all the source codes of other libs to look for a better way for us.

[janat08]

The only lib that uses intl: https://date-fns.org/v2.4.1/docs/Time-Zones

At first I was just going to use that, not sure why I didn't.

[iamkun]

also luxon

[janat08]

Any chance my code is actually better?

[janat08]

Could use the bounty.

[janat08]

@iamkun

[janat08]

@iamkun

[janat08]

Its been half a year

[janat08]

At this rate they'll probably improve the intl api, and this would be fixing a problem that doesn't exist. If you really want to be conservative about performance why not have users manually trigger DST checks?

[iamkun]

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