-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Fix #32396 #32430
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
Fix #32396 #32430
Conversation
The src/Symfony/Bridge/Doctrine/Form/DoctrineOrmTypeGuesser.php implements the FormTypeGuesserInterface and that interface is only defined in the symfony/form component. By implementing that interface that code depends on it not only for test but also in production use. We discovered it because we have an api which doesn't require symfony form who blew up with the update to php 7.2.20. The release of php 7.2.20 fixed a bug that was allowing the implementation of interface that are not available.
This is not the right fix. |
Plus the PR would only heal a symptom and does not address the real problem. There are enough other constellations that will break because of this bug and adding unnecessary dependencies can't be the solution to all of them. |
@teohhanhui As long as that class in that package require the interface that interface is required all the time not only when you use the form component. That dependency has nothing optional right now. |
@derrabus How can you see that dependency as optional. The code is using it thus it's not optional. |
The real problem is php fixed it the wrong way, inconsistent with how optional dependencies are actually used in the real world. Or perhaps there's still a way to catch the fatal error. I don't know. But this is definitely not the right fix. |
@teohhanhui How do you want php to validate whether you correctly implement the interface if you don't give it the interface. Is it just supposed to assume it's correct. Because if that's what you believe, great you just made interface utterly useless. PHP corrected it the right way |
@mikeSimonson Please refer back to the discussion in https://bugs.php.net/bug.php?id=76980 @nikic said:
I'm not sure if it's a PHP bug or a Symfony bug at this point, but generally speaking, using optional dependencies is a very widespread practice in many Composer packages. |
Closing as explained in the linked issue. The root cause is discussed in #32395 |
The src/Symfony/Bridge/Doctrine/Form/DoctrineOrmTypeGuesser.php implements
the FormTypeGuesserInterface and that interface is only defined in the
symfony/form component.
By implementing that interface that code depends on it not only for test but
also in production use.
We discovered it because we have an api which doesn't require symfony form
who blew up with the update to php 7.2.20.
The release of php 7.2.20 fixed a bug that was allowing the implementation
of interface that are not available.