moment / luxon

⏱ A library for working with dates and times in JS
https://moment.github.io/luxon
MIT License
15.05k stars 730 forks source link

Fix/12 hour format validation #1631

Open Sho-ki opened 1 month ago

Sho-ki commented 1 month ago

Fix issue: https://github.com/moment/luxon/issues/1625

Description of the Bug

The DateTime.fromFormat() function incorrectly validates certain 24-hour formats as valid when they should be considered invalid. When using the 12-hour (h) format in the format parameter, the function does not properly invalidate times where the hour exceeds 12.

Actual vs Expected Behavior

DateTime.fromFormat('18:30 AM', 'h:mm a').isValid;

Actual: isValid returns true for DateTime.fromFormat('18:30 AM', 'h:mm a'). Expected: isValid shouldn't be true because 18 is not a valid hour for the 12-hour (h) format.

Fix Description

This PR addresses the issue by ensuring that when the 12-hour (h) format is used in the format parameter, any hour value greater than 12 is considered invalid, and isValid will throw error.

linux-foundation-easycla[bot] commented 1 month ago

CLA Signed


The committers listed above are authorized under a signed CLA.

diesieben07 commented 1 month ago

While this is a valid bugfix, I am reluctant to merge this in and release it as a minor update, because it is technically a breaking change. Normally that would be fine for a bug fix, but I can imagine many people intentionally or unintentionally relying on this behavior. Maybe we should add a strictHours option for backwards compatibility?

What do you think, @icambron?

jw642459986 commented 1 week ago

@diesieben07 are you suggesting something like this? DateTime.fromFormat('18:30 AM', 'h:mm a').isValid('strictHours': true);