laravel / ideas

Issues board used for Laravel internals discussions.
938 stars 28 forks source link

PSR-20 Clock #2578

Open kylekatarnls opened 3 years ago

kylekatarnls commented 3 years ago

Hello 👋

You may have heard about the draft PSR-20: Clock: https://github.com/php-fig/fig-standards/blob/master/proposed/clock.md

It aims to:

\Carbon\FactoryImmutable will implement this interface in a next major version (3 or 4, it will depend when this draft and Carbon get released).

What it means for Laravel

It sounds to me like it would benefit to Laravel to comply this PSR. But this PSR expect the clock to return DateTimeImmutable object, this would match CarbonImmutable extending it but not Carbon class extending DateTime.

Behavior difference is:

mutable: add/sub/set/etc. modify and return the current object

$date = Carbon::parse('2021-01-01');
$date->addMonth()->addDays(10);
// $date is 2021-02-11

immutable: add/sub/set/etc. return a new object

$date1 = CarbonImmutable::parse('2021-01-01');
$date2 = $date1->addMonth()->addDays(10);
// $date1 is still 2021-01-01
// $date2 is still 2021-02-11

So switching to CarbonImmutable (as the default for Date facade) would be a breaking change for users that may be not so easy to migrate.

A possible non-breaking implementation would be to use a middle factory that would get $now from FactoryImmutable->now() then basically returns: $now->toMutable() so date objects would still be Carbon instances by default but the moment itself would be generated by a FactoryImmutable which would comply PSR-20. So it would still benefit the interoperability and mocking of it.

Still if PSR-20 promotes immutable objects it's because it's particularly relevant for objects supposed to represent moments. Semantically when adding 3 days to January 1st you don't make January 1st happen later, you get a new date as the result of the addition of date + duration. This is what makes more accurate to replace the date object (of an event or any) instead of mutating it.

Regarding this, I would be favorable that Laravel would also switch to CarbonImmutable when/if switching to nesbot/carbon 3. Carbon 3 will come with few breaking changes in the API, gap will be pretty equivalent to what happened between 1 and 2.

I will plan dev accordingly to which ones of the suggestions above you would agree. Then if you want, I would be glad to take care of the PR for the changes required in laravel/framework.