fix: narrow type for add and subtract

[susiwen8]

Close #1575

[addisonElliott]

Your fix removes the UnitTypeLongPlural type. So any pluralized types would return errors now. Looking at the code, it also applies to diff() function. Not only that, this doesn't remove the dates plural type.

However, I wonder if it would be better to add support for date in add/subtract/diff. It would be relatively easy and make it consistent with startOf, endOf functions.

FYI, if you use dayjs.add(1, 'date'), it will add 1ms to the date. See here: https://github.com/iamkun/dayjs/blob/dev/src/index.js#L243

If the date keyword is not going to be supported for add/subtract/, I recommend just omitting the date & dates types from the OpUnitType itself.

type OpUnitType2 = Omit<OpUnitType, 'date' | 'dates'>;

[susiwen8]

@addisonElliott Thanks,

So any pluralized types would return errors now.

Indeed pluralized type should be included.

if you use dayjs.add(1, 'date'), it will add 1ms to the date

You are right, I tried myself, date act like ms

I wonder if it would be better to add support for date in add/subtract/diff

Agree, I was going to fix this way. But I thought this might be intentional or someting that left date behind

[codecov]

Codecov Report

Merging #1576 (bf7c478) into dev (5a79cc6) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               dev     #1576   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          177       177           
  Lines         1989      1989           
  Branches       505       505           
=========================================
  Hits          1989      1989           

---

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 5a79cc6...bf7c478. Read the comment docs.

[iamkun]

Yes, date should not be supported in dayjs.add, cause add a date make no sense

[susiwen8]

@iamkun should I reopen this PR?

[iamkun]

why not 😁

[susiwen8]

@iamkun please review 😁

[iamkun]

BTW, is any special reason to close it?

[susiwen8]

Just thought you might not interested in this pr.

[iamkun]

@addisonElliott according to moment.js date is not support as well.

let's just omit it

https://momentjs.com/docs/#/manipulating/add/

[iamkun]

🎉 This PR is included in version 1.10.7 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀