Layout builder cannot recover on missing layout

I think this would be better handled directly in LayoutBuilderEntityViewDisplay::getDefaultRegion()

Or even hasSection()...

Layout Builder can break in a lot of different ways if you remove the plugin from your system while still using it (though that's true of most things in PHP? I don't know that we want to babysit this too much...)

Patch #3 works as expected for me and solves the config:import issue on deployment after removing a layout that is now unused.
I can't see any issues so far.
So +1 on this! Very useful, thanks.

Regardless of implementation, this needs automated tests that follow the "steps to reproduce".

Just ran into this. Attached patch might be a subtle approach to not break when a layout is removed. Thoughts?

Sorry, ran into an error while saving as well, I guess the checks need to be just a bit more.

Reroll for 10.1.x

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 200, following Review a patch or merge require as a guide.

Moving back to NW for the test cases

Also tagging for IS update for the proposed solution and if any steps are different from D8 when this was reported.

Removing 'needs review' tag.

This is potentially related to https://www.drupal.org/project/drupal/issues/2860346

The definitions are cached during a theme install.

If a theme provides custom layouts and has configuration objects that use those layouts (like `entity_view_display`), the layouts plugin definitions will already be cached and newly installed layouts will not have a chance to be discovered during the process of the theme installation.

Adding a cache tag that depends on installed extensions resolve this issue.

In LayoutPluginManager:
$this->setCacheBackend($cache_backend, $type, ['config:core.extension']);

Patch attached.

For me #20 doesn't work.
I have a theme that is already installed, which declares some layouts, and some configs using those layouts (in config sync).
When I change a layout machine name and attempt to clear cache and config import, it fails.

#16 and #3 are working but a clear cache is required it seems before config import.

#20 worked for me on a fresh install with CivicTheme 1.6.2 on Drupal Core 10.2.3. Thanks!

I used patch #20 on Drupal 10.3.0 and CivicTheme 1.7.1 and the fatal error went away.

It would be great if someone wanted to review the different approach in #20, update the issue summary, and add tests :)

@joshua1234511 The test is failing in the pipeline. I tried it in the local by following the test steps where I could not find a message as in the test case. Is the test written as per requirement? Should the test be rewrorked on?

Reverted the added test. Mock tests are complex to integrate into the layout build scenario for scenario-based execution. Manual testing is required for this fix.

Going to disagree that we should have test coverage added but will leave in review.

This absolutely needs test coverage.

I haven't thoroughly looked into this issue yet, would it work to have a test module that provides a layout, add that layout to a page, then uninstall that module and verify everything still works as expected? If I get a chance I'll look into this.

For the tests

I've added test coverage, and also fixed issues where this wasn't working on the default layout or on nodes that did not have overridden layouts (Yay for tests)

Lets see how the pipeline goes here. I'm a little iffy of how many places we are calling ::hasLayout. I wonder if we can put that in a central place instead.

Test coverage added and tests are green. I'd be interested to get a second set up eyes on this one. Mainly around this comment.

I'm a little iffy of how many places we are calling ::hasLayout. I wonder if we can put that in a central place instead.

I get this error when installing the new Navigation module. (I'm not using any patches for this issue, just commenting that this is a way the issue occurs.) The error occurs for every single route I visit.

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "navigation_layout" plugin does not exist.

This issue is prohibiting me from converting my site to the new module. I'm currently on Drupal 10.4

I applied the MR diff as a patch to my Drupal 10.4 site, and I still get the error.

Per #36

Let me know if you need me to provide a stack trace or anything else.

Hi,

I made an addition in getLayoutSettings to avoid it to fail if the layout plugin does not exist.

This is helpful in case of update like for UI Patterns 1 to UI Patterns 2 upgrade.

Attaching patch for Composer usage.

MR rebased, attaching current snapshot.

Some performance tests are failing and affected by the added cache tag in LayoutPluginManager.
I tried to reproduce the issue mentioned in #20 and #35 but without success.
Could you please provide steps to reproduce?

I tried this in LayoutBuilderMissingLayoutTest, but it passes without the added cache tag.

  /**
   * Tests that installing the Navigation module does not cause a plugin error.
   */
  public function testInstallingNavigationModuleDoesNotBreakLayouts(): void {
    // Warm the layout definitions cache.
    /** @var \Drupal\Core\Layout\LayoutPluginManagerInterface $manager */
    $manager = \Drupal::service(LayoutPluginManagerInterface::class);
    $definitions = $manager->getDefinitions();
    $this->assertArrayNotHasKey('navigation_layout', $definitions);

    \Drupal::service('module_installer')->install(['navigation']);

    $user = $this->drupalCreateUser(admin: TRUE);
    $this->drupalLogin($user);
    $this->drupalGet('<front>');
    $this->assertSession()->statusCodeEquals(200);

    // Verify the layout is now properly discovered.
    $manager = \Drupal::service(LayoutPluginManagerInterface::class);
    $this->assertTrue($manager->hasDefinition('navigation_layout'));
  }