iamkun / iamkun/dayjs

Adding a 1000 ms duration doesn't change the timestamp

Open
#2,605 0 comments 1 reaction 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**
- Adding durations of >1000 milliseconds only changes the timestamp by % 1000 ms

**Expected behavior**
```js
import dayjs from 'dayjs';
import Duration from 'dayjs/plugin/duration';

dayjs.extend(Duration);

const duration = dayjs.duration({milliseconds: 1000});
const timestamp = dayjs('2021-01-01T00:00:00Z');

const timestampAfterDuration = timestamp.add(duration); // nothing added
const isAfterDuration = timestampAfterDuration.isAfter(timestamp); // false

const timestampAfterMillis = timestamp.add(1000, 'millisecond'); // 1s added
const isAfterMillis = timestampAfterMillis.isAfter(timestamp); // true
```

`isAfterDuration` should be `true`
`timestampAfterDuration` should be at the same time as `timestampAfterMillis`

**Information**
- Day.js Version 1.11.10 though I suspect it's broken since 1.11.9
- OS: macOS 14.4 (23E214)
- ~Browser~ node v20.11.0
- Time zone: GMT +1, CET, Berlin

I suspect it broke [with this PR](https://github.com/iamkun/dayjs/pull/2337/files#diff-ab4244f985fbde8ce9d0f5a09abd5ed28c9ff797beb05c377005a70734983176R282-R283) because with
```js
const duration = dayjs.duration({milliseconds: 1000});
duration.years() // 0
duration.months() // 0
duration.days() // 0
duration.hours() // 0
duration.minutes() // 0
duration.seconds() // 0
duration.milliseconds() // 0, see https://github.com/iamkun/dayjs/blame/dev/src/plugin/duration/index.js#L196
```
and
```js
const manipulateDuration = (date, duration, k) =>
date.add(duration.years() * k, 'y')
.add(duration.months() * k, 'M')
.add(duration.days() * k, 'd')
.add(duration.hours() * k, 'h')
.add(duration.minutes() * k, 'm')
.add(duration.seconds() * k, 's')
.add(duration.milliseconds() * k, 'ms')
```
it ends up adding nothing

Contributor guide

Open the contributing guide

Research direction

Start in src/plugin/duration/index.js, especially the duration accessors and manipulateDuration path referenced in the issue and linked PR. Reproduce the provided Node example, then verify that adding a 1000-millisecond duration changes the timestamp exactly like adding 1000 milliseconds directly.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
tooling
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.