Layout Builder does not consult layout plugin for dependencies

+1

https://cgit.drupalcode.org/dynamic_layouts/tree has no code in it so I can't see how you are computing/storing the derivatives.

This should be fixed via the dependencies system within your plugin/deriver.

Then the only change needed in LB would be this (which we should do anyway).

No, I do not necessarily agree.
We should use the config dependency system correctly, and the UI where you change the layout derivatives should explain what you are about to affect.

This will add any layout's dependencies to the config. Your module should correctly declare its dependencies on the config entities that define it.

Note that this has to expand the method that is still slated to be removed in #2939925: Add a getPluginDependencies() method to \Drupal\Core\Plugin\PluginDependencyTrait

Rerolled now that the getPluginDependencies() is fixed.

Hi @Falco010 - I was looking through the Dynamic Layouts module and I think you would just need to add a dependency to the related Dynamic Layout config entity (https://cgit.drupalcode.org/dynamic_layouts/tree/src/Entity/DynamicLayou...).

You will not need to add dependencies for systems which use your layout like Panels and Display Suite. Your Layout Plugins are derived from Config Entities, so you need to make sure they're dependent on them so that the logic in this patch works.

I opened #2945083: Declare config dependencies in derived Layout Plugins to help fix Dynamic Layout

Keep in mind that you will need both that patch AND this patch

Ouchy, this is at least major and might be critical. Thanks @Falco010 for reporting this!

Marking as RTBC per my earlier review and #16.

+++ b/core/modules/layout_builder/src/Entity/LayoutBuilderEntityViewDisplay.php
@@ -215,6 +216,15 @@ public function onDependencyRemoval(array $dependencies) {
+      $layout_dependencies = $this->getPluginDependencies($section->getLayout());
+      $layout_removed_dependencies = $this->getPluginRemovedDependencies($layout_dependencies, $dependencies);
+      if ($layout_removed_dependencies) {
+        // @todo Allow the plugins to react to their dependency removal in
+        //   https://www.drupal.org/project/drupal/issues/2579743.
+        $this->removeSection($delta);
+        $changed = TRUE;
+      }
+
       foreach ($section->getComponents() as $uuid => $component) {

If we remove the entire section should we process it's components? Or should we process the section after the components. Not sure leaving at RTBC and pinging @tim.plunkett in slack.

There's no need to process the components after we remove their section.
Interdiff is generated with -w

Tagging as this is currently on our shortlist.

Since this missed the 8.5.0 ship we need an update path, unfortunately. Something like views_post_update_table_display_cache_max_age() to just update all the LayoutBuilderEntityViewDisplay entities. Sorry we didn't get this in earlier.

Here's an update path and update path test.

Missed @group

Slowly losing my mind. The PASS patch was correct, the FAIL was not.

This is identical to 27-PASS but reuploading so the bot is happy.

Update looks good. The issue of experimental commits during RC was discussed more by @catch & @xjm. At some point in the past we agreed that since experimental module are at a state less than RC we can continue to commit during a RC phase. So once this is RTBC I'll commit to both 8.5.x and 8.6.x.

The interdiffs since my last review look good, marking as RTBC.

Adding credit for patch reviews.

Committed and pushed 2678eabf6e to 8.6.x and bc6f172f55 to 8.5.x. Thanks!

Discussed with @catch and he was +1 to this going in to 8.5.x.

It looks like this broke HEAD on 8.5.x (but not 8.6.x) Reverting there. Also setting the branch correctly; please set it to the minimum branch that includes an issue.

Probably something else was not backported to 8.5.x and this depends on it.

The lack of a backport on #2939925: Add a getPluginDependencies() method to \Drupal\Core\Plugin\PluginDependencyTrait is what caused the fail.
However, that changes non-experimental code, so I'm not sure it's eligible for backport.
Here's a patch with the fix from there copied into the method that is still @todo'd for removal.

Interdiff looks good to me. RTBC assuming tests pass.

Eclipse

Let's leave #2939925: Add a getPluginDependencies() method to \Drupal\Core\Plugin\PluginDependencyTrait for 8.6.x it adds a method to a trait so it potentially disruptive.

Second time lucky. @tim.plunkett thanks for the quick turn-around. Committed ae5ffa9 and pushed to 8.5.x.