Introduce a new cache tags aware prop shape repository, so changes affecting prop shape calculation can force the re-invoke of hook_canvas_storable_shape_prop_alter

How will a cache miss due to cache tag invalidation cause recalculation of prop_field_definitions? Cache tags ensure cache miss, but nothing about prop_field_definitions works by a missed cached, it is explicitly calculated.

---

I'm not sure collecting this cacheable metadata will solve the problem. The `prop_field_definitions` is calculated inside of the component's config. This is calculated whenever the config entity is created or updated per

- \Drupal\canvas\Plugin\Canvas\ComponentSource\SingleDirectoryComponent::createConfigEntity
- \Drupal\canvas\Plugin\Canvas\ComponentSource\SingleDirectoryComponent::updateConfigEntity
- \Drupal\canvas\Plugin\Canvas\ComponentSource\JsComponent::createConfigEntity
- \Drupal\canvas\Plugin\Canvas\ComponentSource\JsComponent::updateConfigEntity

These invoke `\Drupal\canvas\Plugin\Canvas\ComponentSource\GeneratedFieldExplicitInputUxComponentSourceBase::getPropsForComponentPlugin` and update the `prop_field_definitions` settings which are passed as configuration to the component source plugin.

Canvas is regenerating SDC backed components in \Drupal\canvas\Plugin\ComponentPluginManager::setCachedDefinitions, but there's no way for us to push the cacheable metadata here. And the config changes happen after the cache is set, along with the fact `\Drupal\Core\Plugin\DefaultPluginManager::setCachedDefinitions` sets the cache tags based on the property for the class (okay, I guess we could dynamically alter that during processing.) But if we solved this for SDC's it wouldn't work for Code Components.

We just ran into this with testing out the Acquia DAM module support with image props. We had to re-save our Code Components so that a new version of the component was created that respected the new bundle. Technically all changes to the `prop_field_definitions` should trigger a new version and shouldn't be dynamically updated unless something re-saves/updates the component.

Now, that does make it pretty annoying if you add a new media type and it isn't usable, with no clear way to make it usable (need to trigger your component entity to be re-run with `updateConfigEntity` by its source.)

For transparency's sake - @mglaman asked me to look at this and validate his comment in #6. I have to admit I have the same question; at first blush, it is not clear to me how invalidating the cache tags of (let's say) media types will also trigger a cache tag invalidation for blocks and SDCs, although I could see a way (potentially) to rig that up by refactoring setCachedDefinitions() in both plugin managers.

But where does that leave code components? Those are just entities, so they're not centrally cached anywhere that I'm aware of. How can we possibly trigger an update for them if a new media type is created, apart from just a regular hook_media_type_insert() hook implementation?

If that's the vision, then it would be good to get explicit confirmation about that from Wim.

Yes we had the same issue with automatic regeneration in #3550334: [update path] [upstream] Text formatted with CKEditor within Canvas gets double escaped when output yesterday - i.e. the fact SDC component config entities can be resaved on a cold cache during discovery meant that update hook changes were applied during discovery rather than during the post update hook.

The same applies for code components.

I think yeah we need a hook_media_type_insert/hook_media_type_update here to flag that we need to regenerate code component config entities - we can query for JavascriptComponents with a prop of type `$ref: 'json-schema-definitions://canvas.module/image'` and resave them

I'll have a crack at that

i.e. the fact SDC component config entities can be resaved on a cold cache during discovery meant that update hook changes were applied during discovery rather than during the post update hook.

😱 I didn't even think about the fact UpdateKernel uses NullBackend so the plugin managers are always calling setCachedDefinitions and causing component churn, unless the update hooks mark config as syncing.

I think yeah we need a hook_media_type_insert/hook_media_type_update here to flag that we need to regenerate code component config entities - we can query for JavascriptComponents with a prop of type `$ref: 'json-schema-definitions://canvas.module/image'` and resave them

Makes sense to me. Let's tackle the common use case. We should also tackle video as well.

Working MR up https://git.drupalcode.org/project/canvas/-/merge_requests/298

Turns out hook_media_type_insert is too early, because at that point the source field doesn't yet exist. So used hook_field_config_insert and the equivalent update hook instead. Works just fine!

This doesn't cover any contrib-based source plugin that extends from \Drupal\canvas\Plugin\Canvas\ComponentSource\GeneratedFieldExplicitInputUxComponentSourceBase

But I think we can live with that.

😱 I didn't even think about the fact UpdateKernel uses NullBackend so the plugin managers are always calling setCachedDefinitions and causing component churn, unless the update hooks mark config as syncing.

We should consider opening a new issue to prevent this, e.g. by checking if we're in an update kernel in the component manager and block manager before we blindly save new entities - if this happens during database updates we're breaking the rules around what you can do in an update kernel vs what should wait for the post update hooks. Namely, using the Entity APIs is not supported in an update kernel until post update hooks.

Opened #3556327: Don't save `Component` config entities unnecessarily for #18

Need some feedback for validating the new approach.

Working on this in MR 145

I'm done for today, but I've moved the needle on MR 145. I've left @todos for breadcrumbs where things need work

Assigning to Wim for a sanity check of the current progress/approach before we go any further.

There is certainly more refactoring we can do here - such as

• #3526045-7: Settle on final name for `ComponentSource` plugins (`ComponentType`?) + expand their scope (discovery, maybe more?) but I think the current state fixes the stable blocker and we can work on further refactoring as the source plugin API solidifies.
• Saving less config entities when we get an invalidation by querying by the unique url schema for the prop shape that changed

Discussed this in a meeting and @penyaskito will pick up on this where @larowlan left off.

This needs a Issue summary update, and potential follow-ups for #27.

Blocker's committed.

The main reason we were trying to get this resolved before the 1.0.0 release is that it's making a breaking change to a hook that's documented in canvas.api.php. However, we can document in #3553397: Document our @internal API policy that this isn't a public API yet.

Also, someone could run into this bug when adding media types after installing Canvas. But, even though that's annoying, a full cache clear resolves it.

So, downgrading this from a stable blocker to a post-stable priority. This issue is close. We'll hopefully get it fixed next week and can release a 1.0.1 or 1.1.0 with it.

Re-titling. Issue summary was already updated.
This has tests. All follow-ups exist already.

Assigning to Wim as he is very familiar with this space and with the MR itself.

I'll create a follow-up to update /API.md

UPDATE: #3562890: Document PHP APIs in API.md

Another epic effort (+100 commits! +1007 −295!). This is a huge improvement overall (also in terms of performance), with a small penalty for code components vs SDCs on re-discovery, which we already have follow-ups to alleviate (e.g. #3561272: Add the ability for ComponentSourceManager to generate a Component config entity for a single source, or even a single specific component).

As this will trigger _less_ rediscoveries, this is an improvement over-all!

Published the 3 change records.

🎉🚀