diff(..., 'year'|'month', true) returns 0 instead of NaN for an Invalid Date operand
- Dominant language
- JavaScript
- Stars
- 48.7k
- Forks
- 2.5k
- PR merge metrics
- No merged PRs in 30d
Description
**Describe the bug**
`monthDiff` in `src/utils.js` silently swallows `NaN` into `0` via its final `|| 0`:
```js
const monthDiff = (a, b) => {
if (a.date() < b.date()) return -monthDiff(b, a)
const wholeMonthDiff = ((b.year() - a.year()) * 12) + (b.month() - a.month())
const anchor = a.clone().add(wholeMonthDiff, C.M)
const c = b - anchor < 0
const anchor2 = a.clone().add(wholeMonthDiff + (c ? -1 : 1), C.M)
return +(-(wholeMonthDiff + ((b - anchor) / (c ? (anchor - anchor2) :
(anchor2 - anchor)))) || 0)
}
```
This is used by `.diff(other, 'year'|'month', true)`. When either side of the diff is an Invalid Date, the intermediate arithmetic naturally produces `NaN`, but the `|| 0` at the end turns that into a plausible-looking `0` instead of propagating the invalidity -- so `.diff()` silently reports "0 months apart" for a comparison that is actually meaningless.
**Reproduction**
```js
const dayjs = require('dayjs');
const invalid = dayjs('not a date');
const now = dayjs();
invalid.diff(now, 'month', true); // 0 -- should be NaN
invalid.diff(now, 'year', true); // 0 -- should be NaN
// Compare with day/hour/etc units, which use raw ms diff and correctly propagate NaN:
invalid.diff(now, 'day', true); // NaN (correct)
invalid.diff(now, 'hour', true); // NaN (correct)
```
**Expected behavior**
`.diff(..., 'year'|'month', true)` should return `NaN` when either operand is an Invalid Date, consistent with every other unit (`day`, `hour`, `minute`, `second`, `week`, `quarter` all correctly return `NaN` already, since they compute from the raw millisecond `diff = this - that` without an `|| 0` fallback).
**How I found this**
While looking at #3006 / PR #3024 (`duration.humanize()` returning `"a month"` for `dayjs.duration(NaN)`) -- that bug's actual root cause is exactly this `monthDiff` fallback (`humanize()` ends up diffing an Invalid-Date-wrapping instant against "now" in Y/M units, and `monthDiff`'s `|| 0` turns the resulting `NaN` into `0`, which the `relativeTime` threshold loop then reads as "0 months" and prints `"a month"`). PR #3024 already fixes the `humanize()` symptom directly at that boundary, so this issue is just the underlying `monthDiff` behavior itself, which can still surface through any other code path that calls `.diff(..., 'year'|'month')` with an invalid operand (not just via `duration`).
**Information**
- Day.js Version: 1.11.22 (current `dev`)
Contributor guide
Research direction
Start in src/utils.js at monthDiff and reproduce the invalid-date cases from the issue through the public diff(..., 'month', true) and diff(..., 'year', true) entry points. Verify that invalid operands produce NaN while valid month and year differences remain unchanged; add or update regression coverage wherever the existing diff tests are located.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- api
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 84/100