Skip to content
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

Update node 18 to use ICU4C 74 #51556

Closed
abster opened this issue Jan 23, 2024 · 4 comments
Closed

Update node 18 to use ICU4C 74 #51556

abster opened this issue Jan 23, 2024 · 4 comments
Labels
feature request Issues that request new features to be added to Node.js.

Comments

@abster
Copy link

abster commented Jan 23, 2024

What is the problem this feature will solve?

LTS versions node 18 and 20 are not on the latest ICU4C release (ICU4C 74).

What is the feature you are proposing to solve the problem?

Update node 18 and node 20 to use ICU4C 74.

I noticed that node 21 (current version) was updated to use ICU4C 74.1 couple of months ago, but LTS versions node 18 and 20 are still on ICU4C 73.2.

What alternatives have you considered?

No response

@abster abster added the feature request Issues that request new features to be added to Node.js. label Jan 23, 2024
@anonrig
Copy link
Member

anonrig commented Jan 24, 2024

cc @nodejs/releasers

@richardlau
Copy link
Member

This is a tricky one because ICU updates typically change formatting between releases and we often get issues complaining about broken snapshot tests when we update ICU majors (the advice from ICU developers is to not rely on the output of any localizations).

FWIW ICU 74.1 (in Node.js 21) has a bug that is fixed in 74.2: #51090
Our dependency update automation hasn't picked up ICU 74.2 yet because of https://unicode-org.atlassian.net/browse/ICU-22622

@richardlau richardlau linked a pull request Mar 10, 2024 that will close this issue
@richardlau
Copy link
Member

This happened in Node.js 18.20.0.

@sphassan
Copy link

sphassan commented May 8, 2024

Putting this here for visibility in case other developers run into the same issue I did in the hope that they waste less time than me hunting this down. The ICU update from 73 to 74 did, in fact, cause a breaking change by upgrading from CLDR 43 to CLDR 44. Specifically CLDR-16358 changed all Spanish speaking locales in the Americas from using a 24 hour clock to a 12 hour clock which will become immediately apparent if you're using toLocaleString or similar. This, in turn, introduces the no-break space issue many have mentioned in this repo before by using a no-break in between "a."/"p." and "m." instead of simply not having a space there at all.

Before: "2 ene 2020, 0:00:00 UTC"
After: "2 ene 2020, 12:00:00 a. m. UTC"

There is, unfortunately, no easy answer or fix to this, and the process of discovering all of this did little but inform me of the sheer futility of the testing suite I inherited - but that's verging dangerously close to venting frustration.

Oh, and for those thrown by the comma after the year, yeah that one was added by an earlier change in the same library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants