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

Make differenceInBusinessDays return NaN instead of Invalid Date #2022

Closed
wants to merge 1 commit into from

Conversation

dmarcr1997
Copy link

Saw there were already tests in place for this issue. I got them to pass by changing the return from new Date(NaN), which returns invalid date, to NaN.

@kossnocorp kossnocorp added this to the v3 milestone Oct 27, 2020
@kossnocorp kossnocorp changed the title fixed issue with Date returning instead of NaN Make differenceInBusinessDays return NaN instead of Invalid Date Oct 27, 2020
Copy link
Member

@kossnocorp kossnocorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the change, but I will not merge it right now as it introduces a breaking change. However, it will be handy when I start working on v3. Tests actually check for Invalid Date, which internally represented by NaN, so the PR didn't fix tests 😉

"name": "date-fns",
"version": "DON'T CHANGE; IT'S SET AUTOMATICALLY DURING DEPLOYMENT; ALSO, USE YARN FOR DEVELOPMENT",
"lockfileVersion": 1
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the file from the PR.

Comment on lines +44 to +46
// fix for issue # 2012
// returning undefined to fix issue of returning a Date when either date is invalid
if (!isValid(dateLeft) || !isValid(dateRight)) return NaN
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for the comment.

Suggested change
// fix for issue # 2012
// returning undefined to fix issue of returning a Date when either date is invalid
if (!isValid(dateLeft) || !isValid(dateRight)) return NaN
if (!isValid(dateLeft) || !isValid(dateRight)) return NaN

@fturmel
Copy link
Member

fturmel commented Jan 30, 2022

Going to close this PR if you don't mind. This got fixed and merged when the function was converted to TypeScript (perhaps early by accident), and it's been in v2 releases for a while. See #2012 and #2414

@fturmel fturmel closed this Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants