temporalio
[DX] Unexpected behaviour with lax DateInterval string parsing
This is the intended behaviour, but I definitely think this needs calling out in the external docs. I have read the documentation pretty well, and this surprised me.
When strings are passed to methods that expect a DateIntervalValue, strings are parsed using CarbonInterval::fromString (vs DateInterval::__construct). This can result in very surprising behaviour, especially because CarbonInterval::fromString is very loose with its parsing, and will silently drop some parts rather than erroring.
For example
yield Workflow::timer('P2M')
will result in a timer of 2 minutes, rather than the 2 months as you might expect as in PHP's DateInterval. You can use DateInterval-style strings, and it will work most of the time
/** Identical results: */
yield Workflow::timer('PT2M') // 2 minutes
yield Workflow::timer(new \DateInterval('PT2M')) // 2 minutes
yield Workflow::timer('P1Y') // 1 year
yield Workflow::timer(new \DateInterval('P1Y')) // 1 year
/** Different results with no errors: */
yield Workflow::timer('P2M') // 2 minutes
yield Workflow::timer(new \DateInterval('P2M')) // 2 months
yield Workflow::timer('PT1H5M') // 1 hour
yield Workflow::timer(new \DateInterval('PT1H5M')) // 1 hour 5 minutes
Is there any scope to change the type to be more explicit or restrictive? (though this would be a breaking change)
his needs calling out in the external docs
Makes sense.
Is there any scope to change the type to be more explicit or restrictive? (though this would be a breaking change)
This does indeed look like a breaking change. However, I would consider it more of a bug fix. I think we should explore an alternative option: issuing a warning when a string interval can be interpreted in different ways.
