feat: add parseISODuration

[matinzd]

Related: #3150

[matinzd]

@leshakoss @kossnocorp

[alfaproject]

Is there any hope to get this in?

[matinzd]

Is there any hope to get this in?

I messaged @kossnocorp. It seems that they are waiting for v3.0.0 release so this can be included there if there won't be any bugs.

Hey Matin! Thank you so much for the PR. It looks good at first glance. However, you've sent it to the main branch, which represents v3 that we're working on now. It might take some time before we can release it. I hope you're ok with that.

[alfaproject]

Perhaps we can re-target for v2 instead of main branch?

[kossnocorp]

If you need to use the function, please copy-paste the code to your project locally. It's not worth it for us to rebuild the function for v2.

[alfaproject]

Alright, fair enough

[alex-collibra]

First of all, thanks a lot for this workaround.
It seems like parsing of negative durations doesn't work.

parseISODuration('P-1D');

// results in Invalid format

Is it on purpose? Can this be added to this feature as well? It is supported in libs like luxon or dayjs, so technically it may be possible. Here is an example of luxon and dayjs if you want live example:
https://codesandbox.io/s/dazzling-liskov-4dqzg5?file=/src/App.tsx

[matinzd]

First of all, thanks a lot for this workaround. It seems like parsing of negative durations doesn't work.

parseISODuration('P-1D');

// results in Invalid format

Is it on purpose? Can this be added to this feature as well? It is supported in libs like luxon or dayjs, so technically it may be possible. Here is an example of luxon and dayjs if you want live example: https://codesandbox.io/s/dazzling-liskov-4dqzg5?file=/src/App.tsx

According to the specification all these values are positive and above zero. In my understanding I would assume we should not have a negative day or negative year?

[alex-collibra]

First of all, thanks a lot for this workaround. It seems like parsing of negative durations doesn't work.

parseISODuration('P-1D');

// results in Invalid format

Is it on purpose? Can this be added to this feature as well? It is supported in libs like luxon or dayjs, so technically it may be possible. Here is an example of luxon and dayjs if you want live example: https://codesandbox.io/s/dazzling-liskov-4dqzg5?file=/src/App.tsx

According to the specification all these values are positive and above zero. In my understanding I would assume we should not have a negative day or negative year?

Agree, but it was the same discussion in other libraries (sorry to link to other libs from here, but this is the one of many discussions: moment/moment#2408 (comment)) Don't mean that if others support it, date-fns must support it - just curious about the roadmap.

[cj-christoph-gysin]

Any chance to get the conflicts resolved so we can still get this into v3?

[kossnocorp]

Thank you a lot for your work and waiting! I finally have time to work on new stuff and will be happy to ship it with the next version

[kossnocorp]

Please rebase and reformat code with the default Prettier config, thanks!

[matinzd]

Please rebase and reformat code with the default Prettier config, thanks!

Rebased it now and all suggestions are applied. However I don't know how to test generated docs to make sure if the example works properly. Let me know if that still has an issue.

[matinzd]

I guess we may need to add more test cases as I have heard a lot about negative values. WDYT? @kossnocorp

[kossnocorp]

@matinzd don't worry about the docs. Adding more tests never hurt, if you feel there's something to cover please go ahead.

[andrioid]

What is missing so this can be merged? Anything I can do to help?

[matinzd]

@andrioid
There are some test cases that the current implementation is not covering. I will try to update the PR accordingly.

[yjukaku]

@matinzd thanks so much for taking care of this.

IMO, negative durations shouldn't hold up merging of this feature that has been asked for since 2016 :), especially if negative durations aren't explicitly stated in the standard.

Again my opinion, but If it's needed, it can be a followup PR/release.

[matinzd]

@yjukaku Yes, you are right! The test case I found was not related to supporting negative values. Also negative values are not in the standard as far as I checked.
I really want to be mindful when merging the PR because I don't want people to use a broken function in their codebase especially that date-fns is widely used in the ecosystem.

[ckcherry23]

Hi @matinzd, can you let us know what test cases you think can be added before we merge this in? I can help to contribute as well.

[ckcherry23]

@matinzd I think what we need here is for the entire result object to be formatted as a comment. You can refer to src/intervalToDuration/index.ts for an example.

Suggested change

	* //=>
	* const result = {
	* years: 3,
	* months: 6,
	* weeks: 0,
	* days: 4,
	* hours: 12,
	* minutes: 30,
	* seconds: 5,
	* }
	* //=> { years: 3, months: 6, weeks: 0, days: 4, hours: 12, minutes: 30, seconds: 5 }

[ckcherry23]

Hi @matinzd, maybe we could also add more invalid test cases for this, for example:

• P - only P
• PT - only P and T
• P1H - missing T
• PT1D - unneeded T
• P1W1YT1M - incorrect date order before T
• P1WT1M1H - incorrect time order after T

We could also have:

• P0D - gives zero instead of undefined
• PT0S- gives zero instead of undefined
• PT0.0021S - 0.0021S (instead of 2.1 milliseconds, which could be useful for ExtendedDuration if supported later)