Declare a separate ImagesUploadListener service #9310
Conversation
Zales0123
added
the
Documentation
| class: Sylius\Bundle\CoreBundle\EventListener\ImagesUploadListener | ||
| decorates: sylius.listener.image_upload | ||
| arguments: ['@sylius.image_uploader'] |
There was a problem hiding this comment.
Don't we need @app.listener.image_upload.inner here now?
There was a problem hiding this comment.
We're not using it, because we're basically "abusing" service decoration. 😛
| class: Sylius\Bundle\CoreBundle\EventListener\ImagesUploadListener | ||
| decorates: sylius.listener.image_upload |
There was a problem hiding this comment.
Okay, I guess we should use parent: sylius.listener.image_upload here, then we can remove arguments (as they are inherited) and just include tags (as we're not really decorating the service).
There was a problem hiding this comment.
tags are not inherited from the parent service. Or do you mean we should use decorates together with parent?
There was a problem hiding this comment.
I don't think it'll work: symfony/symfony#14543
There was a problem hiding this comment.
IIRC tags aren't inherited from the decorated service as well (for this PR, these product & taxon tags are still assigned to @app.listener.image_upload.inner and shipping method ones will be assigned to @app.listener.image_upload).
There was a problem hiding this comment.
Tags are copied over when decorating. See the referenced PR in the description. :)
There was a problem hiding this comment.
Anyway, since we use decoration, anywhere sylius.listener.images_upload is used, it'll basically be replaced by app.listener.images_upload. The original service will not be used anymore. In fact, it will be automatically removed from the container if it's a private service and unused.
There was a problem hiding this comment.
Oh, and I just noticed the original typo in the service name (or it was changed). The free extra benefit we get by using decoration is that it'll fail if the service being decorated is not found. 😃
teohhanhui
force-pushed
the
docs/images-upload-listener-decorator
branch
from
566e9ec
to
9b75032
Compare
I've just tried it, and using |
|
@pamil Any decision on this? |
|
I just needed a while to test it out, it looks like Using Using |
Don't override sylius.listener.images_upload
teohhanhui
force-pushed
the
docs/images-upload-listener-decorator
branch
from
9b75032
to
d24b2d4
Compare
teohhanhui
changed the title
|
@pamil This should be good now. 😄 |
|
Thank you, Teoh! 🥇 |
Don't override
sylius.listener.images_upload, so that we don't have to duplicate tags that might change at anytime.