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

DateTime.fromFormat fails in some cases when running Node 18.13.0 #1364

Closed
jnn-incom opened this issue Jan 19, 2023 · 6 comments · Fixed by #1369
Closed

DateTime.fromFormat fails in some cases when running Node 18.13.0 #1364

jnn-incom opened this issue Jan 19, 2023 · 6 comments · Fixed by #1369

Comments

@jnn-incom
Copy link

Describe the bug
I Node 18.13.0 parsing DateTime using the fromFormat method can in some cases fail parsing, where running in previous Node version it would not.

To Reproduce

const { DateTime } = require("luxon");

const format = "M/d/yyyy tt";
const dateTimeString = "1/17/2023 11:00:00 PM";

const dateTime = DateTime.fromFormat(dateTimeString, format);

console.log(dateTime.isValid);
// "false" when running in 18.13.0
// "true" when running in 18.12.0

console.log(dateTime.toISO());
// "null" when running in 18.13.0
// "2023-01-17T23:00:00.000+01:00" when running in 18.12.0

Actual vs Expected behavior
I would expect the outcome of running the snippet to be the same in 18.12.0 and 18.13.0 and that the parsed DateTime is valid.
Instead in 18.12.0 the DateTime is correctly parsed while in 18.13.0 it is invalid.

Desktop (please complete the following information):

  • OS: Windows 11 (but originally noticed noticed in debian based docker container)
  • Browser: Node 18.13.0
  • Luxon version: 3.2.1
  • Your timezone: Europe/Copenhagen

Additional context
Add any other context about the problem here.

@damisv
Copy link

damisv commented Jan 22, 2023

Hello 👋 !
I've encountered the same issue and I've noticed that the locale is not passed anymore if defined on options. Also it seems the default to be set to "en-GB".
For example:

DateTime.fromFormat("10:00 pm", "t", { locale: "en-US" })

// ------ Node v18.12 ------
DateTime {
  ....
  loc: Locale {
    locale: 'en-US',
    numberingSystem: null,
    outputCalendar: null,
    intl: 'en-US',
    weekdaysCache: { format: {}, standalone: {} },
    monthsCache: { format: {}, standalone: {} },
    meridiemCache: null,
    eraCache: {},
    specifiedLocale: 'en-US',
    fastNumbersCached: null
  },
  invalid: null,
  weekData: null,
  c: {
    year: 2023,
    month: 1,
    day: 22,
    hour: 22,
    minute: 0,
    second: 0,
    millisecond: 0
  },
  o: 60,
  isLuxonDateTime: true
}

// ------ Node v18.13 ------
DateTime {
  ....
  loc: Locale {
    locale: 'en-GB',
    numberingSystem: null,
    outputCalendar: null,
    intl: 'en-GB',
    weekdaysCache: { format: {}, standalone: {} },
    monthsCache: { format: {}, standalone: {} },
    meridiemCache: null,
    eraCache: {},
    specifiedLocale: null,
    fastNumbersCached: null
  },
  invalid: Invalid {
    reason: 'unparsable',
    explanation: `the input "10:00 pm" can't be parsed as format t`
  },
  weekData: null,
  c: null,
  o: null,
  isLuxonDateTime: true
}

@mohd-akram
Copy link
Contributor

The issue is caused by an update to ICU in Node.js. See nodejs/node#46123. Hopefully this can be fixed on Luxon's side as parsing is currently broken because of it.

@thatsmydoing
Copy link
Contributor

Note that the ICU is not inherently tied to the nodejs version. That only applies to official builds. If you (or your distribution) builds an older nodejs against the new ICU, the changes noted here still apply (like having non breaking spaces in certain places).

@Evan-McDonagh
Copy link

This issue has cropped up running on Chrome now as well since updating the browser to version 110.

@pgaspar
Copy link

pgaspar commented Feb 16, 2023

This issue has cropped up running on Chrome now as well since updating the browser to version 110.

Also seeing this on Chrome 110. Running fromFormatExplain("12:00 AM", "t") for example, returns a regex with a weird unicode character instead of a blank space:

/^(\d{1,2})(:)(\d{1,2})(\ )(AM|PM)$/i

This returns no matches. In Safari, the regex has a normal blank space character and works well.

(We're using v1.28.1)

@gBusato
Copy link

gBusato commented Feb 26, 2023

This issue has cropped up running on Chrome now as well since updating the browser to version 110.

Also seeing this on Chrome 110. Running fromFormatExplain("12:00 AM", "t") for example, returns a regex with a weird unicode character instead of a blank space:

/^(\d{1,2})(:)(\d{1,2})(\ )(AM|PM)$/i

This returns no matches. In Safari, the regex has a normal blank space character and works well.

(We're using v1.28.1)

True, I'm currently experiencing the same issue with node 18.10.0 and Chrome v110

I have found a temporary solution while waiting for a fix :

"9:23 AM".replace(/\s/g, '\u202F')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants