phpro / grumphp

A PHP code-quality tool
MIT License
4.12k stars 430 forks source link

Insufficient condition to switch interfaces for event interface. #928

Closed ansergeyg closed 2 years ago

ansergeyg commented 2 years ago
Q A
Branch master for features and deprecations
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Documented? no
Fixed tickets comma-separated list of tickets fixed by the PR, if any

Latest version of grumphp uses a workaround to switch Event class implementation depending on the symfony component's versions (for example: event-dispatcher and dependency-injection component versions).

But it doesn't consider the case when, some other contributed components use this new component directly: Event-dispatcher-contracts:

Symfony event-dispatcher component uses it starting from the version ^4.3. Now, imagine that some other component uses symfony event dispatcher ^3.4 and installs another contributed component that directly uses https://github.com/symfony/event-dispatcher-contracts). It means that now the event dispatcher will be an old one, but the grumphp latest version will bring this workaround and the Event class will be switched to the SymfonyEventContract and the the conflict occurs.

This PR fixes this issue.

New Task Checklist:

veewee commented 2 years ago

Do you have an example configuration in which it breaks? I am not sure I am following the explanation above.

This package requires:

        "symfony/event-dispatcher": "~4.4 || ~5.0",

Meaning you will get conflicts if another package requires 3.4 right?

If the statements about the versions above are correct, we might as well drop the legacy event fallback all together?

ansergeyg commented 2 years ago

Hi @veewee , Sorry it's not the latest version of grumphp, but v0.17.2:

https://github.com/phpro/grumphp/blob/ecfc01920fb5733193a41da292e8abb702b6d55d/composer.json#L27

Sure, if you can drop it, it also works, but if clients use symfony components ^3.4, it means that they are not ready yet to upgrade to symfony ^4.3, but they still might want to use grump.

Regards, Sergey

veewee commented 2 years ago

Hello @ansergeyg,

Sorry, We are not actively supporting the 0.X branch anymore. If you want to avoid having old dependency conflicts, you might want to use our dependency-less version: https://github.com/phpro/grumphp-shim

veewee commented 2 years ago

I think it is safest to keep this event fallback system until we drop support for Symfony 4.4 next year.

In any case, thanks for your time and keeping it on the radar!