Skip to content

Conversation

@yanaemon
Copy link

Describe the bug

On isBetween plugin with invalid date, inclusive returns true, but exclusive returns false.

> dayjs('2021-01-01').isBetween(null, null, 'date', '[]')
true
> dayjs('2021-01-01').isBetween(null, null, 'date', '()')
false

Expected behavior

This behavior is very strange compared to moments (return false if either argument is invalid date).

> moment('2021-01-01').isBetween(null, null, 'date', '[]')
false
> moment('2021-01-01').isBetween(null, null, 'date', '()')
false
> moment('2021-01-01').isBetween('2020-01-01', null, 'date', '[]')
false
> moment('2021-01-01').isBetween(null, '2022-01-01', 'date', '[]')
false

This is because !d.isBefore/!d.isAfter for invalid date always returns true.
I think it would be better to make this behavior the same as moment.js.

@codecov
Copy link

codecov bot commented Jul 29, 2021

Codecov Report

Merging #1578 (9ede212) into dev (b5e40e6) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               dev     #1578   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          177       177           
  Lines         1986      1991    +5     
  Branches       503       506    +3     
=========================================
+ Hits          1986      1991    +5     
Impacted Files Coverage Δ
src/plugin/isBetween/index.js 100.00% <100.00%> (ø)
src/plugin/advancedFormat/index.js 100.00% <0.00%> (ø)
src/plugin/customParseFormat/index.js 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5e40e6...9ede212. Read the comment docs.

@keidarcy
Copy link

keidarcy commented Aug 23, 2021

@iamkun
What about the status of this issue.
Looks like a pretty necessary feature :)

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 this pull request may close these issues.

2 participants