Add automated image optimization to image component

grasmash → closed merge request !821

Changing tag from "sprint" to "spike", because we're not planning on completing this issue this sprint, we're just trying to get a handle on what Claude Code prompted by @grasmash created here.

Why is this re-implementing responsive images and image styles almost from scratch?

This desperately needs an issue summary update with the following:

1. What is the requirement for XB?

2. How far does core's existing image style and responsive image support go towards meeting #1. If there are gaps, what are the specific gaps and do they already have existing issues - and is it technical capabiities, usability etc.

Responding to the new issue summary (thanks for writing it up):

Image styles and responsive images are configured in config. This means that using the existing system requires introducing dependencies on the image style and responsive image config entities. This goes against the SDC principle where everything a component needs is supposed to be contained by the component.

The SDC principle is that it should receive the props, but the responsive image style config itself doesn't need to be a prop - instead the SDC component could receive the image URLs + dimensions + alt text, which can be built in advance based on the data stored in Experience Builder itself and passed in - just enough information to generate the img + srcset (or picture element) from.

Since the URL or URLs contain all the necessary information for the image style callback to render the image, the SDC doesn't need to know anything about anything else - it can render the markup/attributes based on what's passed in.

At the moment, any component library would have to re-implement all the various configuration options in the SDC added here in order to support responsive images, whereas really they should just be able to take the minimum set of information to build the markup.

Unless the only way to render a responsive image in XB is going to be nesting this component in a slot? Also what happens if a component wants to accept either a responsive image or an external one (which iirc is a requirement)?

The current implementation is shoe-horning half a dozen configuration options into the component, including whether AVIF or webp should be used. In general, avif or webp feels much more like a site-building task than a content editor one - why add the load of choosing this every time? There could eventually be even more formats available, as well as composite formats, like avif with webp fallback, potentially even with a file size check if we do 📌 Consider falling back to webp based on filesize Active . Is every combination of those going to have to be provided in the SDC as they're added? This really doesn't seem to be something to configure for every XB component.

On top of this, the existing image style system provides a lot more options than are mentioned here: cropping which is necessary when uploads can be different aspect ratios, focal point crop etc. I imagine Experience Builder would also want to support these - are they going to need to be baked into the SDC component too?

This is a reason to improve the usability of responsive images in core, not to build an entirely parallel system that may or may not be able to do the same things and can only be used by experience builder.

The SDC principle is that it should receive the props, but the responsive image style config itself doesn't need to be a prop - instead the SDC component could receive the image URLs + dimensions + alt text, which can be built in advance based on the data stored in Experience Builder itself and passed in - just enough information to generate the img + srcset (or picture element) from.

But where would you choose the responsive image style in this scenario? This should not be up to the content creator to decide. The component should always render with the same (responsive) image style.

Since the URL or URLs contain all the necessary information for the image style callback to render the image, the SDC doesn't need to know anything about anything else - it can render the markup/attributes based on what's passed in.

I looked at \Drupal\image\Controller\ImageStyleDownloadController again and it looks like we may be able to re-use the download parts of image styles. I think this is something that @justafish indicated she would be looking into. There's already a PoC in https://git.drupalcode.org/project/image_style_dynamic/-/tree/8.x-1.x?re... for this.

I somehow had the impression that the garbage collection of images was relying on file entities but after reviewing this again, it doesn't seem to be the case. Based on this, we should be able to use some of the lower level APIs from image styles. I removed this challenge from the issue summary.

At the moment, any component library would have to re-implement all the various configuration options in the SDC added here in order to support responsive images, whereas really they should just be able to take the minimum set of information to build the markup.

Based on above, we may be able to provide additional components for loading image styles and responsive images from components as a fallback.

The current implementation is shoe-horning half a dozen configuration options into the component, including whether AVIF or webp should be used. In general, avif or webp feels much more like a site-building task than a content editor one - why add the load of choosing this every time? There could eventually be even more formats available, as well as composite formats, like avif with webp fallback, potentially even with a file size check if we do #3516434: Consider falling back to webp from AVIF based on filesize. Is every combination of those going to have to be provided in the SDC as they're added? This really doesn't seem to be something to configure for every XB component.

The component is not necessarily designed to be exposed in XB – it's a developer tool for whoever is building the components. Also, there are settings that should not be in the component itself. For example, the image types that are generated should be a global configuration. See https://nextjs.org/docs/pages/api-reference/components/image#advanced for how Next.js does this.

On top of this, the existing image style system provides a lot more options than are mentioned here: cropping which is necessary when uploads can be different aspect ratios, focal point crop etc. I imagine Experience Builder would also want to support these - are they going to need to be baked into the SDC component too?

We should support modules like focal point with the component. I'm not sure if that's the case for the MR as it currently stands but it is mentioned in the issue summary.

This is a reason to improve the usability of responsive images in core, not to build an entirely parallel system that may or may not be able to do the same things and can only be used by experience builder.

It would be very hard to iteratively address this in the responsive image module. I think it would be better to build an alternative module in contrib, and battle test it before putting it to core.

But where would you choose the responsive image style in this scenario? This should not be up to the content creator to decide. The component should always render with the same (responsive) image style.

This seems like something for a site builder to configure. Even if not using responsive image styles as such, there needs to be settings for things like whether to crop to specific aspect ratios etc. as discussed above, which should never be exposed to content creators on a per-image basis either. So there has to be some kind of configuration step for the component not just within the component at the content creator level, and then it can happen there.

I somehow had the impression that the garbage collection of images was relying on file entities but after reviewing this again, it doesn't seem to be the case.

Yes it has nothing to do with file entities, this is why it's good to verify requirements before thousands of lines of code are written, even if an LLM is writing the code.

Also, there are settings that should not be in the component itself. For example, the image types that are generated should be a global configuration.

If the image type is a global setting, then why is the MR passing it to the component as a prop in the first place? And why is the component controlling which types are available? I think the answer to this is 'because the MR was written by Claude and posted without human review without clear instructions' but that's why I asked for an issue summary here defining the actual requirements.

Also if for some reason the format (or anything else) is taken from global configuration, then translated to a string to pass to the component, how is this translation layer different to what would be necessary to use a responsive image style to generate the relevant attributes and URL from to pass to the component?

It would be very hard to iteratively address this in the responsive image module. I think it would be better to build an alternative module in contrib,

Whether the work was done directly in core or in contrib, it would still be a coherent module in its own right, and not an entire subsystem hidden in experience builder.

There's already a PoC in https://git.drupalcode.org/project/image_style_dynamic/-/tree/8.x-1.x?re... for this.

OK but from that page,

In order to protect sites from DDOS you can configure the allowed query strings inside
image_styles_dynamic.settings

this is not very encouraging.

Adding important info to the issue title per #7.

This seems like something for a site builder to configure. Even if not using responsive image styles as such, there needs to be settings for things like whether to crop to specific aspect ratios etc. as discussed above, which should never be exposed to content creators on a per-image basis either. So there has to be some kind of configuration step for the component before it's available to site builders, not just within the component at the content creator level, and then it can happen at that step.

The proposal is for the "(responsive) image style" to be able to live inside component code.

For example, the card component may do the following:

{% include 'image' with image|merge({ width: 250 })}) %}

This would automatically generate the image with the width 250px.

The hero component might do the following:

{% include 'image' with image|merge({ width: 2500 })}) %}

This would automatically generate a responsive image style where the highest width would be 2500px.

In future, we could add support to configure these using a config entity so that they could be used in the UI.

Yes it has nothing to do with file entities, this is why it's good to verify requirements before thousands of lines of code are written, even if an LLM is writing the code.

Looking at this again, it does look like the garbage collection only works for file entities because the only place where \image_path_flush is called is \Drupal\image\Hook\ImageHooks::filePredelete and \Drupal\image\Hook\ImageHooks::fileMove. So if we use image styles for non-file entities, we'd have to add another way of handling garbage collection.

Also if for some reason the format (or anything else) is taken from global configuration, then translated to a string to pass to the component, how is this translation layer different to what would be necessary to use a responsive image style to generate the relevant attributes and URL from to pass to the component?

There's nothing wrong in having global settings in configuration so long as the set of configurations is predictable, so that we are not introducing a dependency from components to a specific config entity which may or may not exist on a site. We can also provide sensible defaults for the configured image types. The problem with image styles and responsive images is dependency on specific config entities that may or may not exist, and the fact that they are extremely difficult to configure. Even if they exist, they may or may not have the configuration expected by a specific component because how would a component know if e.g., "small" image style has the configuration that the component expects?

this is not very encouraging.

This is one of the problems solved in the current MR. I think it would be better to wait for an actual proposal until doing further evaluation on this level.

Looking at this again, it does look like the garbage collection only works for file entities because the only place where

Image styles as a whole can be flushed, which will clean anything remaining out, however also any new system would also have to add garbage collection both for styles and individual images.

But... I don't see how that can be achieved with the current proposal.

If a component defines a 2500px responsive image style which generates say five different image derivatives, then the hmac in the MR will ensure that no-one can generate a 2499px responsive image style arbitrarily. However, if a site stops using that component (swaps it out for one that's 1800px instead), how will the system know that it needs to delete all of those images (or not if the component is used elsewhere in a different layout)? This could be hundreds of thousands of derivative images on disk to worry about.

Equally, because the hmac only validates that the request is a genuine Drupal request and doesn't rely on configuration, even if there is some kind of event that deletes images when the last usage of a component is removed, nothing stops cached HTML requests from regenerating those images again because the URL is always going to be valid. You might not be able to regenerate lots of URLs for a DDOS attack but they might be referenced from internet archive or similar (or still in a CDN). Both image styles and asset aggregates have explicit protection against this kind of stale file disk filling situation.