Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

[Proposal] Switch from Carbon to Chronos #362

Closed
it-can opened this issue Jan 7, 2017 · 53 comments
Closed

[Proposal] Switch from Carbon to Chronos #362

it-can opened this issue Jan 7, 2017 · 53 comments

Comments

@it-can
Copy link

it-can commented Jan 7, 2017

It seems that the Carbon project (https://github.com/briannesbitt/Carbon) is not very active anymore. The CakePHP team has forked the Carbon repo and created their own (that they also maintain and use in their own projects), called Chronos (https://github.com/cakephp/chronos).

Chronos aims to be a drop-in replacement for nesbot/carbon. It focuses on providing immutable date/datetime objects. Immutable objects help ensure that datetime objects aren't accidentally modified keeping data more predictable.

These are the differences between Carbon en Chronos

Differences with nesbot/carbon

The biggest and main difference is that Chronos extends DateTimeImmutable instead of DateTime. Immutability for date values has proven to be a great way of avoiding bugs and reduce the amount of code, since developers don't have to manually copy the instance every time they need a change.

Another important feature it offers is the Date class, which is used for representing dates without time (calendar dates). Any time method called on this type of object is basically a no-op.

A minor but still noticeable difference is that Chronos has no external dependencies, it is completely standalone.

Finally, Chronos is faster than Carbon as it has been optimized for the creation of hundreds of instances with minimal overhead.

Chronos also strives for HHVM compatibility, this library can be used safely with HHVM 3.11.
@brunogaspar
Copy link

brunogaspar commented Jan 7, 2017

Probably for Laravel 5.5 since Laravel 5.4 is right around the corner, but i would like to see this change regardless of the Laravel version.

@it-can
Copy link
Author

it-can commented Jan 7, 2017

It is a big change I think... It's sad that the Carbon project is not very active anymore

@michaeldyrynda
Copy link

"Inactive" or "feature complete"?

Either way, immutable dates would be appreciated 👌

@barryvdh
Copy link

barryvdh commented Jan 8, 2017

But would be a big breaking change..

@sebastiandedeyne
Copy link

Maybe revisiting the improved type casting issue would be a better idea so it's opt-in. (I think most of the Carbon use is related to eloquent? Or am I missing a major section?)

#9

@hipsterjazzbo
Copy link

I've suggested this before, and been shot down. I think it's a great idea though.

  1. Chronos seems to be much more maintained than Carbon
  2. It is actually a lot more efficient for those of us that are doing hardcore large scale apps with Laravel
  3. I personally find it a lot easier to reason about immutable dates, but I recognize that's not enough to justify the change in and of itself.

In terms of how big of a change it'd be, I agree it would have to be a 5.5 change. If anyone is interested I'd be happy to have a look at what'd need to be done to make it work.

In terms of people using Carbon in their business code, I feel like it'd be simple to just inform about the change and people can require Carbon\Carbon explicitly if they want.

@Garbee
Copy link

Garbee commented Jan 9, 2017

This would need to be like a 6.0 revision change. Modifying the way you provide dates and times from the DB layer is massive. Apps could rely on these being Carbon instances quite heavily throughout their application. This certainly isn't the level of thing you pawn off to just a regular upgrade.

Chronos seems to be much more maintained than Carbon

Can we define maintained here? What are major issues with Carbon that aren't being addressed? What about carbon (besides Mutability) is deficient?

Personally, I think the move would be good as well as I like immutable objects as well. However I'd like there to be more substance to a change this critical than "a few of us feel it is a good move." What is the actual benefit to Laravel in terms of long-term maintenance? What value is this providing to everyone using Laravel that they didn't have before?

@jbrooksuk
Copy link
Member

If only we'd used the CarbonInterface https://github.com/cakephp/chronos/blob/master/src/carbon_compat.php

@jbrooksuk
Copy link
Member

Actually, even Carbon isn't using that 😂

@lucasmichot
Copy link

Carbon is on its way to version 2.0 and a new release has been created past weekend.

@IonutBajescu
Copy link

IonutBajescu commented Jan 20, 2017

@Garbee

What value is this providing to everyone using Laravel that they didn't have before?

Chronos is supposed to be faster than Carbon and have lower memory consumption. I'm saying "supposed" because I haven't actually benchmarked it myself and blindly trusted their readme, which says:

Finally, Chronos is faster than Carbon as it has been optimized for the creation of hundreds of instances with minimal overhead.

We'll obviously need to benchmark it ourselves though so we can see how relevant these speed changes are.

@tillkruss
Copy link

@it-can: Are you doing a PR for 5.5?

@it-can
Copy link
Author

it-can commented Feb 25, 2017

@tillkruss I am not working on a pr at this moment... seems though that carbon is getting active again...

@DanielDarrenJones
Copy link

Would defo like to see this be added, especially after learning about this "feature" of carbon: briannesbitt/Carbon#923

@lucasmichot
Copy link

This is neither a "feature", neither as bug as this is the way \Datetime works.
Still curious why nobody uses \Carbon\Carbon::addMonthsNoOverflow?

@DanielDarrenJones
Copy link

@lucasmichot hasn't been used because honestly I didn't realise it was a problem, it's not obvious behaviour and in fact quite strongly goes against expected behaviour. Granted the php DateTime object does it that way but that appears to me one of the many missteps and oddities of PHP, I feel like the expected behaviour should be the default while the unexpected behaviour should be pushed back into an opt it.

this is off topic from the discussion at hand, but I'm legitimately concerned about the number of bugs this is likely to have caused across lots of peoples code bases, especially in areas like billing (obviously this should be driven out by testing, but i'm sure you would agree that's unlikely to have happened in most cases).

@lucasmichot
Copy link

lucasmichot commented Jun 29, 2017

This is exactly the reason why \Carbon\Carbon::useMonthsOverflow has been implemented.

while the unexpected behaviour should be pushed back into an opt it

Next version will have \Carbon\Carbon::$monthsOverflowdisabled by default.
You can do it right now.

@lucasmichot
Copy link

\Carbon\Carbon::useMonthsOverflow(false);

Then add/sub-Month/Year(s) functions will work without overflowing
Pretty simple

@joshmanders
Copy link

@lucasmichot

Still curious why nobody uses \Carbon\Carbon::addMonthsNoOverflow?

Never heard of it or thought it would be needed, that could be why... if it wasn't for this thread, I'd have never known, would have just used different paths and work arounds.

@DanielDarrenJones
Copy link

@joshmanders agreed, there is no mention of it in the docs as far as I could tell...

@coquer
Copy link

coquer commented Sep 21, 2017

@lucasmichot has been on the way for over a year.. how much more until we get it?

@gregmartyn
Copy link

The creator of PHP's DateTime recommends DateTimeImmutable: https://derickrethans.nl/immutable-datetime.html

It's a footgun that we keep running into, so an immutable solution would be very welcome.

@carusogabriel
Copy link

Since Carbon and Chronos have defenders, why we don't think a way to support them, or other DateTime package? And yes, as @Garbee said, this should be in a 6.0 version of Laravel, since would rewrite the how the Core generates Date ⚠️

@ikari7789
Copy link

ikari7789 commented Jan 11, 2018

I'm strongly in favor or figuring out a way to support more than Carbon. Carbon hasn't had any update since Feb 7, 2017 (basically an entire year now), and it doesn't seem like neither of the maintainers are coming around any time soon to merge in any of the 74 pull requests or handle any of the 131 issues. It has issues with PHP 7.1+ as it doesn't have proper handling of microseconds in certain use cases, which forces some funky workarounds to ensure they don't cause any issues.

I'm not sure if replacing Carbon directly with Chronos is the best way to handle it or not, but something should be done.

@sisve
Copy link

sisve commented Jan 11, 2018

Wait... are there defenders of Carbon?

$carbon->addDays(1)->subDays(1) results in wrong values outside of UTC, since a year back.
briannesbitt/Carbon#863

Just throw it out. We cannot depend on a datetime library that has these severe bugs for this long. The entire purpose of the library is lost.

Chronos is an obvious replacement. Are there any other wellknown solutions we need to evaluate?

@ikari7789
Copy link

@sisve I believe people are only wanting to keep it around due to how much it could break existing systems, especially in the case where it may be serialized in a job or a database somewhere.

As Chronos extends DateTimeImmutable, and Carbon extends DateTime, wouldn’t the wiser choice be to add a new DateTimeImmutable data type and keep both Carbon and Chronos around for at least a major version where both could be used within an application. Laravel could handle an automatic conversion when it encounters a carbon object for something like Eloquent objects.

@carusogabriel
Copy link

Isn't easier to ask the owner of Carbon, @briannesbitt to add Collaborators to the repo to merge all bug fixes?

@mfn
Copy link

mfn commented Jan 11, 2018

I'm not sure. I think it's not only just merging the PRs/issues on Carbon. It's also about a thriving community, a working OSS contributor system.

Just because some people now might get elected, this doesn't yet prove a mid/long-term continued activity.

Granted, the Chronos repo does not have so much activity but it has activity (I took a quick look at the contributor pages: chronos and carbon). Just food for thought.

@GrahamCampbell was active there in the past too, maybe he has some insights. Currently the last contributions had come from @lucasmichot .

@nicolus
Copy link

nicolus commented Jan 20, 2018

Also laravel Horizon requires chronos, so I think it would make sense to eventually use chronos in both projects.

@carusogabriel
Copy link

@nicolus Oh, Horizon does uses Chronos 😱

This is a good argument for this change. We will just need to figure out how to handle conflicts between branches.

@sisve
Copy link

sisve commented Jan 22, 2018

There's a large problem with switching to Chronos now, package developers that wants to target both 5.5 LTS and 5.6 would need to know what they are doing. I'm using my all-seeing bitter eye and guess that package developers will mostly drop 5.5 and only support the latest and shiniest of releases. Or just not test it on 5.5. Anyway, both options will mean that the people on the 5.5 LTS release will see fewer or broken packages that supports them.

Can we wait with this change until the next LTS release in about 18 months? It would help if the LTS release, and the following normal releases, had the same core dependencies.

@driesvints
Copy link
Member

@sisve switching to Chronos should be done on a major Laravel release so it'd be normal for Package maintainers to also do a major release then. So it wouldn't be that much of a deal.

@wouterj
Copy link

wouterj commented Jan 22, 2018

Carbon has also been forked to https://github.com/CarbonDate/Carbon , preserving full backwards compatibility (it just continues where Brian stopped maintaining).

I'm maintaining the EloquentBundle for Symfony, and currently running into issues because of the dead Carbon package that Laravel depends upon. The dependencies doesn't yet allow Symfony 4, while the package does support it.

The new fork does have this support. Can Laravel please use this fork in the next minor release? It's full BC and maintained.

This issue can be discussed in the future for new major versions, but I think Laravel should avoid depending on dead packages.

@Miguel-Serejo
Copy link

I think using a Carbon fork might be a good idea for 5.6, but would still like to see a switch over to Chronos for 6.0.

@driesvints
Copy link
Member

I wouldn't switch to something different now. That would only make it even harder to switch later on.

@wouterj
Copy link

wouterj commented Jan 30, 2018

@driesvints I do not agree with that. The fork is no change with the real one, except that it's maintained. So for code, it shouldn't make anything more difficult/harder.

@driesvints
Copy link
Member

@wouterj I can see that but it wouldn't make sense to swap out a much-used component for another one, only for it to be replaced again in a rather short time period after that.

@wouterj
Copy link

wouterj commented Feb 12, 2018

Well, the "sense" that it makes is that I'm able to use Eloquent with a full Symfony 4 app :)

@briannesbitt
Copy link

Hey guys ! As you all can see, I haven't had the time that Carbon deserves. In an effort to revive the project, today I have communicated with and added 2 new committers on the Carbon project.

@taylorotwell
Copy link
Member

@briannesbitt nice! :)

@taylorotwell
Copy link
Member

@briannesbitt who are the additional maintainers?

@briannesbitt
Copy link

@taylorotwell basically active members of github (for years). @kylekatarnls has the 3rd most commits on the project and @Glavic is an active current member that was starting to take a stab at outstanding issues etc.

@kylekatarnls
Copy link

kylekatarnls commented Feb 13, 2018

Hi, my 2 cents. I understand many people could prefer immutable, chronos or any other library. I would say the safe way is keeping Carbon by default for all the Laravel 5.x live at least for backward compatibility. Changing the date typing on a minor release is a violent breaking change.

And the keyword is "by default", the same way we can use any template engine easily (a simple ServiceProvider and implementing Illuminate\View\Compilers\CompilerInterface), it should be a way to choose the DateTime class to use by default. So you could do something like that in your laravel config:

'default_date_time_class' => Chronos::class, // To use an other library or extended class
'default_date_time_class' => DateTime::class, // To use the PHP native basic class

I see only two entry points to update in Laravel:

// HasAttributes.php
if ($value instanceof DateTimeInterface) {
    return new Carbon(
        $value->format('Y-m-d H:i:s.u'), $value->getTimezone()
    );
}
// HasTimestamps.php
return new Carbon;

This would just need to be: new $defaultDateTimeClass instead.

And so Chronos project would be able to provide an adapter (with auto-discover to ease the install) that you would just have to install to get all your Eloquent DateTime has any type you want.

@tillkruss
Copy link

https://github.com/briannesbitt/Carbon/releases/tag/1.23.0

@kylekatarnls
Copy link

Yep new version released with many good things and we yet started to merge PRs for 1.24 which will have many new methods. Carbon is not dead!

@jamiehd
Copy link

jamiehd commented Jun 21, 2018

@taylorotwell does the closing of this mean you have no plans to do it? Would you accept a pull request that acts in the way @kylekatarnls suggested? That would ensure backwards compatibility, while allowing people who would rather immutable dates to use Chronos. I am planning on building an app that makes heavy use of dates in the near future, and even in my proof of concept I have found I need to write "clone $date" all over the place. This would be a nicer way of handling it if possible.

@kylekatarnls
Copy link

@jamiehd Note that Carbon 2 has immutable dates (CarbonImmutable class), check the version-2.0 branch of https://github.com/briannesbitt/Carbon

This way you could benefit of all new features and methods of Carbon without switching from library (and the backward compatibility is still quite better from Carbon 1 to Carbon 2 than to Chronos).

@jamiehd
Copy link

jamiehd commented Jun 21, 2018

That is great! However I guess there is no way to currently change it? Or am I missing something?

@kylekatarnls
Copy link

kylekatarnls commented Jun 21, 2018

You can set your dependency to "dev-version-2.0 as 1.25.0". Of course, this comes with no warranty as the version-2.0 is a development branch, not even an alpha release so it can still change a lot. But it would be great to have some feedback on this new version if you want to test it in your project.

@jamiehd
Copy link

jamiehd commented Jun 21, 2018

Sorry I mean how would I set Laravel to use it? Or is it just a case of overriding the methods Laravel uses to create Carbon objects?

@kylekatarnls
Copy link

Set your composer.json as mentionned above, run ˋcomposer update ˋ then search "Illuminate\Support\Carbon" in the alias list and replace it with "Carbon\CarbonImmutable"

@jamiehd
Copy link

jamiehd commented Jun 22, 2018

Amazing! Thank you.

@lucasmichot
Copy link

See See laravel/framework#24718

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests