Return ChronosInterval from Chronos::diff()#515
Conversation
|
For downstream users: Anyone using $chronos->diff() with type hints will need updates: // If they have:
function process(DateInterval $interval) { ... }
// They need:
function process(DateInterval|ChronosInterval $interval) { ... }
// Or:
$date->diff($other)->toNative() // to get DateInterval CI StatusAll 954 tests pass. The CI failures are due to:
Both issues are expected for this BC break. We will need to decide how to handle:
|
This changes Chronos::diff() and ChronosDate::diff() to return ChronosInterval instead of DateInterval. This provides: - ISO 8601 duration formatting via __toString() - Convenience methods like totalSeconds(), totalDays(), isZero(), isNegative() - Arithmetic methods add(), sub() - toDateString() for strtotime-compatible output - toNative() for backwards compatibility when DateInterval is needed Users who need a DateInterval can call ->toNative() on the result. Also updates fromNow() to return ChronosInterval. Closes #511
Suppress PHP deprecation notice and PHPStan errors for the intentional return type change from DateInterval to ChronosInterval.
15c25e2 to
556c707
Compare
|
Docs are missing, other than that, it looks good. |
|
Done. |
jamisonbryant
left a comment
There was a problem hiding this comment.
Looks good to me. Docs look good too. Should we add a “since” admonition like the book does or is the migration guide sufficient?
Summary
Chronos::diff()andChronosDate::diff()to returnChronosIntervalinstead ofDateIntervalChronosIntervalclass (merged from 3.x branch)fromNow()to returnChronosIntervalThis provides:
__toString()totalSeconds(),totalDays(),isZero(),isNegative()add(),sub()toDateString()for strtotime-compatible outputtoNative()for backwards compatibility when aDateIntervalis neededMigration Path
Users who need a
DateIntervalcan call->toNative()on the result:Note
Good news: No changes required in CakePHP core!
PHPStan reports an error about covariant return types - this is expected for this BC break. The baseline will need to be updated.
Closes #511