theofidry / PsyshBundle

A command line REPL bundle for Symfony using PsySH.
MIT License
208 stars 29 forks source link

[RFC] Build container with public services #36

Closed Nek- closed 4 years ago

Nek- commented 6 years ago

Hello guys. First thanks for this nice bundle :) .

This is about providing a new idea for the bundle, I don't know how to do it but it would be useful.

What do you think about? Any idea of how to implement this?

theofidry commented 6 years ago

I agree with you. Maybe something similar to https://github.com/symfony/symfony/pull/26499 would do

Nek- commented 6 years ago

This PR is a real opportunity indeed. But I think injecting test.service_container instead of the real container into psysh would be a better idea.

This RFC will wait for Symfony 4.1 :) .

theofidry commented 6 years ago

Yeah although we can always go ahead and I'll just tag it once 4.1 is out :)

kubk commented 6 years ago

Hello @theofidry , thanks for the Bundle. I tried to use Psysh bundle in Symfony 4.1 (beta) with test.service_container and this container works as expected, private services are available in this container.

In order to use test.service_container in Psysh bundle we need to register test.service_container in dev environment (currently this service available only in test environment: https://github.com/nicolas-grekas/symfony/blob/a840809e5dad429c95eafe40b5dd2ea593a7a232/src/Symfony/Bundle/FrameworkBundle/Resources/config/test.xml)

So I think adding support for test.service_container in Psysh bundle must be implemented without BC break.

I tried to change this line: https://github.com/theofidry/PsyshBundle/blob/160c95a55f2085ab6f90eacc74569620f6f158ae/src/DependencyInjection/PsyshExtension.php#L48 to

'container' => $container->hasDefinition('test.service_container') ? new Reference('test.service_container') : new Reference('service_container'),

but $container->hasDefinition('test.service_container') always returns false, even if the service is registered. But it will work if I change this line to

'container' => new Reference('test.service_container'),

Did I miss something? Here is my services.yml:

    service_locator:
        class: Symfony\Component\DependencyInjection\ServiceLocator
        arguments: [['@session']]
        tags: [container.service_locator]

    test.service_container:
        class: Symfony\Bundle\FrameworkBundle\Test\TestContainer
        public: true
        arguments:
             - null
             - '@service_container'
             - '@service_locator'
theofidry commented 6 years ago

Hello :)

but $container->hasDefinition('test.service_container') always returns false, even if the service is registered.

If you registered it yourself then it's likely that the bundle is ran before your services are registered. So maybe this part (the Psysh one) should be done later

kubk commented 6 years ago

@theofidry Thanks for the response, you are right. I think we have 2 options:

Which option do you prefer? I can make a PR.

theofidry commented 6 years ago

I'm fine with the second, this bundle is bound to be tightly coupled to framework bundle anyway

theofidry commented 4 years ago

Closed by #38