-
Notifications
You must be signed in to change notification settings - Fork 736
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
Inconsistent DateTime format when using toLocaleString #1333
Comments
I've noticed the same sort of thing happening when I run the luxon test
suite on GitHub Code Spaces (ie: visual studio code online on github). I
attributed the weirdness to me having the wrong version of node installed,
but I am not really sure if that is the root cause.
|
@dobon, I just tried out node.js version 16 and 18 as well, but the results were the same. |
I'm speculating, but I've always assumed that this kind of thing is a change in the minor version of Node, across all the supported major versions. They update the ICU data in all those majors and we get string changes in all of them |
@icambron, I went and checked the node versions on the different machines to get some more data: Tests passing for:
I made some changes to the build system to use the current up to date LTS version which is at 18.12.1 and the build system is also now consistently failing on the same tests. I am inclined to believe your hunch is right about something related to Node changing something. What do you think should be done about this? |
I tried out a couple of more tests between node versions.
I tried 18.2.0 because this was the only release in version 18 that have changes related to ICU Locale and I am seeing some bizarre results. All three versions are generating this format: I am not really sure what's going on, but I was expecting 18.2.0 to cause the breakage, but using 18.0.0 is also showing breakages. |
Personally I do not think testing against the output of |
I agree that users of Luxon shouldn't be testing against the output of However, Luxon itself needs to test, at the very least, that the inputs it sends to Intl are right. It doesn't currently have a good way to do that, so we test the output of |
Hey everyone. Tried out my test on Node19 and here's what I got (works well on node 16, 17, 18): In Node 19 space character code is Any ideas on a workaround? Same space issue with both |
@inomn Yeah, this is wild. I ran into the same problem where the space character is different. I took a look at the byte representation and prior to this, the encoded space character was It is related to this: Since these issues are not caused by luxon, I think this issue should be closed. |
The Node behaviour change looks to have been reverted in 18.15: nodejs/node#46646 |
I don't know if this is related to something on my MacBook, but for some reason, my tests started failing with this:
On any other machine, this is the sample formatted output, but should be correct according to the documentation:
May 12, 2022, 7:00 AM PDT
On my Macbook, I would get this instead:
May 12, 2022 at 7:00 AM PDT
The text was updated successfully, but these errors were encountered: