Fix options passed to date-fns/parse #4474

emilecantin · 2024-01-26T20:28:41Z

To provide symmetry with date formatting, also use the special unicode tokens flags on date parsing.

To provide symmetry with date formatting, also use the special unicode tokens flags on date parsing. See Hacker0x01#4469

pullrequest

✅ This pull request was sent to the PullRequest network.

@emilecantin you can click here to see the review status or cancel the code review job.

pullrequest

PullRequest Breakdown

Reviewable lines of change

+ 11
- 2

100% JavaScript

Type of change

Fix - These changes are likely to be fixing a bug or issue.

codecov · 2024-01-26T20:29:57Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (06ac055) 95.47% compared to head (562b238) 95.47%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4474   +/-   ##
=======================================
  Coverage   95.47%   95.47%           
=======================================
  Files          29       29           
  Lines        2518     2518           
  Branches     1034     1034           
=======================================
  Hits         2404     2404           
  Misses        114      114

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pullrequest

Code looks great, one minor opportunity involving code reuse

Tyler A

Reviewed with ❤️ by PullRequest

pullrequest · 2024-01-26T20:31:25Z

src/date_utils.js

@@ -95,7 +97,11 @@ export function parseDate(value, dateFormat, locale, strictParsing, minDate) {
    return parsedDate;
  }

-  parsedDate = parse(value, dateFormat, new Date(), { locale: localeObject });
+  parsedDate = parse(value, dateFormat, new Date(), {
+    locale: localeObject,


If you'd like, and it's not a big deal, we could refactor this object into a const and use it everywhere
https://en.wikipedia.org/wiki/Rule_of_three_(computer_programming)

🔹 Code Reuse (Nice to have)

Tyler A

Yeah, it's just that sometimes it gets joined with the locale, and sometimes not. We'd have to spread it everywhere ({...object} syntax), which I'm not sure is an improvement. If you think it is, I'll be happy to refactor it that way.

That's alright, I think this was the right tradeoff.

Tyler A

pullrequest

This change looks good, no particular concern.

Jacques

Reviewed with ❤️ by PullRequest

pullrequest · 2024-01-26T21:37:24Z

src/date_utils.js

@@ -82,6 +82,8 @@ export function parseDate(value, dateFormat, locale, strictParsing, minDate) {
    dateFormat.forEach((df) => {
      let tryParseDate = parse(value, df, new Date(), {
        locale: localeObject,
+        useAdditionalWeekYearTokens: true,


Just pointing out the discrepancy with the option below that includes an Of in DayOfYear and not this one, in case that is not intended.

🔹 Style Consistency (Nice to have)

Jacques

These come from date-fns, that's how the options are named.

👍

Jacques

emilecantin · 2024-01-29T21:00:37Z

@martijnrusschen Is it possible to do another release with these changes?

If you were planning on a release this week, I can wait, but this PR does fix a crash that's happening in our app with some specific dateformats that we want to use in our next release.

Thanks!

Fix options passed to date-fns/parse

562b238

To provide symmetry with date formatting, also use the special unicode tokens flags on date parsing. See Hacker0x01#4469

pullrequest bot reviewed Jan 26, 2024

View reviewed changes

martijnrusschen merged commit af0049b into Hacker0x01:main Jan 27, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix options passed to date-fns/parse #4474

Fix options passed to date-fns/parse #4474

emilecantin commented Jan 26, 2024

pullrequest bot left a comment

pullrequest bot left a comment

codecov bot commented Jan 26, 2024

pullrequest bot left a comment

pullrequest bot Jan 26, 2024

emilecantin Jan 26, 2024

pullrequest bot Jan 26, 2024

pullrequest bot left a comment

pullrequest bot Jan 26, 2024

emilecantin Jan 27, 2024

pullrequest bot Jan 27, 2024

emilecantin commented Jan 29, 2024

Fix options passed to date-fns/parse #4474

Fix options passed to date-fns/parse #4474

Conversation

emilecantin commented Jan 26, 2024

pullrequest bot left a comment

Choose a reason for hiding this comment

pullrequest bot left a comment

Choose a reason for hiding this comment

PullRequest Breakdown

codecov bot commented Jan 26, 2024

Codecov Report

pullrequest bot left a comment

Choose a reason for hiding this comment

pullrequest bot Jan 26, 2024

Choose a reason for hiding this comment

🔹 Code Reuse (Nice to have)

emilecantin Jan 26, 2024

Choose a reason for hiding this comment

pullrequest bot Jan 26, 2024

Choose a reason for hiding this comment

pullrequest bot left a comment

Choose a reason for hiding this comment

pullrequest bot Jan 26, 2024

Choose a reason for hiding this comment

🔹 Style Consistency (Nice to have)

emilecantin Jan 27, 2024

Choose a reason for hiding this comment

pullrequest bot Jan 27, 2024

Choose a reason for hiding this comment

emilecantin commented Jan 29, 2024