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

18.20, 20.12 update changed output of Date().toLocaleString('nl'); #52244

Closed
cvisserMaxedy opened this issue Mar 28, 2024 · 5 comments
Closed
Labels
icu Issues and PRs related to the ICU dependency.

Comments

@cvisserMaxedy
Copy link

cvisserMaxedy commented Mar 28, 2024

Version

18.20.0, 20.12.0

Platform

Linux, Mac

Subsystem

No response

What steps will reproduce the bug?

in Node 18.19, 20.11:
> new Date().toLocaleString('nl');
'28-3-2024 08:17:14'
in Node 18.20, 20.12:
> new Date().toLocaleString('nl');
'28-3-2024, 08:17:14'
After the 18.20 update a comma was added after the date.

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior? Why is that the expected behavior?

I would not expect output to just change. Broke some tests for us at least

new Date().toLocaleString('nl');
'28-3-2024 08:17:14'

What do you see instead?

new Date().toLocaleString('nl');
'28-3-2024, 08:17:14'

Additional information

No response

@cvisserMaxedy cvisserMaxedy changed the title 18.19 -> 18.20 update changed output of Date().toLocaleString('nl'); 18.20, 20.12 update changed output of Date().toLocaleString('nl'); Mar 28, 2024
@cvisserMaxedy
Copy link
Author

Update, just tested this on node 20.12 and it has the same issue (not present in node 20.11)

@climba03003
Copy link
Contributor

According to #51090 (comment)
The output is not guarantee to be the same over the same major.
You should never rely on the output of formatted date.

cc @flakey-bit
ICU 74.2 released on Dec 12 2023
Chromium picked it up on Mar 13 2024
So, it will eventually the same on Chromium / Chrome in later time.

@Rucadi
Copy link

Rucadi commented Mar 29, 2024

According to #51090 (comment)
The output is not guarantee to be the same over the same major.
You should never rely on the output of formatted date.

cc @flakey-bit
ICU 74.2 released on Dec 12 2023
Chromium picked it up on Mar 13 2024
So, it will eventually the same on Chromium / Chrome in later time.

In my opinion, breaking changes that don't have positive sides or just for the sake of changing should not be allowed on the same major.
Only makes people lives harder and (maybe not up-to-best practices software but) already working software, that may be used in unimaginable contexts, fail.

@climba03003
Copy link
Contributor

climba03003 commented Mar 29, 2024

In my opiniom, breaking changes that don't have positive sides or just for the sake of changing should not be allowed on the same major.

I am not member of Node.js, I know it is not optimal and I asked for the same reason.
But the answer I received is NO. and here I just forward the same answer to you.

ICU data handling is hard, because it always changing and breaking.
Waiting until next major also not optimal either because some people may see as bug (outdated) too.

@VoltrexKeyva VoltrexKeyva added the icu Issues and PRs related to the ICU dependency. label Mar 30, 2024
@aduh95
Copy link
Contributor

aduh95 commented Apr 3, 2024

breaking changes that don't have positive sides or just for the sake of changing should not be allowed on the same major.

The issue is that such changes are quite hard to detect, and even harder to decide if such change has a positive side or not – e.g. in this case, for someone who is not Dutch, it's hard to judge whether that comma is important or not. As you can imagine, there are much harder cases, and even with all the time in the world, there would be no way for us to try to assess each change.

I'm going to close this issue as "Won't fix".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
icu Issues and PRs related to the ICU dependency.
Projects
None yet
Development

No branches or pull requests

5 participants