Improve documentation of hook_entity_embed_alter()

Problem/Motivation

In \Drupal\entity_embed\EntityHelperTrait::renderEntityEmbed():

    $this->moduleHandler()->alter(array("{$context['data-entity-type']}_embed", 'entity_embed'), $build, $entity, $context);
    $entity_output = $this->renderer()->render($build);

This triggered my instinctive oh, that's interesting, but … does it deal with cacheability metadata correctly? reaction.

Problem 1: the documentation doesn't say anything about cacheability, and how altering this affects also non-embedded rendering

It should say something like:

+ * Note that any alterations made here must update the cacheability metadata in
+ * $build: otherwise alterations made for embedding an entity in a certain view
+ * mode will also affect that entity in that view mode when not embedded.
+ *
+ * It is recommended to NOT use this hook and instead create embedding-specific
+ * view modes.

Because that is the problem: Entity Embed allows you to embed an entity using a certain view mode. Great! But this hook allows you to alter it. And if you do so without modifying #cache[keys], you end up with a race condition:

1. if entity X in view mode Y is rendered first outside of Entity Embed, then the Entity Embed filter will also render it like that, without the alterations made by this hook, because the CID will be the same
2. if entity X in view mode Y is rendered first by Entity Embed, then it will also be rendered like that outside of Entity Embed filter, with the alterations made by this hook, because the CID will be the same

Conclusion: it'd be better to not use this hook to alter existing view modes (entity view displays), but instead have site builders create custom view modes/entity view displays. Note that this does not mean you MUST do that. It's only necessary to do so if you want the embedded entities to be rendered differently than one of the existing view modes. So you can still choose to render the teaser view mode, for example.

Problem 2: hook_entity_embed_alter() does not operate on a fully built render array despite it claiming so

So I went to look and found this in entity_embed.api.php:

function hook_entity_embed_alter(array &$build, \Drupal\Core\Entity\EntityInterface $entity, array &$context) {
  // Remove the contextual links.
  if (isset($build['#contextual_links'])) {
    unset($build['#contextual_links']);
  }
}

This cannot work. Contextual links do not exist yet at this stage. So the example is broken.

The documentation says:

  * This hook is called after the content has been assembled in a structured
 * array and may be used for doing processing which requires that the complete
 * block content structure has been built.

This is wrong: the content structure has NOT yet been built at this stage (the whole point is that viewing an entity results in a very thin, very cheap render array, which uses a #pre_render callback to build it *fully*, and which is only called on a cache miss, so that on a cache hit, we do as little work as possible). And also, it's not a block, but an entity — but that's a nitpick :)

Conclusion: First of all this is broken — what this aims to do can only be done using hook_entity_view_alter(), which is called by EntityViewBuilder. But using that hook means you wouldn't be able to know that this entity render array is actually being rendered for embedding purposes, because hook_entity_view_alter() doesn't carry any contextual information. Nor could/should it do so, because this is only invoked after the entity's render array is fully built, and so it only happens on a cache miss, which means the CID was already known, which means modifying it only for the "embed" context would be wrong. So hook_entity_build_defaults_alter() would be better because that'd allow you to alter the cache keys, but there too you don't have that contextual information that this is an entity display being rendered for embedding.
And that is the whole point: given a certain entity display, it is always rendered in the same way. Entity Embed shouldn't allow manipulating that. Entity view modes/entity view displays are specifically designed to allow you to create different ones for different situations. Embedding is a different situation.

Proposed resolution

Either:

1. Document the limitations.
2. Remove hook_entity_embed_alter(), recommend creating custom view modes instead.

Remaining tasks

TBD.

User interface changes

None.

API changes

TBD.

Data model changes

None.