Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Make ServiceSubscriberTrait compatible with AbstractController for Symfony 7 with strong types#54490

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
pableu wants to merge1 commit intosymfony:7.0frompableu:patch-2

Conversation

@pableu
Copy link
Contributor

QA
Branch?7.0
Bug fix?yes
New feature?no
Deprecations?🤔
LicenseMIT

If you have a class like this:

You get the following error in Symfony 7 with symfony/contracts 3.4.2:

<?phpnamespaceApp\Controller;useSymfony\Bundle\FrameworkBundle\Controller\AbstractController;useSymfony\Contracts\Service\ServiceSubscriberTrait;abstractclass DemoControllerextends AbstractController{use ServiceSubscriberTrait;// ...}

Compile Error: Symfony\Bundle\FrameworkBundle\Controller\AbstractController and Symfony\Contracts\Service\ServiceSubscriberTrait define the same property ($container) in the composition of App\Controller\DemoController. However, the definition differs and is considered incompatible. Class was composed

ServiceSubscriberTrait::$container is defined as:

/** @var ContainerInterface */protected$container;

WhileAbstractController::$container is:

protected ContainerInterface$container;

This leads to the incompatible composition.

This PR would fix that, but I'm not sure which versions of symfony/framework-bundle and symfony/contracts to target. Maybe we should add a conflict against symfony/framework-bundle < 7.0 to symfony/contract'scomposer.json.

@derrabus
Copy link
Member

Your change would be a breaking change for other users of the trait. I'm afraid, we cannot merge it.

@pableu
Copy link
ContributorAuthor

pableu commentedApr 4, 2024•
edited
Loading

I see. Can we.. tag a higher version maybe? How is versioning and the bc-promise done for symfony/contracts?

My issue appeared when upgrading from 6.4 to 7.0. I can easily work around it, but I thought an easier upgrade and more type safety in the trait might be nice.

@nicolas-grekas
Copy link
Member

That's quite unfortunate!
I think we can do it more smoothly by deprecating the trait and providing another one that declares the type of the property.
See#54496
Thanks for sharing this concern and proposing a fix.

pableu reacted with heart emoji

@pableu
Copy link
ContributorAuthor

Oh lovely@nicolas-grekas, thanks for your much more complete solution in#54496, that looks great.

@pableupableu deleted the patch-2 branchApril 5, 2024 10:52
fabpot added a commit that referenced this pull requestApr 5, 2024
…thodsSubscriberTrait (nicolas-grekas)This PR was merged into the 7.1 branch.Discussion----------[Contracts] Rename ServiceSubscriberTrait to ServiceMethodsSubscriberTrait| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      | no| New feature?  | no| Deprecations? | yes| Issues        |Fix#54490| License       | MITAs described in the linked PR, AbstractController is incompatible with ServiceSubscriberTrait because of the added type to the AbstractController::$container property, while ServiceSubscriberTrait's $container property cannot have a type without a BC break.There are two parts to this PR:- Deprecate ServiceSubscriberTrait in favor if ServiceMethodsSubscriberTrait, which declares the type of the $container property. The new name better conveys its purpose as a bonus.- Fix the incompatibility with AbstractController by removing the property declaration on ServiceSubscriberTrait. This means the trait will create a dynamic property. Those are deprecated, but since the trait is also deprecated, the upgrade path is clear.I also tried to improve the description of the trait in the meantime./cc `@kbond`Commits-------8f47ced [Contracts] Rename ServiceSubscriberTrait to ServiceMethodsSubscriberTrait
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

7.0

Development

Successfully merging this pull request may close these issues.

4 participants

@pableu@derrabus@nicolas-grekas@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp