Trash and node modules cannot be installed at the same time on 11.x

This is happening because Trash has a config listener \Drupal\trash\EventSubscriber\TrashConfigSubscriber::onSave() that is triggered when trash.settings is saved an adds fields to any enabled entity types. At this point the node module has been enabled but is yet to be processed by the loop that processes the entity installations and simple config in config/install.

A core fix could be

• to prioritise modules that declare entity types over modules that don't - this feels hard and fragile.
• Install entity definitions for all modules and then simple config - not sure of the impact

Or we could move this to the trash module and use the container_rebuild_required flag.

This feels amazing but this has broken \Drupal\FunctionalJavascriptTests\Ajax\AjaxTest::testGlobalEvents() - I wonder how this has broken that and how that test is the only test of something in the module installer!!!!

#5 turned out to be unrelated - it was caused by a recent commit to HEAD - see 📌 Prefer to replace some of obj && obj.prop with optional chaining possibly Active

I've discussed this with @amateescu and we both think that this change is likely to make the move to multi-module install easier as it means that database schema will be correct before any modules start installing config.

Note that the trash module is now fixed due to 🐛 Fatal when entity type doesn't exist (anymore) Active being committed.

Note that the trash module is now fixed due to 🐛 Fatal when entity type doesn't exist (anymore) Active being committed.

What’s a good way to test this one? Or just follow steps to reproduce

NW for the tests, but admit I'm not super familiar with the trash module (didn't know of it will this ticket) but wonder if whatever it was doing could be copied in a core test module?

Added a test. The test works similarly to how trash does: there is a new test module with event subscriber for saving simple config provided by the module. When node and the test module are installed at the same time, the subscriber sets a flag in keyvalue for whether the node entity field table exists at the time of config save.

Going to lean on the test coverage for this one

1) Drupal\KernelTests\Core\Extension\ModuleInstallerTest::testEntityStorageInstalledBeforeSimpleConfig
Failed asserting that true is not true.
/builds/issue/drupal-3496558/core/tests/Drupal/KernelTests/Core/Extension/ModuleInstallerTest.php:278
FAILURES!

Looking at the changes I don't see anything off or needing update.

I'm not sure if this kind of change needs a CR but will let the committers decide.

Will go on a limb.

This looks good, nice that we're finding edge cases with the multi-install this far ahead of the 11.2 release.

Committed/pushed to 11.x, thanks!

catch → closed merge request !10740

Automatically closed - issue fixed for 2 weeks with no activity.