Add prepareBuild step similar to entityprepareView

I'd need a bit more precise concept to address this issue.

I see there is \Drupal\Core\Field\FormatterInterface::prepareView that is then mostly used on entity reference field formatters and it has the same doccomment as prepareBuild from custom elements module.

I'm not sure what is meant by entity-prepare-view step.

We need to add functionality that mirrors hook_entity_prepare_view - as invoked for traditional renderring in \Drupal\Core\Entity\EntityViewBuilder::buildComponents

This is mostly (if not only) used to make enttity-loading more efficient on the typical use-case of rendering 100 node teasers. Consider each of them needs to show information of a referenced entity, like the node author. If we do a single entity load for each of the nodes it results in 100-load queries what hurts performane a lot. Instead, a single entity-load for all node authors can be done, which are then cached and the single load will hit the caches.

This works via \Drupal\Core\Field\FormatterInterface::prepareView which is implemened by \Drupal\Core\Field\Plugin\Field\FieldFormatter\EntityReferenceFormatterBase::prepareView.

Thus we should:
a) provide the API for efficient building of multiple entities at once. We have only \Drupal\custom_elements\CustomElementGenerator::buildEntityContent - we need to introduce buildMultipleEntityContent.
b) We need to introduce the prepareBuild step and invoke our formatters prepareBuild method with it + implement it in our Entity-Reference CE-Field-Formatters like it is implemented EntityReferenceFormatterBase. This means we need to rewrite some of our building logic to work with multiple entities at once, such that we can invoke that hook properly.
c) related, it would be nice to add some hooks like hook_custom_elements_entity_build and hook_custom_elements_entity_prepare_build - but that seems like a dedicated issue we should probably better add as follow-up, so we can focus on making good hooks there. (no stable blocker)

Thank you for the details. This is as I had expected - though I had not checked it with you -- and I would not have known to look in EntityReferenceFormatterBase::prepareView() for the partial-entity-loading code.

Working on this.

This is a first request for review, to see if we agree on the new call structure:

• There are now two split-up 'multiple' phases, and they are now called from the one caller that would potentially benefit from speedups
• but I haven't added the code in our Entity-Reference CE-Field-Formatters yet, that would provide actual speedup. (I am stopping for the day. Potentially this could also be done in a followup.)

Relevant public methods in CustomElementsGenerator:

• generate(ContentEntityInterface $entity, string $viewMode, string $langcode = NULL)
  • I think this needs a 'multiple' version.
• buildEntityContent(ContentEntityInterface $entity, CustomElement $element, $viewMode)
  • did not exist in 2.x branch. Isn't called from 'external' classes yet.
  • doesn't do much, nowadays: just creates a CE display and calls buildCeDisplay()
• buildCeDisplay(ContentEntityInterface $entity, CustomElement $custom_element, EntityCeDisplayInterface $display, string $viewMode, AccountInterface $account = NULL)
  • did not exist in 2.x branch. Isn't called from 'external' classes yet.
  • contains the logic we want to split up.
  • has an extra $account argument.

I believe that we can

• add generateMultiple(); generate() and generateMultiple() are going to remain the 'simple' entry points.
• deprecate both buildEntityContent() and buildCeDisplay()
• add for the 'advanced' callers (and call from within generateMultiple()):
  • prepareBuildEntities(array $entities, string $viewMode, AccountInterface $account = NULL): EntityCeDisplay[]
  • buildEntities(array $entities, array $custom_elements, string $viewMode, AccountInterface $account = NULL): void

thx. I reviewed the PR and it seems very solid already. I only added a couple of rather small remarks.

If we filterEmptyItems() here in prepare(), then we should also filterEmptyItems() in the next step / the existing buildCeDisplay() code.

When I recall things correctly, this should be fine. filterEmptyItems is removing those items from the $entity, so they will be gone and there is no need to call it again in a later stage. This is necessary since some edit-workflows / widgets might cause some temporary empty items, like an empty textfield line, be added. This won't be there after loading/saving anyway, so this is good for consistency. But doing it once should be exactly right.

I now feel like

merging the code from this method and buildEntities() into a single method (so I also don't need to call getEntityCeDisplaysByBundle() twice)
adding $account as an extra optional parameter to generateMultiple(), and only keeping generate() + generateMultiple() as a public entry point.

Agreed, this would be perfect. Let's do this, and merge it after that final refactoring!

Regarding those deprecated methods, I'd even vote for removing them. We are in a very short beta phase and no one will be using those APIs which where around for a couple of weeks only + marked beta anyway. Let's better gain more simplicity by having less methods flying around that we don't need. Let's simply drop it. We could create a change-record instead, but I'd be even ok without it. This is new enough imo.

So setting nw for the suggested final step.

This took longer than I expected. Yesterday's code wasn't good, because it was also prepare()ing entities that don't even go through field-based processing. New one is up now; the single added method is... long, but IMHO readable.

specifically on the one comment I left, for the specification/use of the $langcode method parameter. I trust the rest of the code enough to commit.

I have created a followup for actually using this in entityreference formatters etc. (If it's just a question of doing a loadMultiple, then fine - but the example code contains access/caching magic that I don't want to get stuck in, before handling some other priority issues.)

I created a draft CR for the removed methods. (They only existed for a few months, but it's some record at least.)

thanks, that seems reasonable!

>Request for review: specifically on the one comment I left, for the specification/use of the $langcode method parameter. I trust the rest of the code enough to commit.

Yes, that special empty string option is a bit weird. But moreover it's something we are inventing over what core does typically.

Instead, I'D suggest the following:
* offer two methods, one which we call generate() and works as the one in the MR, minus the empty string langcode thing.
* another one, which we call buildEntity() and buildEntityMultiple(), or buildEntityContent() which works like generation, minus the translation context.

Imo, this terminology introduced by the CE-formatters of "Builds an entity's content as custom element." is fine and should apply to the whole render-process, i.e. including layout builder. Thus, it should include everything, minus the language-negotation. Then we have that re-usable utility also + shortened the code.

The same way, I'd find it logical that the ce-prepare-build hook and a likewise ce-entity-build hook fires in every case of our 3 rendering variants (force-auto, layout-builder, formatters).

However, no strong feelings. If you prefer, we could also move on with the current code.

* offer two methods, one which we call generate() and works as the one in the MR, minus the empty string langcode thing.
* another one, which we call buildEntity() and buildEntityMultiple(), or buildEntityContent() which works like generation, minus the translation context.

I now implemented this:

• generate() - works as before (does language translation) except also has $account
• generateMultiple() - works as previous commit, minus the empty language string (always translates)
• buildEntityContent() - works on multiple entities, does not translate

There's no "single" equivalent of buildEntityContent() ; imo it woud not make things easier to understand (or code).

Imo, this terminology introduced by the CE-formatters of "Builds an entity's content as custom element." is fine and should apply to the whole render-process, i.e. including layout builder. Thus, it should include everything, minus the language-negotation. Then we have that re-usable utility also + shortened the code.

The same way, I'd find it logical that the ce-prepare-build hook and a likewise ce-entity-build hook fires in every case of our 3 rendering variants (force-auto, layout-builder, formatters).

Yes. This last thing was already the case. Hence the large size of the outer foreach()es.

But I've 1) split out some code to make the flow a bit better, 2) implemented your suggestion to make 'force-auto' a more distinct piece of code.

> Yes. This was already the case -- if we take the ce-entity-build hook to mean the existing "alter". (For the rest, the hook stuff was split out into a followup per an earlier suggestion.)

usually, there is a both a regular hook which you can use for adding things, and then an alter hook. But introducing that hook is out of scope for this one here, we can add it in another issue any time.

>generateMultiple() - works as previous commit, minus the empty language string (always translates). This one is needed; the PR implements it. (Unless you tell me explicitly that this part does not need entity translation;

No, this is fine and exactly as I'd have seen it needed also.

> There's no "single" equivalent of buildEntityContent() ; imo it woud not make things easier to understand (or code).

ok, fine with me!

Thank you for improve the MR - with the helper methods the overall logic is now much easier to follow. I think generally this is great now and good to go. However I found a small issue/left-over in docs/comments which should be fixed before merge. - so nw for that

Thank you for checking! I again got sloppy toward the end.

Merged. will follow up on the entityreference implementation soonish.

Automatically closed - issue fixed for 2 weeks with no activity.