EVA views pollute the whole page with the view's cache contexts - should use a lazy builder

Rebased.

I can't seem to get this patch to apply on 3.0.x-dev or 3.0.0

https://git.drupalcode.org/project/eva/-/merge_requests/1.diff

It gets rejected on the module file.

Cheers Dan

I've rebased the branch - the diff will apply now.

It's still not applying for me on 3.0.0 or 3.0-dev.x

It seems that it rejects

@@ -50,63 +50,55 @@ function eva_entity_extra_field_info()

In the module file this line is actually line 51.

Ah, the MR was against the wrong branch.

Sorry my fault, I realised this was because I was using another patch that is modifying the module file
https://www.drupal.org/project/eva/issues/3083993 ✨ Hide output if empty option Fixed

hi @joachim
I've updated MR to make it work with 3.1 version.
could you pls change target branch in MR to 3.1.x ?
tnx

I've rebased the branch on 3.1.x, rather than have a merge commit. In general, rebasing produces a cleaner history for a feature branch.

Though now I revisit the code, I'm not sure about this bit:

              '#cache' => [
                'contexts' => $view_cache_metadata->getCacheContexts(),
                'max-age' => $view_cache_metadata->getCacheMaxAge(),
                'tags' => $view_cache_metadata->getCacheTags(),
              ],

Surely that's polluting the outer render array -- it should be inside instead?

Applying the patch from the latest MR gives me the following error. (Drupal 10.3)

Error: Class "Drupal\eva\View" not found in Drupal\eva\LazyBuilders->displayView() (line 90 of modules/contrib/eva/src/LazyBuilders.php).

Looks like a missing class import.

matej.lehotsky → made their first commit to this issue’s fork.

Great stuff!

Why is there no pipeline running for the MR?
Can someone rebase to current HEAD and force-push? Maybe that's it...

I've brought the MR up to date. Looks like there are some code sniffer issues in the new class if anyone wants to fix, but not a dealbreaker (for me, anyway).

FILE: /builds/project/eva/web/modules/custom/eva/src/LazyBuilders.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
--------------------------------------------------------------------------------
 31 | WARNING | \Drupal calls should be avoided in classes, use dependency
    |         | injection instead
    |         | (DrupalPractice.Objects.GlobalDrupal.GlobalDrupal)
 42 | WARNING | \Drupal calls should be avoided in classes, use dependency
    |         | injection instead
    |         | (DrupalPractice.Objects.GlobalDrupal.GlobalDrupal)
 93 | WARNING | Line exceeds 80 characters; contains 81 characters
    |         | (Drupal.Files.LineLength.TooLong)
 95 | WARNING | Line exceeds 80 characters; contains 81 characters
    |         | (Drupal.Files.LineLength.TooLong)

I'm looking for testing from the community so if you're able to patch this on your site and confirm functionality, please set RTBC so we can merge!

Fixing services dependency injection is straightforward:

in eva.services.yml:

  eva.token_handler:
    class: Drupal\eva\TokenHandler
    arguments: ['@token']
  Drupal\eva\TokenHandler: '@eva.token_handler'
  eva.lazy_builders:
    class: Drupal\eva\LazyBuilders
    autowire: true

in LazyBuilder.php:

...
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\eva\TokenHandler;
...
  /**
   * The entity type manager.
   *
   * @var \Drupal\Core\Entity\EntityTypeManager
   */
  protected $entity_type_manager;

  /**
   * The token handler.
   *
   * @var \Drupal\eva\TokenHandler
   */
  protected $token_handler;
  public function __construct(
    EntityTypeManagerInterface $entity_type_manager,
    TokenHandler $token_handler
) {
    $this->entity_type_manager = $entity_type_manager;
    $this->token_handler = $token_handler;
  }

You can then switch your \Drupal::service calls out for the appropriate $this-> call.

There is however an issue I ran into in testing that I am struggling to resolve. When using Layout Builder and the block_content module (both core), new blocks that have an EVA field on them cause a WSOD. This is because prior to saving layout, the custom block entities exist in a limbo state, where they have no ID and thus cannot be loaded from entity. This breaks the entity load in displayView().

Unfortunately, passing the entity itself from eva_entity_view() to the LazyBuilder isn't possible, so I'm testing out passing values for spot-recreation instead. Kind of clunky, but that's the only thing I can think of.

> This is because prior to saving layout, the custom block entities exist in a limbo state, where they have no ID and thus cannot be loaded from entity. This breaks the entity load in displayView().

I've run into this problem elsewhere. There are two options:

- bypass the lazy builder if there is no entity ID and render directly. We know that we're not in a main rendering situation, so the performance hit is fine
- don't render anything if there is no entity ID

Experimentally:

eva.module:

function eva_entity_view(array &$build, EntityInterface $entity, EntityViewDisplayInterface $display, $view_mode) {
  $type = $entity->getEntityTypeId();
  $views = \Drupal::service('eva.view_displays')->get($type);

  foreach ($views as $info) {
    $longname = $info['name'] . '_' . $info['display'];

    if ($display->getComponent($longname)) {
      if ($view = Views::getView($info['name'])) {
        if ((empty($info['bundles']) || in_array($display->getTargetBundle(), $info['bundles'])) && $view->access($info['display'])) {
          $view->setDisplay($info['display']);

          // Hand over to lazy builders, so that the cache contexts of the view
          // don't bubble to the whole page if they would make it uncacheable.
          $view_cache_metadata = $view->display_handler->getCacheMetadata();

          if ($info['uses exposed'] && $display->getComponent($longname . '_form')) {
            $build[$longname . '_form'] = [
              '#lazy_builder' => [
                'eva.lazy_builders:displayView',
                [
                  $entity->getEntityTypeId(),
                  $entity->id(),
                  $entity->isNew(),
                  json_encode($entity->toArray()),
                  $info['name'],
                  $info['display'],
                  TRUE,
                ],
              ],
              '#cache' => [
                'contexts' => $view_cache_metadata->getCacheContexts(),
                'max-age' => $view_cache_metadata->getCacheMaxAge(),
                'tags' => $view_cache_metadata->getCacheTags(),
              ],
            ];
          }

          $build[$longname] = [
            '#lazy_builder' => [
              'eva.lazy_builders:displayView',
              [
                $entity->getEntityTypeId(),
                $entity->id(),
                $entity->isNew(),
                json_encode($entity->toArray()),
                $info['name'],
                $info['display'],
              ],
            ],
            '#cache' => [
              'contexts' => $view_cache_metadata->getCacheContexts(),
              'max-age' => $view_cache_metadata->getCacheMaxAge(),
              'tags' => $view_cache_metadata->getCacheTags(),
            ],
          ];
        }
      }
    }
  }
}

LazyBuilders.php:

/**
   * Builds either the view or the view exposed form for the attached view.
   *
   * @param string $entity_type_id
   *   The entity type ID of the attaching entity.
   * @param string $entity_id
   *   The ID of the attaching entity.
   * @param bool $entityIsNew
   *  Whether the entity is new.
   * @param string $entity_values
   *  The values of the entity.
   * @param string $view_id
   *   The ID of the attached view.
   * @param string $view_display_name
   *   The name of the view display to show.
   * @param bool $show_exposed_form
   *   (optional) Whether to show the exposed form, or the rest of the view.
   *   Defaults to FALSE.
   */
  public function displayView($entity_type_id, $entity_id, $entityIsNew, $entity_values, $view_id, $view_display_name, $show_exposed_form = FALSE) {
    $entity_storage = $this->entity_type_manager->getStorage($entity_type_id);
    $entity = $entityIsNew ? $entity_storage->create(json_decode($entity_values,JSON_OBJECT_AS_ARRAY)) : $entity_storage->load($entity_id);
...

This seems to correct the issue I was seeing, but no idea on any other implications past my corner case.

Putting the whole seriali

yeah, even as I did it it felt wrong LOL. More just an experiment to see if it would work, definitely not advocating it now in the cold light of morning. Bypassing the lazybuilder if it is unsaved is definitely the better way to go.