Fix: performance issue in tz plugin

[logeyG]

This PR is an adaptation of the following closed PR: #1889

Fixes performance issue in timezone plugin by using a cached Intl.DateTimeFormat formatter instead of toLocaleString(). No functional changes, just an order of magnitudes faster.

The only change I made was to create a default config object that is used for both getDateTimeFormat as well as getLocaleStringifier.

Fixes #1236

[defghy]

It seems this 2 function not generate same format;

In my case, getLocaleStringifier worked fine, use getDateTimeFormat make date not correct

[logeyG]

Hi @defghy, yes you are correct, but I don't think this is an issue. The dates are the same, just in different formats. The previous code that getLocaleStringifier is replacing had the exact same format when using toLocaleString instead (see line 115):

let t = new Date().toLocaleString('en-US', { timeZone: 'Europe/Paris' })
'8/10/2022, 10:07:10 AM'

[tevin]

looks good to me, great fix for the perf issue

[defghy]

Has some problem when date is Invalid Date;

In this situation, an Error throw;
Maybe need some try/catch to remain same as before

@logeyG

[logeyG]

@defghy this actually seems to be an improvement, in my opinion.

I think it is OK to throw an error if anything other than a date object is passed in. I can't think of an instance where catching the error is the preferred behavior here.

[logeyG]

@iamkun can you please review this?

[logeyG]

@iamkun can you please review this?

[Prabhakar-Poudel]

@BePo65 @iamkun any chance of getting this reviewed?

[kamrankhan54]

Anyone know when this could be merged please? We are blocked from using DayJS due to the performance issue mentioned here. @logeyG @iamkun

[Klaitos]

up 👍

[Serg-Mois]

up up up)

[tgensol]

up up ?

[tduyng]

Why haven't there been any updates for this issue?

[serg-mois-capital]

Repo looks like dead, the only author doesn't support it.

[dominikbrazdil]

up

[kozak-codes]

Thanks for making such an awesome project @logeyG @iamkun ! I'm hoping to bump this up in your priority list to review as the current state is currently contributing to 2.5% of my nodejs backend application's CPU profile with a wall time of about 162ms. This would be an easy win for improving our applications speed. We've seen tons of performance improvements since switching from moment but this one is definitely an outlier.

Thanks again!

[janousek]

@logeyG This fix is not correct. Try this:

let date = new Date(2023,9,2,0,0,0);

let val1 = new Intl.DateTimeFormat('en-US', {
		year: 'numeric',
		month: '2-digit',
		day: '2-digit',
		hour: '2-digit',
		minute: '2-digit',
		second: '2-digit',
		hour12: false,
		timeZone: 'Europe/Prague',
    })
	.format(date);

let val2 = date.toLocaleString('en-US', { timeZone: 'Europe/Prague' });


console.log(val1, 'VS', val2);
console.log(new Date(val1), 'VS', new Date(val2));

I belive that you have to set hour12 to true.

[keithwillcode]

@iamkun Bumping this as well. Seeing major performance issues with using the tz plugin as it exists now. The changes proposed in this PR result in orders of magnitude better perf. Happy to help with testing or whatever is needed. Let me know. 🙏

[JorgeHabib]

Hello guys, firstly I'd like to thank you @iamkun for all for the effort you put into enhancing the dayjs library.

I've also encountered some performance challenges related to the timezone function, which have been impacting my work. I came across PR #2542 and noticed that it addresses these issues effectively, as well as this PR.

Could you share any insights on the timeline for the next release that would include these significant improvements? This update would greatly benefit our projects, and we're looking forward to integrating it.

Thank you for your continued dedication to improving dayjs, we love it.

[adlanarifzr]

For temporary measure, you guys can use my forked library for the timezone issue. I have merged this PR.

https://www.npmjs.com/package/@adlanarifzr/dayjs

import timezone from '@adlanarifzr/dayjs/plugin/timezone';
dayjs.extend(timezone);

[JorgeHabib]

For temporary measure, you guys can use my forked library for the timezone issue. I have merged this PR.

https://www.npmjs.com/package/@adlanarifzr/dayjs

import timezone from '@adlanarifzr/dayjs/plugin/timezone';
dayjs.extend(timezone);

These improvements have significantly enhanced our performance here. Are you/anyone aware of any other functions that might be encountering similar performance issues?

[rushi]

@iamkun Is it possible for you to review this? It solves a major performance issue with the timezone plugin

[ikeyan]

#2753 is an improved version of this PR I made.
I fixed this bug and this breaking change, improved performance even more, and shrunk the gzipped code size.

[ngouy]

@defghy this actually seems to be an improvement, in my opinion.

I think it is OK to throw an error if anything other than a date object is passed in. I can't think of an instance where catching the error is the preferred behavior here.

As much as it seem reasonable, it's a breaking change
For example react-big-calendar always start it's state with some null-ish data and start internally to format it. With regular TZ it just silence it and then it's using user param to override state and everyone is happy

With this version, you can try ANY param, state is always initiated with null-ish values and it just crashes