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

Intl.DateFormat changes output between Node 16.16 and Node 16.17 #44454

Closed
rmehner opened this issue Aug 30, 2022 · 11 comments
Closed

Intl.DateFormat changes output between Node 16.16 and Node 16.17 #44454

rmehner opened this issue Aug 30, 2022 · 11 comments
Labels
icu Issues and PRs related to the ICU dependency.

Comments

@rmehner
Copy link

rmehner commented Aug 30, 2022

Version

v16.17.0

Platform

Darwin 21.6.0 Darwin Kernel Version 21.6.0: Wed Aug 10 14:25:27 PDT 2022; root:xnu-8020.141.5~2/RELEASE_X86_64 x86_64

Subsystem

No response

What steps will reproduce the bug?

Given this script:

const assert = require("node:assert/strict")

const date = new Date("2022-03-16T14:25:38")
const options = { day: "2-digit", month: "long", hour: "2-digit", minute: "2-digit" }
const formatted = new Intl.DateTimeFormat("en", options).format(date)

assert.strictEqual(formatted, "March 16, 02:25 PM")

Run with Node 16.16 will run just fine, with Node 16.17 it will throw:

node:assert:124
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ 'March 16 at 02:25 PM'
- 'March 16, 02:25 PM'
           ^
    at Object.<anonymous> (script.js:7:8)
    at Module._compile (node:internal/modules/cjs/loader:1126:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1180:10)
    at Module.load (node:internal/modules/cjs/loader:1004:32)
    at Function.Module._load (node:internal/modules/cjs/loader:839:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:17:47 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 'March 16 at 02:25 PM',
  expected: 'March 16, 02:25 PM',
  operator: 'strictEqual'
}

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

Always

What is the expected behavior?

March 16, 02:25 PM

What do you see instead?

March 16 at 02:25 PM

Additional information

I suspect that #42655 introduced the change. And I guess it's fine, since current browsers I've checked actually produce the same output as 16.17, so this issue is likely just documentation for people who stumble into this :)

@cola119 cola119 added the icu Issues and PRs related to the ICU dependency. label Aug 31, 2022
@Christian0411
Copy link

Christian0411 commented Sep 1, 2022

I also noticed formatRange outputs differently as well:

const assert = require("assert/strict")

const date = new Date("2022-03-16T14:25:38")
const date2 = new Date("2022-03-16T16:25:38")
const options = {  hour: "2-digit", }
const formatted = new Intl.DateTimeFormat("en", options).formatRange(date, date2)

assert.strictEqual(formatted, "2 – 4 PM")

With node 16.17.0

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ '2 – 4 p'
- '2 – 4 PM'
         ^
    at Object.<anonymous> (/Users/chriscanizares/Desktop/index.js:16:8)
    at Generator.next (<anonymous>) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: '2 – 4 p',
  expected: '2 – 4 PM',
  operator: 'strictEqual'
}

In this case though, Chrome Version 105.0.5195.52 outputs: 2 – 4 PM

@F3n67u
Copy link
Member

F3n67u commented Sep 6, 2022

cc @nodejs/i18n-api

@srl295
Copy link
Member

srl295 commented Sep 6, 2022

Yes, these are linguistic cultural preferred forms and they can and do shift. Best is not to depend on fixed formats. See https://cldr.Unicode.org for more info on the process and delta charts.

Will add more details.

@ramonsnir
Copy link

ramonsnir commented Sep 6, 2022

@srl295 Can you point to where in the delta (DTD or tickets) this change appears? As someone who's not used to reading this changelog, it's hard to tell 🙂

Even if not depending on fixed formats, it seems strange to me that the time format for en-US would be "The meeting was set for 4 p", but maybe seeing the context in the CLDR ticket would help understand why this change happened.

@srl295
Copy link
Member

srl295 commented Sep 6, 2022

@nodejs/i18n-api we should put this in a FAQ…?

@ramonsnir try the following:

  1. running node -p process.versions.cldr will give you the CLDR versions. 40.0 vs 41.0 in this case.
  2. go to https://cldr.unicode.org/index/downloads
  3. FYI: Δ41 link will give you all tickets between 40 and 41
  4. Go to the Charts41 link
  5. Choose Delta Data
  6. English Delta and this section
  7. Shows the 'at' was dropped.

@ramonsnir
Copy link

Thank you, @srl295 , I appreciate it! 🙂 I missed a step going through the delta earlier and never even reached the right page.

@ramonsnir
Copy link

@srl295 As mentioned, it's my first time dealing with CLDR so I may be misreading things.

From the issues reported above, there were two changes in the format outputs from Node.js:

  1. Date format 'March 16, 02:25 PM' is now 'March 16 at 02:25 PM'
  2. Range format '2 – 4 PM' is now '2 – 4 p'

CLDR version 40 had two date combination formats in English: When using long/full formats, at was used. When using short/medium formats, , was used. Version 41 removes both the at and the , from the combinations (see image below). That wouldn't explain why Node.js switched its default date format from , to at. That would explain the comma being removed, but that's not what happened.

image

For the second issue, I went to the spec to see the definition of time periods. A change from PM to p would appear in the delta as (a) a change in some format from a to aaaaa, or (b) the hour from jj to something like jjjjj, or (c) somewhere else there was a change in encoding that replaced the literal string 'PM' with 'p'. I don't see a change that looks like that, in fact, I don't see any change relating to periods.

If I am misunderstanding and wasting your time, tell me and I'll drop it. However, I have a feeling that this change wasn't intended and there's a mistranslation somewhere between CLDR / v8 / Node.js.

@Mantish
Copy link

Mantish commented Oct 18, 2022

Just to clear up where this change is coming from.
It's indeed related to Unicode, but it's not actually related to the CLDR specification v41 change. That one was a change about adding the en_MV locale.

The change that this issue is referring to actually comes from ICU, which was updated to v71 in node v16.17.0. You can see that change mentioned in the ICU release notes here: https://icu.unicode.org/download/71#h.z0qyjvmbhu9m

@ryzokuken
Copy link
Contributor

Just adding a bit from the ECMA-402 point of view: Never depend on the result of a format function being something very specific, since the results are all subject to changes in locale data and information and the implementation reserves all rights to adapt them at any point of time. This is formatted locale information that should only be used to display to users.

@ryzokuken
Copy link
Contributor

Closing this since this has nothing to fix, but feel free to reopen if you disagree.

@srl295
Copy link
Member

srl295 commented Nov 3, 2022

Just adding a bit from the ECMA-402 point of view: Never depend on the result of a format function being something very specific, since the results are all subject to changes in locale data and information and the implementation reserves all rights to adapt them at any point of time. This is formatted locale information that should only be used to display to users.

💯 — can you comment on #42440 also? I'm concerned about the direction of regression tests assuming format outputs.

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

8 participants