Problem/Motivation
When subscribing to events of the Scaffold plugin from other composer packages, the dispatched events are not received by listeners, because the event dispatcher is initialised as a new instance rather than re-using an existing instance coming from composer. This makes it "forget" all the listeners collected by composer.
Current code:
$dispatcher = new EventDispatcher($this->composer, $this->io);
$dispatcher->dispatch(self::PRE_DRUPAL_SCAFFOLD_CMD);
Fixed code:
$dispatcher = $this->composer->getEventDispatcher();
$dispatcher->dispatch(self::PRE_DRUPAL_SCAFFOLD_CMD);
Steps to reproduce
1. Create a plugin class with the following code
<?php
namespace YourNamespace\Composer\Plugin\YourPlugin;
use Composer\Composer;
use Composer\EventDispatcher\Event;
use Composer\EventDispatcher\EventSubscriberInterface;
use Composer\IO\IOInterface;
use Composer\Plugin\PluginInterface;
/**
* Composer plugin for handling drupal scaffold.
*
* @internal
*/
class Plugin implements PluginInterface, EventSubscriberInterface {
/**
* {@inheritdoc}
*/
public function activate(Composer $composer, IOInterface $io) {
}
/**
* {@inheritdoc}
*/
public function deactivate(Composer $composer, IOInterface $io) {
}
/**
* {@inheritdoc}
*/
public function uninstall(Composer $composer, IOInterface $io) {
}
/**
* {@inheritdoc}
*/
public static function getSubscribedEvents() {
return [
'pre-drupal-scaffold-cmd' => 'preDrupalScaffoldCmd',
'post-drupal-scaffold-cmd' => 'postDrupalScaffoldCmd',
];
}
public static function preDrupalScaffoldCmd(Event $event) {
print 'preDrupalScaffoldCmd' . PHP_EOL;
}
public static function postDrupalScaffoldCmd(Event $event) {
print 'postDrupalScaffoldCmd' . PHP_EOL;
}
}
2. Create a package with the following `composer.json` and place the file with Plugin class into `src` directory and provide references:
{
"name": "yournamespace/yourplugin",
"type": "composer-plugin",
"require": {
"php": ">=8.2",
"composer-plugin-api": "^2"
},
"autoload": {
"psr-4": {
"YourNamespace\\Composer\\Plugin\\YourPlugin\\": "src"
}
},
"extra": {
"class": "YourNamespace\\Composer\\Plugin\\YourPlugin\\Plugin"
}
}
3. Create a consumer website package:
composer create-project drupal-composer/drupal-project:10.x-dev some-dir --no-interaction
4. Add your custom package as a dependency to the newly created project.
5. Run `composer drupal:scaffold`
6. Observe that plugin's listeners were ignored.
Proposed resolution
Use the event dispatcher from provided by composer:
$dispatcher = $this->composer->getEventDispatcher();
Remaining tasks
1. Update event dispatcher.
2. Add tests. Note that there are currently no tests for this functionality, so initial tests would need to be added as well.
User interface changes
None
API changes
None
Data model changes
None
Release notes snippet
None
| Comment | File | Size | Author |
|---|
Issue fork drupal-3438034
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
alex.skrypnykComment #5
adwivedi008 commentedHi @alex.skrypnyk
Thanks for raising the issue and proposing solution
Implemented the proposed solution and created MR
It needs review so moving the issue to needs review
Comment #6
tannguyenhn commentedComment #8
tannguyenhn commentedHi @alex
I have provided the patch to proof the bug and MR to fix the bug.
Comment #10
alex.skrypnykLooks like MR has passed.
The code change in the scaffold plugin works for me.
Waiting for the core team to check the code.
Also wondering if this can be cherry-picked into 10.x branch as this plugin is not specific to a Drupal version.
Comment #11
alexpottAssigning issue credit.
Comment #12
alex.skrypnykComment #13
alexpottThe title doesn't make sense to me.
Comment #14
alexpottLeft a comment on the MR... just need to move some test code around so we're running the tests in ComposerHookTest twice.
Comment #15
alex.skrypnykthanks for review @alexpott
we are working on the update to the MR.
Comment #16
alex.skrypnykComment #17
greg.1.anderson commentedPatch looks good to me -- thanks for finding this and submitting a test. It definitely should have been like this from the beginning.
Comment #18
longwaveYep this looks correct to me too, thanks for adding the test coverage - confirmed that without the change that the new test fails because the listener is not discovered.
Comment #19
longwaveComment #20
alexpottCommitted and pushed f35f0419c7 to 11.x and 2fc0e9b818 to 10.3.x and 46c3c5d946 to 10.2.x. Thanks!