fix: duration.minutes() should guaranteed to be in [0, 60)

[KronosDev-Pro]

fix #2329

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (a4ff1d2) 100.00% compared to head (9019ba7) 100.00%.

❗ Current head 9019ba7 differs from pull request most recent head 0d86dd6. Consider uploading reports for the commit 0d86dd6 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##               dev     #2338    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          183       183            
  Lines         2249      2142   -107     
  Branches       636       566    -70     
==========================================
- Hits          2249      2142   -107     

Files Changed	Coverage Δ
src/plugin/duration/index.js	100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[iamkun]

after careful thought, the main issue leading to this bug is not related to the display get getter, but the setter and the constructor.

dayjs.duration({ minutes: 359 }).toJSON() should return "PT5H59M" rather than ""PT359M""

that is to say, you can try to move your calc logic to https://github.com/iamkun/dayjs/blob/dev/src/plugin/duration/index.js#L78

that will make the getter and setter both correct.

ps: IMHO, you have made a nice refactor to the test cases. However, it will be better to keep all the existing test cases to make the lib stronger. Any new PRs should only add new cases or modify the error cases. 😀

[iamkun]

after checking the PR again, still I think, we should update the input part (setter and the constructor) to make the inner data correct, not in the output part

What do you think then?