⬆️ Drop Symfony < 4.4 and PHP < 7.2.5 support

[kylekatarnls]

Next major version of Carbon will drop Symfony < 5 (and so PHP < 7.2.5) and so old Translator interface in favour of the new Contracts namespace.

[c33s]

please reconsider dopping symfony 4.4 support, look at the symfony version/release policy:

Symfony develops at the same time two versions: the new major one (e.g. 4.0) and the latest version of the previous branch (e.g. 3.4).

Both versions have the same new features, but they differ in the deprecated features. The oldest version (3.4 in this example) contains all the deprecated features whereas the new version (4.0 in this example) removes all of them.

https://symfony.com/doc/current/contributing/community/releases.html#backward-compatibility

so 4.4 is euqal with symfony 5.0 only that is still has the old features marked as deprecated plus the new symfony 5.0 features. so symfony 4.4 supports the new contracts namespace.

as symfony 4.4 is an LTS version and supported until nov 2022 (bug) & nov 2023 (security) it would be a better choice to only drop symfony support lower than 4.4 instead of 5.0 and allow to use this awesome lib with the symfony LTS version.

[kylekatarnls]

That's exactly the same for Carbon. We're developing 3.x while we still continue to maintain 2.x.

Then the support of a given Symfony version by other libraries is not driven by the Symfony version policy. That policy does only concern the symfony namespace library.

As you may see Laravel is developing its 8.0 version and already dropped Symfony < 5 since 7.0.

Carbon won't stop to be compatible with Symfony 4, Carbon 2 will still be available as usual from packagist. Only the new version Carbon 3 which will come with other breaking changes will require Symfony 5 as the minimum requirement.

[c33s]

@kylekatarnls

i know that your version policy don't match symfonys, thats the reason why i added this comment, just wanted to let you know that this two symfony versions are basically exactly the same.

but why? as 4.4==5.0 there is no good reason for dropping support for it.

As you may see Laravel is developing its 8.0 version and already dropped Symfony < 5 since 7.0.

i don't know laravel but i simply take your info for real, so my comment would also match there, it makes no sense for laravel to drop symfony 4.4 support as, i repeat myself, symfony 4.4 has the same feature set as symfony 5.0.

in my symfony projects i stick with LTS -> 4.4, and i use quite a lot 5.x symfony components. they are compatible

i assume that you will add return types and other great php language features which i love to use, so i would love to use carbon 3.0 with symfony 4.4

[kylekatarnls]

4.4==5.0

That may look so from the application side point of view, but that's not true, and using 1 version for you application is nothing like maintaining a range of multiple versions for each library (+ the language itself) Carbon must remain compatible with.

If you don't want to upgrade to Symfony 5, you should not worry about staying with Carbon 2 either.

symfony 4.4 has the same feature set as symfony 5.0

That's not a decisive reason for keeping/dropping support.

i use quite a lot 5.x symfony components

Great, so you can upgrade the translation component to ^5.0 (that's the only one we need) and that's the one component that forced us previously to have many if-else code to support both symfony/translation 4 and symfony/translation 5.

Carbon 3 is still far from being released as a stable version. Maybe you'll no longer use Symfony 4.4 at this moment. And we still have time to check and re-enable the Symfony 4.4 at the single condition it would not require to alias namespaces/classes or to deal with differences in method signatures.

[c33s]

thanks for your time and for your fast, friendly and detailed replies <3

maybe you can be so kind and give me a code example where the access/api to the symfony 4.4 translation differs from the symfony 5.0?

i would be really interested in this topic because the symfony docs states 4.4 == 5.0, so symfony 4.4 translation component should be able to be accessed as if it would be a 5.0 component with the ability to access it with the old 4.x api but if you access it with the 4.x api it would show a deprecation warning.

from my understanding the BC layer lies inside of symfony and it should not require you to add the bc layer in your lib.

to have many if-else code to support both symfony/translation 4 and symfony/translation 5.

so this should only be required if you also support 4.3 but not if you support only 4.4

for example 8106250#diff-40f47bcf6cf989bec1e8ef5c8f7bb08aR729 here component is replaced with contract. in my symfony 4.4 installation the translation-contracts are installed. so i have the right translation interface. Symfony\Contracts\Translation\TranslatorInterface is available and compatible with 4.4

do is overlook or miss something?

and thanks for this awesome lib 👍 ❤️

[kylekatarnls]

give me a code example

Sadly very easy, here's what happen if you try to install Symfony 4.4 and run the unit tests:
https://travis-ci.org/github/briannesbitt/Carbon/jobs/716152162#L366

As namespace of interface is not the same, the same code cannot be used as is.

Currently there are 8 broken tests in the branch 3.x (they will be progressively re-enabled). Omitting those ones, I except all unit tests to pass. If installing symfony/translation 4.4 breaks tests, I consider it's not compatible with the usage we made of it in Carbon 3 following the way documented for the symfony/translation 5 API.

If we have to require "translation-contracts" while it's actually only needed for Symfony 4 users, that's pretty annoying. That's a small package but I feel not very comfortable with adding additional libraries for backward-compatibility. It does not sound like the Carbon library's responsibility. That's actually maybe worst than aliasing classes/interfaces as it's duplicate the code and classes will still only implement one or the other but not both.

[c33s]

hmm, have not tried the tests locally but having a quick look on the dependnecies of symfony/translation there is a chance to quick fix the compat issue.

sf5.0 and sf4.4 symfony/translation requires symfony/translation-contracts so it will be installed for all carbon user not only for sf4.4 users.

sf4.4 symfony/translation allows symfony/translation-contracts 1.1.16 or 2 while sf5 symfony/translation requires symfony/translation-contracts 2

so adding the following to carbons composer file should/maybe fixes the tests and as the contracts gets installed anyway it is not an additional requirement only the correct requirement

symfony/translation-contracts: ^2

https://packagist.org/packages/symfony/translation

v4.2.0 2018-11-27 07:20 UTC
Requires
    php: ^7.1.3
    symfony/contracts: ^1.0
    
v4.3.0 2019-05-28 09:09 UTC
Requires
    php: ^7.1.3
    symfony/polyfill-mbstring: ~1.0
    symfony/translation-contracts: ^1.1.2

v4.4.0 2019-11-12 17:18 UTC
Requires
    php: ^7.1.3
    symfony/polyfill-mbstring: ~1.0
    symfony/translation-contracts: ^1.1.6|^2

v5.0.0 2019-11-18 17:27 UTC
Requires
    php: ^7.2.5
    symfony/polyfill-mbstring: ~1.0
    symfony/translation-contracts: ^2

[kylekatarnls]

Requiring symfony/translation-contracts: ^2 does not seem to help:
https://travis-ci.org/github/briannesbitt/Carbon/jobs/716159698#L385

Travis actually install 2.1.2 in this build and still fail on Component vs. Contracts interface matching.

[kylekatarnls]

Actually, the interface is not missing, it's just not compatible so I doubt it could be fixed so easily.

[c33s]

you maybe already have seen i played around with your test branch #2153

for testing purposes i hardcoded the php version and i personally use composer.yaml simply ignore it. i also added the return types of the methods as 3.x requires at least php 7.2 which is able to handle return types.

i dont understand why the deprecated interface is still in the codebase. requiring symfony 5.x also means there is no Compontent/../TranslatorInterface only Contracts/../TranslatorInterface so it can completly removed. then the bc layer for handling the components one also can be removed. so no interface renaming as ContractsTranslatorInterface required everything is Contracts/../TranslatorInterface which should be perfectly fine for sf4.4 and sf5.x

after my change the tests look not that bad: https://travis-ci.org/github/briannesbitt/Carbon/builds/716173343

sidenote there are some risky code parts i, my awesome phpstorm plugin made me aware of some risky code parts. maybe have a look at https://plugins.jetbrains.com/plugin/7622-php-inspections-ea-extended-

[kylekatarnls]

Carbon 2 supports symfony/translation 3.4+ so it has too keep both namespaces. In Carbon 3, we'll definitely drop Symfony 3 and the old namespace.

This state of tests looks good. But it has to pass with the composer flag --prefer-lowest meaning dependencies themselves are still compatible if they includes the lowest versions of their own dependencies.

Still this POC is very good.

[c33s]

Carbon 2 supports symfony/translation 3.4

but we talk about carbon 3 here, where you want to drop symfony 4.4, if you drop everything below symfony 5.0 you would also drop symfony 3.4. ;)

from my knowledge it should work with the following:

• completly drop Component/../TranslationInterface
• require symfony/translation-contracts: ^2 which is the same sf5.0 would do
• remove the BC layer in the translation class

with this changes sf4.4 should behave like sf5.0 and no drop of supporting it is required as it is really basically the same as 5.0

just ping me if you need more info or testing. for me it is an important thing that sf4.4 will be supported in carbon 3.0 (slack chat is also possible)

[kylekatarnls]

if you drop everything below symfony 5.0 you would also drop symfony 3.4

Obviously, and Carbon 3 is not released and this issue is still open. The branch I mentioned in your PR is the one more advanced on this topic: feature/issue-1967-week-based-on-locale already drop all the old interfaces.

4.4 => 5.0 means breaking change, and when Symfony 6.0 will be released, we'll could fall again in the same situation than Carbon 2 with 3.4 => 5.1 range and incompatibilities to handle.

[kylekatarnls]

Symfony 5.0.0:

public function setLocale(string $locale)

https://github.com/symfony/translation/blob/v5.0.0/Translator.php#L147

Symfony 4.4.0:

public function setLocale($locale)

https://github.com/symfony/translation/blob/v4.4.0/Translator.php#L159

This first difference force us to pick the less precise typehint in our Translator to support 4.4.0 while the "string" enforcing would actually help users as you can see in #2157.

[c33s]

if people put in an array an error occurs, as it happend in the referenced ticket. if you want to catch it near the setLocale() call "webmozart assert string" is also an option.
there is not thaaat impact in supporting 4.4, you simply don't want. well can't do anything about it.

[kylekatarnls]

The same thing happen for the type of the trans() method, the type weakening is a loss that does not worth it in my opinion. I will first merge #2165 which is the branch that switches to the new namespace for TranslatorInterface. So the cost of the 4.4 support can be properly evaluated and considered relevant or not.

Here is the typings that are not compatible from Symfony 4.4 to 5.0:
https://github.com/briannesbitt/Carbon/pull/2165/files#diff-898d24de83d5ec58cd507ef80cbedcc8R149
https://github.com/briannesbitt/Carbon/pull/2165/files#diff-898d24de83d5ec58cd507ef80cbedcc8R314

And I'm not enjoying to level down the hints which make error messages clean with no extra code.

[kylekatarnls]

Symfony 4.4 will be supported in Carbon 3.0. I won't guarantee the continuity of the support for the whole lifetime of Carbon 3. But for now as only the Translator class has to have weak types and is an inner class which methods are rarely called in user-land, it's acceptable to keep the old typehints in it for now.