Field, Theme or module uninstalled

But all display with a component from this theme will be deleted (Page layout, entity view, views).

Maybe we need to implement ConfigEntityInterface::onDependencyRemoval():

This method allows configuration entities to remove dependencies instead of being deleted themselves. Configuration entities can use this method to avoid being unnecessarily deleted during an extension uninstallation. For example, entity displays remove references to widgets and formatters if the plugin that supplies them depends on a module that is being uninstalled.

Thanks, i will have a look

Hello,

Thanks for your proposal and sorry for my late answer (i was in a Drupal event in Greece).

So, first what I understand. You have used ConfigEntityInterface::onDependencyRemoval() on 2 config entities:

• EntityView: on EntityViewDisplay config entity
• PageLayout: on PageLayout config entity

There, you call the newly added DisplayBuildableInterface::removeDependency() method, implemented in DisplayBuildablePluginBase. That's nice to centralize the logic in the plugin like that.

Also, I believe it was a good decision to add a ::removeDependency() method instead of implementing RemovableDependentPluginInterface::onCollectionDependencyRemoval() because the "buildable" plugins are not attached to PageLayout and EntityView displays as "plugin collections".

Feedbacks:

• Most of the logic resides in src/SourceTree.php. SourceTree has been created to isolate the tree alteration logic from the Drupal logic, it must stay very agnostic of what is happening CMS side. Can we do differently? (i am asking but i honestly have no clue)
• EntityViewOverride: is not covered, and that's OK, I guess, because i am not sure it is possible for content entities and it belongs to UI Patterns scope anyway
• ViewDisplay: a specific removeDependency() method has been implemented. How is it triggered?

Hi pdureau ,
Thanks for your review:

Most of the logic resides in src/SourceTree.php. SourceTree has been created to isolate the tree alteration logic from the Drupal logic, it must stay very agnostic of what is happening CMS side. Can we do differently? (i am asking but i honestly have no clue)
EntityViewOverride: is not covered, and that's OK, I guess, because i am not sure it is possible for content entities and it belongs to UI Patterns scope anyway
ViewDisplay: a specific removeDependency() method has been implemented. How is it triggered?

• 1. SourceTree has all nodes with flat structure , maybe is the best place to make change
• 2. is not covered
• 3. it's not triggered indeed and i have an idea not too good:
  1. DisplayBuilderArea (area plugin) — A Views area handler (#[ViewsArea("display_builder_area")]) implementing DependentWithRemovalPluginInterface. It renders nothing; it's a sentinel that lives in the View's header section only when Display Builder is enabled for that display.
  2. display_builder_views.views.inc — hook_views_data() registers the area handler in $data['views']['display_builder_area'] so ViewsHandlerManager::getHandler() can find it. Without this, Views falls back to a broken handler.
  3. DisplayExtender::submitOptionsForm() — When a user enables Display Builder on a display, it calls addDisplayBuilderArea(); when disabling, removeDisplayBuilderArea().
  4. addDisplayBuilderArea() — Calls $this->displayHandler->setOverride('header', FALSE) to mark the header as overridden (otherwise getOption('header') inherits from the default display and ignores our area). Then adds the area handler config via setOption('header', ...).
  5. removeDisplayBuilderArea() — Checks isDefaulted() per section first — skips if inherited from default. Otherwise removes the area from header/footer/empty.
  6. Dependency handling via the area plugin:
  - calculateDependencies() → Gets ViewDisplay buildable from $this->displayHandler->getExtenders(), iterates getSources(), calls SourcePluginManager::getSource() on each, merges source dependencies into the View.
  - onDependencyRemoval() → Calls ViewDisplay::removeDependency() which runs SourceTree::onDependencyRemoval() to prune orphaned source nodes.

resolved all threads

So is it back to review?

Can't find a good way to handle views dependencies.

MR !269 is 128 commits behind 1.0.x and no longer merges: it conflicts in ViewDisplay.php, since the sources storage moved to ConfigBuildablePluginBase and saveSources() is gone from that class.

I rebased it locally. ViewDisplay::removeDependency() now uses saveDefaultSources() instead of writing to the view storage by hand. The existing SourceTree, EntityViewDisplay, PageLayout and DisplayExtender kernel tests pass. The original MR did not ship tests of its own, so I will add them (entity view, page layout and views, plus a node with children in slots).

To avoid rewriting @ipumpkin's history I will push it to a separate branch of the fork (3562750-rebase) unless you would rather have a force push on the MR branch.

Update on MR !462 (the rebase of !269 on 1.0.x, on a separate branch to keep @ipumpkin's history intact). Kernel tests are in for SourceTree, entity view, page layout and views, including a real theme uninstall and a field deletion. They exposed three flaws in the original approach, now fixed: calculateDependencies() read stale stored sources, so deleting a field still deleted the display; source plugins were created without contexts, which is a fatal error with entity_field sources; and getPluginRemovedDependencies() read an unchecked array key.

Checked on a real site by uninstalling a module that provides a component used by an entity view display: on 1.0.x the display is deleted; with this MR it is kept and only loses the dependent source. php-cs-fixer style is applied and the pipeline is green.

Still open: Views. View has no onDependencyRemoval() here, so uninstalling something does not prune a view's sources; only ViewDisplay::removeDependency() is covered, by a direct call. @pdureau, one design question: when a node with a broken dependency is removed, its slot children go with it even if they are independent. Should they be kept?

I applied the review from Oct 5 (commit 2f01763) and fixed the PHPStan errors in 0c6284a.

Changes from the review
1. Translation leak: PageLayout::onDependencyRemoval() and EntityViewDisplayTrait::onDependencyRemoval() now prune a SourceTree built from the entity's in-memory sources instead of going through the buildable. The URL language override can no longer end up in the default config. I added testDependencyRemovalInGermanKeepsDefaultConfigUntranslated to PageLayoutMultilingualPublishTest and EntityViewMultilingualPublishTest. It fails on the previous code ('Hallo' instead of 'Hello') and passes now.
2. Removed DisplayBuildableInterface::removeDependency() and its implementation in DisplayBuildablePluginBase.
3. Removed ViewDisplay::removeDependency() and its tests. Views is left for a follow-up.
4. Draft sources are still not pruned (follow-up).
5. Slot children: behavior is unchanged and now documented in the SourceTree::onDependencyRemoval() docblock.
6. Cleanups: simpler return in SourceTree, added @see to getPluginRemovedDependencies(), and the removed() helper now lives in a shared trait (DependencyRemovalTrait).

Pipeline
- The 5 PHPStan errors were pre-existing, in code this MR doesn't touch (assign.overwritten and assign.unused). I fixed them in a separate commit (0c6284a) by removing the dead assignments, with no behavior change. If you prefer to keep them out of this MR, that commit can be dropped without affecting the rest.
- phpunit still fails on 9 Views tests (ViewsSourceOptionsTest and ViewsSourceRenderTest). They were failing before this MR and are unrelated to it. No SourceTree, entity_view or page_layout test fails.

Note: LayoutSource::calculateDependencies() had a line that looked meant to record the layout_discovery dependency but never reached $dependencies. I removed the dead line without changing behavior. If the intent was to emit that dependency, it should go in a separate issue.

Thanks Andres for the update.

However, I am not able to do a proper review for 2 reasons:

• There are a few phpunit fails in Drupal\Tests\display_builder_views\Kernel\ViewsSourceOptions, can you check them locally?
• There are some merge conflicts, can you rebase your branch?

Oops, sorry—I didn't check the PHPUnit code. I'll fix it right away.

Thanks. I guess this review ill be easier than the previous ones because the changes are now located in only 3 places:

• EntityViewDisplayTrait::onDependencyRemoval()
• PageLayout::onDependencyRemoval()
• SourceTree::onDependencyRemoval()

✅ Tested and works — on 1.0.x, a Basic page view display whose Display Builder sources used a string field was listed under "Configuration deletions" on the field's delete confirm page; after applying MR !462 and rebuilding caches, the same page lists it only under "Configuration updates", and deleting the field kept the display with only the textfield source left in its stored sources. Field deletion no longer deletes the display.

Shipped in beta10 release.