Skip to content
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

Allow listeners and event subscribers to be private #594

Merged
merged 5 commits into from
Jul 20, 2017
Merged

Conversation

goetas
Copy link
Collaborator

@goetas goetas commented Jul 18, 2017

Fixes #593

@weaverryan
Copy link

Ha! Was it that easy? :)

Assuming you've tested that this works irl (I'm not very familiar with the JMS event dispatcher), a big +1!

Thanks!

@goetas
Copy link
Collaborator Author

goetas commented Jul 19, 2017

Ha! Was it that easy? :)

actually it turned out to be not that easy. the previous implementation had the side effect that services become not-lazy...

the current solution requires at least symfony 3.3 and uses some kind of "proxy" class to forward events but keeping the listener lazy... I'm not really happy with the solution, but do not see an easy way to do it.

A similar approach can be to use symfony "lazy" services (using https://symfony.com/doc/current/service_container/lazy_services.html), this solution might work with older symfony versions, but will require the proxy library

Update:
the previous implementation had the side effect that public services become not-lazy, now if a service is private, will not be lazy. of course this can be avoided using https://symfony.com/doc/current/service_container/lazy_services.html

@goetas goetas force-pushed the private-services branch from eb11830 to 6171b69 Compare July 19, 2017 10:06
@goetas
Copy link
Collaborator Author

goetas commented Jul 19, 2017

@bgaleotti thanks :)

@goetas goetas merged commit 4748036 into master Jul 20, 2017
@goetas goetas deleted the private-services branch July 26, 2017 14:32
@goetas goetas mentioned this pull request Aug 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants