iamkun / iamkun/dayjs

diff(..., 'year'|'month', true) returns 0 instead of NaN for an Invalid Date operand

Open Beginner friendly
#3,186 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.