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
[DI] Throw an exception if Expression Language is not installed #25285
[DI] Throw an exception if Expression Language is not installed #25285
Conversation
Are we really going to do this now on each place where optional dependency is used? I'm 👎 on that. There should be global solution and/or not doing this at all and just document what such php error usually means. |
@@ -155,6 +156,10 @@ private function getDefinitionId($id) | |||
private function getExpressionLanguage() | |||
{ | |||
if (null === $this->expressionLanguage) { | |||
if (!class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not :
if (!class_exists(ExpressionLanguage::class)) {
//body
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy/pasting from the other check, that's it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah but I think it's better to use this since we already replaced some of the one we had in the symfony code base. Let's introduce something we don't need to re-modify in the futur :p.
Thank you @sroze. |
…alled (sroze) This PR was merged into the 3.4 branch. Discussion ---------- [DI] Throw an exception if Expression Language is not installed | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #25277 | License | MIT | Doc PR | ø The [`PhpDumper` already has this check](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php#L1688-L1690) but it is missing here. Commits ------- 75b21e9 Throw an exception is expression language is not installed
The
PhpDumper
already has this check but it is missing here.