Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
The event dispatcher is from Symfony, which means it's not a Drupalism. Because we do not own and control the system (third non-Drupal parties can provide events too) we must prefix all integrations (machine names) with our vendor name to prevent collissions.
Proposed resolution
- Adopt a convention to prefix event names with the vendor (
drupal
) and package (module) name, such asdrupal.block.active_context
instead ofblock.active_context
. Example. - Convert all event names.
Remaining tasks
To be decided.
User interface changes
None.
API changes
None, because all code should use the constants, of which the names will remain unaltered. If any code uses the machine names directly, that is a bug that should be fixed in that code.
Beta phase evaluation
Issue category | Bug, because Drupal's event names invade Symfony's namespace and can collide with other package's event names. |
---|---|
Issue priority | Normal |
Unfrozen changes | Unfrozen, because it is a bug (prioritized change) |
Prioritized changes | The main goal of this issue is fixing a bug. |
Disruption | Theoretically there is no disruption, as all code should use the event's constants instead of the machine names. The constant's names remain unchanged, so any code using these events should continue to work without changes. |
Comments
Comment #1
eojthebraveThis seems like a good idea to me too. So +1.
You can get a list of the events that need to be renamed here: https://api.drupal.org/api/drupal/core%21modules%21system%21core.api.php..., just note that this is ALL events not just Drupal events so you'll need to differentiate.
The Symfony docs say this about naming events: http://symfony.com/doc/current/components/event_dispatcher/introduction....
So it seems like we're on the right track here.
Comment #2
XanoComment #3
lauriiiWhat about drupal.core? At least for me it would make more sense.
Comment #4
XanoThat's a valid suggestion. It would mimic Drupal core's Composer package name (
drupal/core
). My approach was to use project names for the namespaces. Seeing as config comes from http;//drupal.org/project/drupal,drupal
is the package name.Either way is fine with me, as long as we identify core consistently across systems.
Comment #5
dawehnerComment #9
bander2 CreditAttribution: bander2 as a volunteer commentedCan't apply against 8.4.v-dev because core/modules/block/src/Event/BlockEvents.php no longer exists.
Comment #10
eojthebraveComment #11
piyuesh23 CreditAttribution: piyuesh23 at QED42 commentedComment #12
piyuesh23 CreditAttribution: piyuesh23 at QED42 commentedUpdated Drupal core events with a namespace drupal.drupal, since they belong to drupal project inside drupal package. For core modules exposing events, the namespace as drupal.. Uploading patchfile.
Comment #13
piyuesh23 CreditAttribution: piyuesh23 at QED42 commentedComment #14
piyuesh23 CreditAttribution: piyuesh23 at QED42 commentedComment #15
jofitz CreditAttribution: jofitz at ComputerMinds commentedRemove "Needs re-roll" tag.
Comment #16
bander2 CreditAttribution: bander2 as a volunteer commentedLooks good.
Comment #17
tim.plunkettIf
Drupal\locale => drupal.locale.*
Drupal\migrate => drupal.migrate.*
Then
Drupal\Core => drupal.core.*
Even if drupal.drupal.* is partially correct, it doesn't match our use of 'core' other places, and it just looks silly.
Comment #18
piyuesh23 CreditAttribution: piyuesh23 at QED42 commented@tim, Thanks for the review. Uploading an updated patch here.
Comment #19
piyuesh23 CreditAttribution: piyuesh23 at QED42 commentedComment #20
Dinesh18 CreditAttribution: Dinesh18 as a volunteer commentedPlease provide interdiffs.
Comment #21
bander2 CreditAttribution: bander2 as a volunteer commentedThumbs up
Comment #22
larowlanThis is looking good.
I think we need a change record to go with this - It should spell out what the change is, and how contrib should follow suit.
It should also point out that if you have code that references the strings, it should be updated to reference the constants.
In addition, I think we need to update documentation in core.api.php that deals with events - see https://api.drupal.org/api/drupal/core%21core.api.php/group/events/8.2.x. It has a section that talks about unique event names.
Thanks