GitLab

Overview

Symfony 4.3 changed the contract for dispatch(): https://symfony.com/blog/new-in-symfony-4-3-simpler-event-dispatching Consequently, on systems running Symfony 4.3 or newer, there are warnings about using the old contract.

Loosely, they swapped the order of the parameters ($event<=>$eventName), made $eventName optional, and (for the transition-period of 4.3+) allowed the parameters to be given in either order. For context, the signature evolution is roughly:

public function dispatch(string $eventName, Event $event = null);                 // 2.x, 3.x, 4.0 - 4.2
public function dispatch(string|object $event, string|object $eventName = null);  // 4.3+
public function dispatch(object $event, string $eventName = null);                // 5.x

cc @seamuslee

Reproduction steps

Go to https://test.civicrm.org/job/CiviCRM-E2E-Matrix/ and view test results for drupal9-clean w/Civi 5.33 or 5.34

Current behaviour

When running E2E tests on Drupal 9, the test completes with a warning:

10x: Calling the "Symfony\Component\EventDispatcher\EventDispatcherInterface::dispatch()" 
method with the event name as the first argument is deprecated since Symfony 4.3, 
pass it as the second argument and provide the event object as the first argument instead.

Expected behaviour

No warnings... But the real question is how to achieve that. See "Comments".

Environment information

• CiviCRM: 5.33
• PHP: 7.3
• CMS: D9

Comments

(A) Merely writing code to follow the new contract is not a problem. The main problem is the dependency-hell aspect -- e.g. one is likely to get squeezed between D8 (symfony/event-dispatcher 2.x-3.x), D9 (symfony/event-dispatcher 4.x), and D10 (presumed 5.x or later). The warning is telling you to prepare for Symfony 5.x. But doing so requires that you break away from Symfony 3.x. Put another way: You cannot fix the warnings on Civi-D9 without breaking Civi-D8.

(B) Here's an interesting tidbit: from the POV of a subclass of EventDispatcher, the Symfony 4.2=>4.3 update is a contract break. Implementations of EventDispatcherInterface which target 2.0-4.2 (e.g. Civi) should not be used by consumers which target 4.3+ (e.g. D9). However, to date, we have avoided any functional problem because (a) on a substantive level, the Civi and Drupal dispatchers are de facto separate subsystems with different contracts/audiences and (b) on a syntactic level, the 4.2/4.3 contracts are close enough to avoid hard compilation error.

(C) I think we have a few basic options:

• Hard D8=>D10 Switch: Ignore or discard the warnings for immediate future. Kill D8/S3 support sometime before adding D10/S5 support. Here are a couple variants on that:
  • Wait...Wait...Go!: Drupal 8 support is slated to end in November 2021. In theory, we could stop supporting Civi-D8/S3 for new releases after Nov 2021 -- and then begin the transition to the new contract. But we'd probably want to move this along quickly, since D10 is slated for 2022.
  • Major Version Alignment: This could potentially be made easier-to-communicate if we do a major-version increment on Civi and setup version-alignment in the same way that Drupal/Symfony are doing (e.g. S3/D8/Cv5; S4/D9/Cv6; S5/D10/Cv7).
• Break away from symfony/event-dispatcher: CiviEventDispatcher would not (going forward, on S4+ builds) directly extend symfony/event-dispatcher classes. Instead, do one of these:
  • Hard fork: Copy from symfony/event-dispatcher circa 4.2 and build out parallel classes with different namespace where we can control contracts/scheduling
  • Mapped namespace/Soft fork: Update our build system (e.g. composer plugins) to allow multiple/coexisting versions of Symfony packages. These technically use the same code as symfony/event-dispatcher, but it's automatically rewritten to non-conflicting namespaces (along the lines of https://github.com/humbug/php-scoper).
  • Change to PSR-14: A standard is meant to be more stable than an implementation. The problem here is the substance of the PSR-14 contract -- it's disagreeable with all of Civi's existing events, including/especially hook_* and GenericHookEvent.