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

Extract default listeners from Application #161

Open
wants to merge 4 commits into
base: 4.0.x
Choose a base branch
from

Conversation

Xerkus
Copy link
Member

@Xerkus Xerkus commented Dec 10, 2023

Q A
Documentation no
Bugfix no
BC Break yes
New Feature no
RFC no
QA yes

Description

In order to remove Application reliance on service locator pattern default listeners need to be extracted from Application class. Newly introduced ApplicationListenersProvider takes responsibility of registering default listeners.

Its factory is looking for a config key $config[Application::class]['listeners'] for additional listeners, removing the responsibility from application factory.

ApplicationListenersProvider is not a PSR listener provider.
It operates exclusively on laminas-eventmanager listener aggregates and does not actually allow listeners to be directly registered.

Another approach I considered was to use config provider and configuration to define default listeners. This double-dispatch approach however is more sound in ensuring listeners are always attached on Application instantiation.

Psalm issues which are not directly related intentionally left unfixed to be handled in a separate PR.
Documentation for the changes is to be done via separate PR. See #58

Xerkus added 3 commits July 28, 2024 23:35
Use double-dispatch approach to ensure default listeners are registered.

Signed-off-by: Aleksei Khudiakov <[email protected]>
Signed-off-by: Aleksei Khudiakov <[email protected]>
@Xerkus Xerkus force-pushed the feature/extract-default-application-listeners branch from 877e41e to 64b6064 Compare July 28, 2024 13:42
Signed-off-by: Aleksei Khudiakov <[email protected]>
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.

1 participant