Bump @formatjs/intl-datetimeformat

[plesiecki]

Hello,
I would like to update the @formatjs/intl-datetimeformat dependency as it polyfills the Date class better from version 5.0.2.

[plesiecki]

/ok-to-test

[JakeChampion]

@plesiecki the /ok-to-test command can only be used by people with write access to the repository. I think it also requires the sha of the commit you want to test E.G. /ok-to-test df6f242

[plesiecki]

@JakeChampion Thanks for info 👍

[plesiecki]

Hello @chee. I'm sorry to bother you again, but may I ask you to run tests on this one too, please?

[chee]

/ok-to-test aa0246b

[plesiecki]

Well, it doesn't look like any tests have started.

[chee]

Well, it doesn't look like any tests have started.

it sure doesn't! this is on my todo list for later. i will do it the way i did the other one.

[chee]

Well, it doesn't look like any tests have started.

it sure doesn't! this is on my todo list for later. i will do it the way i did the other one.

https://github.com/Financial-Times/polyfill-library/actions/runs/2338900248

[plesiecki]

I've fixed package.json. Are we going to run tests again? Previously tests passed.

[plesiecki]

Hello @chee Could you merge it or maybe run tests again please?

[JakeChampion]

@plesiecki do you know what the breaking changes included in version 5 are?

[JakeChampion]

@plesiecki do you know what the breaking changes included in version 5 are?

It looks like the breaking change is only to an eslnt rule

https://github.com/formatjs/formatjs/blob/main/packages/intl-datetimeformat/CHANGELOG.md#breaking-changes-1

[chee]

maybe we should add a test to https://github.com/Financial-Times/polyfill-library/blob/master/polyfills/Intl/DateTimeFormat/tests.js to ensure these return "Invalid Date", and make sure we don't break this later

[plesiecki]

Tests have been added ;)

[chee]

https://github.com/Financial-Times/polyfill-library/actions/runs/2382944743

[plesiecki]

@chee Let's try one more time 🙏

[plesiecki]

Phew, all test passed 🎉

[chee]

let's go

[plesiecki]

Looking forward to the release 🙏

[plesiecki]

@chee Please 🙏

[romainmenke]

@plesiecki Thank you again for this update and sorry for the delays :)
We will try to release this in the coming week.

[plesiecki]

Lovely, looking forward to the release.

[romainmenke]

@plesiecki I fear we will have to revert this change because there seems to be a serious performance regression.

I've opened an issue upstream : formatjs/formatjs#3747

I might have jinxed it by saying it would get released in the coming week

[plesiecki]

Oh, that's a pity. Where can I see logs from failing tests?

[romainmenke]

Absolutely : https://github.com/Financial-Times/polyfill-library/runs/7694243715?check_suite_focus=true

Our test suite doesn't really have any special handling or good error messages for these timeouts.
But I noticed something was wrong when the run in Internet Explorer suddenly took more than 20 minutes.

Hopefully it gets resolved soon upstream so that we can merge the updates here :)

[longlho]

hmm it seems weird that IE9 is ok but not IE10/11? I'd say perf regression is expected to a certain degree, since CLDR & IANA data only gets bigger over time. Those 2 are the only major changes from 4.1.0 to 5.0.0 and it's not something we can fix/revert.

Algorithm-wise it's stable cause the spec hasn't changed