-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DI] Shared services should not be inlined in non-shared ones #27265
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
Conversation
…nes (nicolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- [DI] Shared services should not be inlined in non-shared ones | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - [Already good on 2.7/2.8.](https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php#L152) Commits ------- 6f8b862 [DI] Shared services should not be inlined in non-shared ones
@@ -138,6 +138,6 @@ private function isInlineableDefinition($id, Definition $definition, ServiceRefe | |||
return false; | |||
} | |||
|
|||
return true; | |||
return $this->container->getDefinition($ids[0])->isShared(); |
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.
Look like this change break the compilation with Undefined offset: 0
when the node doesn't have any InEdges
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.
hotfixed in 5eb17e5
but I don't understand how this can happen logically (we're here because we did find a reference to the service, to it must have at least one in-edge.)
could you share a reproduder?
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.
The use case is: I use a custom TestKernel which inject mocks in the container and set the service as Synthetic with a compiler pass
NB: Changing the type of the compiler pass to PassConfig::TYPE_AFTER_REMOVING
(to run after InlineServiceDefinitionsPass
) fix the issue
This minimal reproduct case is:
class Kernel extends BaseKernel implements CompilerPassInterface
{
// ...
public function process(ContainerBuilder $container)
{
// routing.resolver is just an example for the minimal reproduct case here
$container->getDefinition('routing.resolver')->setPublic(true)->setSynthetic(true);
}
}
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.
ok, so this means we are missing a check for synthetic service!
added in #27366
Already good on 2.7/2.8.