Skip to content

Error Date.toLocaleString() in node 19.0.0 #45171

Closed
#45573
Closed
Error Date.toLocaleString() in node 19.0.0#45171
#45573
Labels
confirmed-bugIssues and PRs for confirmed bugs.i18n-apiIssues and PRs related to Node.js internationalization support.v8 engineIssues and PRs related to the V8 dependency.

Description

@dmitriym09
dmitriym09
opened on Oct 25, 2022

Version

19.0.0

Platform

Linux 6.0.2-zen1-1-zen #1 ZEN SMP PREEMPT_DYNAMIC Sat, 15 Oct 2022 14:00:51 +0000 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

node

> new Date(new Date().toLocaleString('en-US'))
Invalid Date
> const s = new Date().toLocaleString('en-US')
undefined
> s.at(-3)
' '
> s.at(-3).charCodeAt()
8239
> ' '.charCodeAt()
32
>

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

Stability

What is the expected behavior?

No response

What do you see instead?

Error Date.toLocaleString(): NARROW NO-BREAK SPACE (U+202f) without last whitespace

Additional information

Thanks for node) It`s cool)

Activity

  1. richardlau commented on Oct 25, 2022

    @richardlau
    richardlau
    on Oct 25, 2022
    Member

    NARROW NO-BREAK SPACE sounds like it's from the CLDR 42 change in ICU 72.1
    https://icu.unicode.org/download/72

    In many formatting patterns, ASCII spaces are replaced with Unicode spaces (e.g., a "thin space").

    except Node.js 19.0.0 doesn't ship with ICU 72.1 as that only landed on main today (#45068). Did you build Node.js yourself?

  2. richardlau
    added
    i18n-apiIssues and PRs related to Node.js internationalization support.
    on Oct 25, 2022

  3. targos commented on Oct 25, 2022

    @targos
    targos
    on Oct 25, 2022
    Member

    Another possibility is that you are using a Node.js linked against your system version of ICU, and that version was updated to 72.1.

  4. dharesign commented on Nov 15, 2022

    @dharesign
    dharesign
    on Nov 15, 2022
    Contributor

    Looks like a bug in v8. It's set up to handle unicode whitespace, but instead treats them as keywords. I created https://bugs.chromium.org/p/v8/issues/detail?id=13490

  5. targos
    added
    v8 engineIssues and PRs related to the V8 dependency.
    confirmed-bugIssues and PRs for confirmed bugs.
    on Nov 15, 2022

  6. srl295 commented on Nov 15, 2022

    @srl295
    srl295
    on Nov 15, 2022
    Member

    seems like user error not a bug, don't assume that Localized date format is parseable by Date.

    Looking at the v8 issue, I can see where there is a coding bug on the V8 side, and I haven't looked at the spec yet.

    But that said, I still disagree that the original example code here should always pass… It makes an invalid assumption about the US date format

    Do not use a localized date format if you expect it to be machine readable!

  7. dharesign commented on Nov 15, 2022

    @dharesign
    dharesign
    on Nov 15, 2022
    Contributor

    The spec is here: https://tc39.es/ecma262/#sec-date.parse

    It specifically says:

    The function first attempts to parse the String according to the format described in Date Time String Format (21.4.1.18), including expanded years. If the String does not conform to that format the function may fall back to any implementation-specific heuristics or implementation-specific date formats.

    and:

    However, the expression

    Date.parse(x.toLocaleString())

    is not required to produce the same Number value as the preceding three expressions and, in general, the value produced by this function is implementation-defined when given any String value that does not conform to the Date Time String Format (21.4.1.18)

    I don't know if there's a more canonical reference for what implementation-defined strings v8 supports other than this comment in its source:
    https://github.com/v8/v8/blob/5fe0aa3bc79c0a9d3ad546b79211f07105f09585/src/date/dateparser-inl.h#L23-L69

    It does seem more of a coincidence that Date.parse(d.toLocaleString("en-US")) works, rather than a guarantee. But given it does work, and v8 specifically intended to delegate to icu for determining whitespace, it seems reasonable to continue the status quo.

    In general though, if you want guaranteed-to-work round-trip date formatting and parsing, then you would want to use the ISO 8601 format given by toISOString().

  8. dharesign commented on Nov 21, 2022

    @dharesign
    dharesign
    on Nov 21, 2022
    Contributor

    This was fixed upstream:

    https://bugs.chromium.org/p/v8/issues/detail?id=13494

  9. srl295 commented on Nov 21, 2022

    @srl295
    srl295
    on Nov 21, 2022
    Member

    This was fixed upstream:

    https://bugs.chromium.org/p/v8/issues/detail?id=13494

    FWIW change LGTM

  10. tukusejssirs commented on Nov 23, 2022

    @tukusejssirs
    tukusejssirs
    on Nov 23, 2022

    I just want to leave a note that new Date(new Date().toLocaleString('en-US')) works as expected int node@19.0.1, but not in node@19.1.0.

  11. targos
    added a commit that references this issue on Nov 23, 2022

    deps: V8: cherry-pick 2ada52cffbff

    afc1587
  12. nodejs-github-bot
    closed this as completedin #45573on Nov 24, 2022
  13. nodejs-github-bot
    added a commit that references this issue on Nov 24, 2022

    deps: V8: cherry-pick 2ada52cffbff

    a972144
  14. targos
    added a commit that references this issue on Nov 24, 2022

    deps: V8: cherry-pick 2ada52cffbff

    4b6ab88
  15. ruyadorno
    added a commit that references this issue on Nov 24, 2022

    deps: V8: cherry-pick 2ada52cffbff

    f63ae52

  16. Hexagon commented on Nov 27, 2022

    @Hexagon
    Hexagon
    on Nov 27, 2022

    This is still a problem in 19.1

    image

  17. michelpromonet commented on Nov 29, 2022

    @michelpromonet
    michelpromonet
    on Nov 29, 2022

    Hi,

    It seems working on 19.0

    echo "console.log(new Date((new Date(1000000000000)).toLocaleString()))" | docker run --rm -i node:19.0

    gives

    2001-09-09T01:46:40.000Z

    But not on latest/19.1

    echo "console.log(new Date((new Date(1000000000000)).toLocaleString()))" | docker run --rm -i node:19.1

    fails with

    Invalid Date

    Then it seems fixed and re-broken ?

  18. dharesign commented on Nov 30, 2022

    @dharesign
    dharesign
    on Nov 30, 2022
    Contributor

    It was never an issue in official 19.0.x releases. It was broken in 19.1.0 when Node switched to ICU 72. It is subsequently fixed in 19.2.0 which cherry-picked the v8 commit that fixes it.

    The original report here of it breaking against 19.0.0 must have been linking against an external ICU which was ICU 72.

  19. stalkerg commented on Dec 28, 2022

    @stalkerg
    stalkerg
    on Dec 28, 2022

    Unfortunately, it's a default way to change the timezone for the date https://stackoverflow.com/questions/10087819/convert-date-to-another-timezone-in-javascript and many libraries use it.
    Do you have a better way?

  20. srl295 commented on Dec 28, 2022

    @srl295
    srl295
    on Dec 28, 2022 via email
    Member

  21. stalkerg commented on Dec 29, 2022

    @stalkerg
    stalkerg
    on Dec 29, 2022

    @srl295 indeed, but I don't know a better way now. A very popular library like dayjs uses it. Will be good to provide a better solution and add it to stack overflow.
    I know it's not directly a nodejs issue but because it's so popular will be better to do something.

  22. danielleadams
    added a commit that references this issue on Jan 3, 2023

    deps: V8: cherry-pick 2ada52cffbff

    6ea555e
  23. kazupon
    added 2 commits that reference this issue on Jan 12, 2023

    fix: disalbe node v18

    e68fdab

    fix: support type inference of Translation, NumberFormat and Datetime…

    3ff8b1e
  24. aruniverse
    added 2 commits that reference this issue on Jan 30, 2023

    hacky fix for https://github.com/nodejs/node/issues/45171\#issuecomme…

    3ffb1ad

    Clean up build pipelines (#4995)

    f17bf71
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    confirmed-bugIssues and PRs for confirmed bugs.i18n-apiIssues and PRs related to Node.js internationalization support.v8 engineIssues and PRs related to the V8 dependency.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions