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

make it possible to decorate services #620

Merged
merged 1 commit into from
Nov 30, 2017

Conversation

xabbuh
Copy link
Contributor

@xabbuh xabbuh commented Nov 29, 2017

If another part of the application decorates a built-in service, the
compiler passes will fail if they run after service decorators have
been resolved (in this case, the original service id is an alias
pointing to the decorating service).

If another part of the application decorates a built-in service, the
compiler passes will fail if they run after service decorators have
been resolved (in this case, the original service id is an alias
pointing to the decorating service).
@xabbuh
Copy link
Contributor Author

xabbuh commented Nov 29, 2017

In FOSRestbundle, for example, we currently have a weird workaround which replaces the jms_serializer.handler_registry service with a custom definition after the JMSSerializerBundle passes have been processed. However, this will not work anymore in Symfony 4 where services are private by default (because then, at the time when the pass is run, the JMS services may have been inilined because they are private).

@goetas
Copy link
Collaborator

goetas commented Nov 30, 2017

👍

@goetas goetas merged commit 457f8da into schmittjoh:master Nov 30, 2017
@xabbuh xabbuh deleted the compiler-passes branch November 30, 2017 09:24
@xabbuh
Copy link
Contributor Author

xabbuh commented Nov 30, 2017

Thanks for your quick reaction @goetas! This will greatly simplify things on our end. Much appreciated!

xabbuh added a commit to xabbuh/JMSSerializerBundle that referenced this pull request Dec 14, 2017
There was a conflict between schmittjoh#618 and schmittjoh#620 which should be resolved by
this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants