Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.x] Carbon 3 #11348

Merged
merged 14 commits into from
Feb 21, 2025
Merged

[5.x] Carbon 3 #11348

merged 14 commits into from
Feb 21, 2025

Conversation

duncanmcclean
Copy link
Member

@duncanmcclean duncanmcclean commented Jan 13, 2025

This pull request adds support for Carbon 3, allowing us to support Laravel 12 when it's released.

While Statamic 5 is backwards compatible with Carbon 2, you might find that Carbon 3 is installed as a result of this update. If you'd rather stay with Carbon 2, you may require it using Composer:

composer require nesbot/carbon:^2

@duncanmcclean duncanmcclean changed the title [6.x] Carbon 3 [5.x] Carbon 3 Jan 13, 2025
@duncanmcclean duncanmcclean changed the base branch from master to 5.x January 13, 2025 15:17
@duncanmcclean duncanmcclean marked this pull request as ready for review January 13, 2025 15:20
Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep this targeting master.

If someone does a composer update and unintentionally upgrades to Carbon 3, there might be some breaking changes they'd need to deal with.

@duncanmcclean duncanmcclean changed the base branch from 5.x to master January 22, 2025 17:10
@duncanmcclean duncanmcclean changed the title [5.x] Carbon 3 [6.x] Carbon 3 Jan 22, 2025
@duncanmcclean
Copy link
Member Author

Changed the target branch back to master.

I've kept the Carbon v2 logic for now as Laravel 11 supports it alongside v3. We can probably remove it in Statamic 7?

@jasonvarga
Copy link
Member

Yup keeping v2 is good 👌

@duncanmcclean duncanmcclean mentioned this pull request Jan 27, 2025
duncanmcclean added a commit that referenced this pull request Jan 27, 2025
Until we merge #11348, then we don't need to pretend anymore.
@the-pulli
Copy link

the-pulli commented Feb 3, 2025

What's a bit annoying in v5 are these deprecation warnings on composer update/install with PHP 8.4:

Deprecated: Carbon\Traits\Date::getDaysFromStartOfWeek(): Implicitly marking parameter $weekStartsAt as nullable is deprecated, the explicit nullable type must be used instead in /pulli/vendor/nesbot/carbon/src/Carbon/Traits/Date.php on line 1394

Deprecated: Carbon\Traits\Date::setDaysFromStartOfWeek(): Implicitly marking parameter $weekStartsAt as nullable is deprecated, the explicit nullable type must be used instead in /pulli/vendor/nesbot/carbon/src/Carbon/Traits/Date.php on line 1412

Deprecated: Carbon\Traits\Date::utcOffset(): Implicitly marking parameter $minuteOffset as nullable is deprecated, the explicit nullable type must be used instead in /pulli/vendor/nesbot/carbon/src/Carbon/Traits/Date.php on line 1481

Deprecated: Carbon\Traits\Localization::locale(): Implicitly marking parameter $locale as nullable is deprecated, the explicit nullable type must be used instead in /pulli/vendor/nesbot/carbon/src/Carbon/Traits/Localization.php on line 447

Deprecated: Carbon\Traits\Test::setDefaultTimezone(): Implicitly marking parameter $date as nullable is deprecated, the explicit nullable type must be used instead in /pulli/vendor/nesbot/carbon/src/Carbon/Traits/Test.php on line 203

Deprecated: Return type of Carbon\Traits\Date::createFromTimestamp($timestamp, $tz = null) should either be compatible with DateTime::createFromTimestamp(int|float $timestamp): static, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /pulli/vendor/nesbot/carbon/src/Carbon/Traits/Timestamp.php on line 29

Deprecated: Carbon\CarbonTimeZone::toOffsetName(): Implicitly marking parameter $date as nullable is deprecated, the explicit nullable type must be used instead in /pulli/vendor/nesbot/carbon/src/Carbon/CarbonTimeZone.php on line 158

Deprecated: Carbon\CarbonTimeZone::toOffsetTimeZone(): Implicitly marking parameter $date as nullable is deprecated, the explicit nullable type must be used instead in /pulli/vendor/nesbot/carbon/src/Carbon/CarbonTimeZone.php on line 172

Deprecated: Carbon\CarbonTimeZone::toRegionName(): Implicitly marking parameter $date as nullable is deprecated, the explicit nullable type must be used instead in /pulli/vendor/nesbot/carbon/src/Carbon/CarbonTimeZone.php on line 188

Deprecated: Carbon\CarbonTimeZone::toRegionTimeZone(): Implicitly marking parameter $date as nullable is deprecated, the explicit nullable type must be used instead in /pulli/vendor/nesbot/carbon/src/Carbon/CarbonTimeZone.php on line 230

With Carbon v3 they're gone.

@jasonvarga
Copy link
Member

I think we'll probably get rid of Carbon 2 in v6 after all.

@duncanmcclean duncanmcclean changed the title [6.x] Carbon 3 [5.x] Carbon 3 Feb 7, 2025
@duncanmcclean duncanmcclean changed the base branch from master to 5.x February 7, 2025 17:29
duncanmcclean added a commit that referenced this pull request Feb 7, 2025
Until we merge #11348, then we don't need to pretend anymore.
@duncanmcclean duncanmcclean mentioned this pull request Feb 7, 2025
4 tasks
@jasonvarga
Copy link
Member

What's a bit annoying in v5 are these deprecation warnings on composer update/install with PHP 8.4:
With Carbon v3 they're gone.

They are now also removed in Carbon 2.73.0. Just doing a composer update should resolve them.

@the-pulli
Copy link

the-pulli commented Feb 13, 2025

They are now also removed in Carbon 2.73.0. Just doing a composer update should resolve them.

Yes, I noticed this yesterday on a composer update. Thanks!

A related issue is with thecodingmachine/safe. There are a ton of deprecation warnings with v2. Now there's v3 released (https://github.com/thecodingmachine/safe/releases/tag/v3.0.0), which resolves the issues with PHP 8.4. As I gathered it's backwards compatible.

Maybe you've a look and it's solvable with just bumping the version to v3.0.

UPDATE

As I wanted to provide a pull request for the issue, I noticed that the package rebing/graphql-laravel requires thecodingmachine/safe with v2. So for the moment, there's nothing you can do about it.

ANOTHER UPDATE

Created a PR (rebing/graphql-laravel#1165) on rebing/graphql-laravel to hopefully solve this.

@duncanmcclean
Copy link
Member Author

Yes, we're aware about the thecodingmachine/safe package. We're waiting on third-party dependencies to update.

@jasonvarga
Copy link
Member

All the "ago" modifiers should have also had absolutes applied to them.

e.g. If you did {{ future_date | days_ago }} it would output 3 when using Carbon 2.
On Carbon 3 it would give you a negative. Wrapping in abs() brings it back to a positive integer to keep those modifiers working the same way they did with Carbon 2.

But those modifiers are pretty awkward for the same reasons I'm sure they made those changes in Carbon itself.

For Statamic 6 we should probably just have those output the values Carbon now gives us, as a breaking change.

e.g. {{ future_date | days_ago }} should output -2.38479423 or whatever it is. People can round it and absolute it if they need to.

In the tests I've added comments for the real values. The assertions can be tweaked to use those.

@jasonvarga jasonvarga merged commit 1a50ee8 into 5.x Feb 21, 2025
19 checks passed
@jasonvarga jasonvarga deleted the carbon-3 branch February 21, 2025 17:29
@the-pulli
Copy link

Yes, we're aware about the thecodingmachine/safe package. We're waiting on third-party dependencies to update.

@duncanmcclean thanks for the PR on this. Now all deprecation warnings in PHP 8.4 are finally gone 👍

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

Successfully merging this pull request may close these issues.

3 participants