-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
assert,util: handle invalid dates as equal in deep comparison #57627
base: main
Are you sure you want to change the base?
Conversation
Review requested:
|
Invalid dates are now handled as equal in all deep comparisons.
dcf51d3
to
f83b6c4
Compare
@@ -22,6 +22,7 @@ const primValues = { | |||
'empty_object': {}, | |||
'regexp': /abc/i, | |||
'date': new Date(), | |||
'invalidDate': new Date('foo'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if we want to add the entries to our benchmarks yet, since this would just always fail when comparing it with older versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other option is to wait a release to include, but I don't thing is worthy, so I would keep it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what would have thrown before when comparing two invalid dates? it seems like comparing two invalid dates would have compared as false before, and now will compare as true, which does seem breaking.
|
ohhh right, because it's an |
@@ -22,6 +22,7 @@ const primValues = { | |||
'empty_object': {}, | |||
'regexp': /abc/i, | |||
'date': new Date(), | |||
'invalidDate': new Date('foo'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other option is to wait a release to include, but I don't thing is worthy, so I would keep it
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #57627 +/- ##
==========================================
- Coverage 90.24% 90.22% -0.02%
==========================================
Files 630 630
Lines 185129 185060 -69
Branches 36238 36223 -15
==========================================
- Hits 167064 166967 -97
- Misses 11045 11046 +1
- Partials 7020 7047 +27
🚀 New features to boost your workflow:
|
Invalid dates are now handled as equal in all deep comparisons.
This is more of a usability improvement than a breaking change, since all invalid dates have always thrown errors when being compared until now.