fix: Duration.constructor()/toISOString()/add()/subtract() does not work properly

[chimerast]

Fixes #1515 Fixes #1516

This PR is fixing two issues.

• Duration.constructor()/toISOString() does not work properly with negative value ISO String
• Duration.add()/subtract() does not work properly with adding dayjs.duration(1, 'months')

Sorry for fixing two issues in one pull request and putting them together.

fix: Duration.constructor()/toISOString() does not work properly with negative value ISO String

#1516

Negative value representations are not part of the ISO-8601 standard. However, I would like to align it with Java's implementation of the Duration class.

  it('Part ISO string mixed positive and negative', () => {
    expect(dayjs.duration('P6M4D').toISOString()).toBe('P6M4D')
    expect(dayjs.duration('-P6M-4D').toISOString()).toBe('-P6M-4D')
    expect(dayjs.duration('P6M-4D').toISOString()).toBe('P6M-4D')
    expect(dayjs.duration('-P6M4D').toISOString()).toBe('-P6M4D')

    expect(dayjs.duration('PT3H2M').toISOString()).toBe('PT3H2M')
    expect(dayjs.duration('-PT3H-2M').toISOString()).toBe('-PT3H-2M')
    expect(dayjs.duration('PT3H-2M').toISOString()).toBe('PT3H-2M')
    expect(dayjs.duration('-PT3H2M').toISOString()).toBe('-PT3H2M')
  })

  it('object with units of mixed positive and negative', () => {
    expect(dayjs.duration({ months: 6, days: 4 }).toISOString()).toBe('P6M4D')
    expect(dayjs.duration({ months: -6, days: 4 }).toISOString()).toBe('-P6M-4D')
    expect(dayjs.duration({ months: 6, days: -4 }).toISOString()).toBe('P6M-4D')
    expect(dayjs.duration({ months: -6, days: -4 }).toISOString()).toBe('-P6M4D')

    expect(dayjs.duration({ hours: 3, minutes: 2 }).toISOString()).toBe('PT3H2M')
    expect(dayjs.duration({ hours: -3, minutes: 2 }).toISOString()).toBe('-PT3H-2M')
    expect(dayjs.duration({ hours: 3, minutes: -2 }).toISOString()).toBe('PT3H-2M')
    expect(dayjs.duration({ hours: -3, minutes: -2 }).toISOString()).toBe('-PT3H2M')
  })

To fix this issue, I tweaked the result of parsing strings and numbers into their internal representation.

Before

> dayjs.duration('P2D')
Duration {
  "$d": Object {
    "days": 2,
    "hours": NaN,
    "minutes": NaN,
    "months": NaN,
    "seconds": NaN,
    "weeks": NaN,
    "years": NaN,
  },
  "$l": "en",
  "$ms": 172800000,
}

After

> dayjs.duration('P2D')
Duration {
  "$d": Object {
    "days": 2,
  },
  "$l": "en",
  "$ms": 172800000,
}

fix: Duration.add()/subtract() does not work properly when adding dayjs.duration(1, 'months')

#1515

  it('Add days', () => {
    const a = dayjs('2020-10-01')
    expect(a.add(dayjs.duration(2, 'days')).format('YYYY-MM-DD')).toBe('2020-10-03')
    expect(a.add(dayjs.duration(29, 'days')).format('YYYY-MM-DD')).toBe('2020-10-30')
    expect(a.add(dayjs.duration(30, 'days')).format('YYYY-MM-DD')).toBe('2020-10-31')
    expect(a.add(dayjs.duration(31, 'days')).format('YYYY-MM-DD')).toBe('2020-11-01')
  })

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (06f88f4) to head (d183ce6). Report is 201 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff            @@
##               dev     #1513   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          176       176           
  Lines         1965      2013   +48     
  Branches       497       512   +15     
=========================================
+ Hits          1965      2013   +48     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[chimerast]

I will check why decreased the coverage.

[chimerast]

I found I enbugged another form of #1515 caused by tweaking internal representation in this PR. dayjs('2020-10-01').add(dayjs.duration(30, 'days')) returns dayjs('2020-11-01')

I already fixed it at 09cd2dc.

[iamkun]

Thanks, I'll need some time to review this cool PR

[chuckwondo]

Any chance of getting this approved and merged? I'm running into this issue as well when attempting to add 1 month (as a duration) to a date. It always adds 30 days, which is not what I expect. I expect the same day in the following month, regardless of how many days away that is from the starting date.

[Tharabas]

Is there any reason this has not been merged, yet? That +1 month being 30 days is really a dealbreaker when using dayjs 😞

[longlivedigital]

@iamkun if i update this can we get it reviewed?