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

[Contracts][EventDispatcher] add EventDispatcherInterface to symfony/contracts and use it where possible #30691

Merged
merged 1 commit into from Mar 27, 2019

Conversation

Projects
None yet
4 participants
@nicolas-grekas
Copy link
Member

commented Mar 25, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

This PR adds a new EventDispatcherInterface in Contracts. This interface contains only one method: dispatch($event, $eventName). That covers almost all use cases of the event dispatcher in components (some use add/removeListeners/Subscribers but they are a minority.)

While doing so, it allows dispatching any objects as events - not only instances of Event.

This allows decoupling e.g. Messenger from the EventDispatcher component.
Next steps could be about planning to remove the base Event class from some concrete event classes and/or moving EventSubscriberInterface to symfony/contracts. It would allow decoupling e.g. Workflow from the component - but that's too far away for now, let's think about it in 5.1.

xdebug_enable();
}
}
throw new AutowiringFailedException($this->currentId, $failureMessage);

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Mar 25, 2019

Author Member

not directly related but was still needed to make tests pass locally

@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:ed-contracts branch 3 times, most recently from f7ad7c0 to a83fde3 Mar 25, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member Author

commented Mar 25, 2019

Green! (failure unrelated)

@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:ed-contracts branch from a83fde3 to 3c3db2f Mar 25, 2019

@fabpot

fabpot approved these changes Mar 25, 2019

Show resolved Hide resolved src/Symfony/Bundle/SecurityBundle/EventListener/FirewallListener.php
@@ -29,6 +29,10 @@
"conflict": {
"symfony/dependency-injection": "<3.4"
},
"provide": {
"psr/event-dispatcher-implementation": "1.0",

This comment has been minimized.

Copy link
@xabbuh

xabbuh Mar 26, 2019

Member

Can we do that (because this is only true when the psr/event-dispatcher package is present)?

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Mar 26, 2019

Author Member

It still does provide the implementation - when the package is present of course. When it isn't, nothing can provide it by definition. So yes we can.

Show resolved Hide resolved src/Symfony/Component/HttpKernel/EventListener/ExceptionListener.php
Show resolved Hide resolved src/Symfony/Component/Security/Http/Firewall.php
Show resolved Hide resolved src/Symfony/Contracts/EventDispatcher/EventDispatcherInterface.php
@xabbuh

xabbuh approved these changes Mar 26, 2019

@fabpot

This comment has been minimized.

Copy link
Member

commented Mar 27, 2019

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 3c3db2f into symfony:master Mar 27, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Mar 27, 2019

feature #30691 [Contracts][EventDispatcher] add EventDispatcherInterf…
…ace to symfony/contracts and use it where possible (nicolas-grekas)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[Contracts][EventDispatcher] add EventDispatcherInterface to symfony/contracts and use it where possible

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

This PR adds a new `EventDispatcherInterface` in `Contracts`. This interface contains only one method: `dispatch($event, $eventName)`. That covers almost all use cases of the event dispatcher in components (some use add/removeListeners/Subscribers but they are a minority.)

While doing so, it allows dispatching any objects as events - not only instances of `Event`.

This allows decoupling e.g. `Messenger` from the `EventDispatcher` component.
Next steps could be about planning to remove the base `Event` class from some concrete event classes and/or moving `EventSubscriberInterface` to `symfony/contracts`. It would allow decoupling e.g. `Workflow` from the component - but that's too far away for now, let's think about it in 5.1.

Commits
-------

3c3db2f [Contracts][EventDispatcher] add EventDispatcherInterface to symfony/contracts and use it where possible

@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:ed-contracts branch Mar 28, 2019

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019

@fabpot fabpot referenced this pull request May 9, 2019

Merged

Release v4.3.0-BETA1 #31435

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.