Skip to content

BUGFIX: Provide fallback for async ImageTag#5719

Open
Sebobo wants to merge 1 commit into8.4from
bugfix/5717-async-imagetag-fallback
Open

BUGFIX: Provide fallback for async ImageTag#5719
Sebobo wants to merge 1 commit into8.4from
bugfix/5717-async-imagetag-fallback

Conversation

@Sebobo
Copy link
Member

@Sebobo Sebobo commented Dec 18, 2025

With Neos 8.4 I created a thumbnail instead of an ImageUri to be able to generate the width and height for the image attributes. But in case of async = true the initially created thumbnail is async and has no resource and the ResourceUri in the src attribute throws an error.

So now I provide a fallback for the first time the thumbnail is requested in which the image size cannot be calculated. After a reload everything is back to normal and the thumbnail and its size are usable.

Resolves: #5717

Review instructions

In the demo this can be verified by adjusting the Neos.Demo:Content.Image prototype and using it in the Demo with some cropping.

prototype(Neos.Demo:Content.Image) < prototype(Neos.Neos:ContentComponent) {
    src = Neos.Neos:ImageUri {
        @if.hasAsset = ${this.asset}
        asset = ${q(node).property('image')}
    }
    alt = ${q(node).property('alternativeText')}
    title = ${q(node).property('title')}
    hasCaption = ${q(node).property('hasCaption')}
    caption = Neos.Neos:Editable {
        property = 'caption'
    }

    renderDummyImage = ${renderingMode.isEdit}

    renderer = Neos.Neos:ImageTag {
        asset = ${q(node).property('image')}
        width = 400
        height = 400
        maximumWidth = 800
        maximumHeight = 800
        allowCropping = true
        allowUpScaling = true
        async = true
    }
//    renderer = afx`<Neos.Demo:Presentation.Image {...props} />`
}

With Neos 8.4 I created a thumbnail to be able to generate the width and height for the image attributes.
But in case of `async = true` the initially created thumbnail is async and has no resource and the ResourceUri throws an error.

So we provide a fallback for the first time the thumbnail is requested in which the image size cannot be calculated. After a reload everything is back to normal and the thumbnail and its size are usable.

Resolves: #5717
@Sebobo Sebobo requested review from dlubitz and kitsunet December 18, 2025 09:21
@Sebobo Sebobo self-assigned this Dec 18, 2025
@Sebobo
Copy link
Member Author

Sebobo commented Dec 18, 2025

This solution is a bit messy. I'm open for a neater implementation for the special asynchronous case.

@mhsdesign mhsdesign requested a review from mficzel January 23, 2026 13:47
Copy link
Member

@mficzel mficzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see no real difference compared to making the image non-async. Maybe i do not understand the solution fully.

Would'nt the correct way be to calculate (or predict) the target dimensions of an asymc-thumbnail directly the moment we create the async asset. I played with that in the focal point pr from back then (see #5127). Maybe that part can be reused. However it may be way to breaky for a bugfix.

@Sebobo
Copy link
Member Author

Sebobo commented Jan 23, 2026

I see no real difference compared to making the image non-async. Maybe i do not understand the solution fully.

Would'nt the correct way be to calculate (or predict) the target dimensions of an asymc-thumbnail directly the moment we create the async asset. I played with that in the focal point pr from back then (see #5127). Maybe that part can be reused. However it may be way to breaky for a bugfix.

It is still async. If the resource is not available yet, it will create an asynchronous uri with the ImageUri the async = false case will always result in the thumbnail being used. The dimensions would not solve this issue (but help with the size attributes on first render).

@bwaidelich
Copy link
Member

I have a general question regarding async images (feel free to ignore me if that's out of scope):

src = Neos.Fusion:Case {
      resource {
        condition = ${thumbnail.resource}
        // ...
      }
      asyncFallback {
        // ...  
      }
}

will render the fallback on first hit usually. If that's a regular frontend request, the result will be "baked" into the cached content of the embedding Fusion constructs and it would never be re-evaluated until caches are flushed.
Isn't that a fundamental flaw with this approach?

@mficzel
Copy link
Member

mficzel commented Jan 24, 2026

@bwaidelich eventually there will be a Cache flush after which the final urls will be rendered.

As least we usually flush fusion caches during deployments so this seldomly takes more than a few days.

@bwaidelich
Copy link
Member

bwaidelich commented Jan 24, 2026

eventually there will be a Cache flush after which the final urls will be rendered.

For large scale & high traffic websites flushing the complete content cache is not really an option because it would kill the server (not to imply that you don't have those kind of websites, but maybe your server setup is stronger *g)
I agree, that it's probably mostly not an issue because the first render probably happens in the backend or while the page is being edited anyways. But in some cases (e.g. in reused content parts that share a global cache) we are stuck with the additional async request handling for a large number of pages

@Sebobo
Copy link
Member Author

Sebobo commented Jan 24, 2026

eventually there will be a Cache flush after which the final urls will be rendered.

For large scale & high traffic websites flushing the complete content cache is not really an option because it would kill the server (not to imply that you don't have those kind of websites, but maybe your server setup is stronger *g) I agree, that it's probably mostly not an issue because the first render probably happens in the backend or while the page is being edited anyways. But in some cases (e.g. in reused content parts that share a global cache) we are stuck with the additional async request handling for a large number of pages

As I said before, I advise only using it in the backend. After getting weird reports from SEO tools, having the URLs staying to long in the cache (like you mentioned), not wanting to have urls like that reachable from the outside, etc. I disabled it for the live rendering in all customer projects. But in the backend it's helpful, when each image has 10 responsive variants etc. and editors are playing around with images and cropping.

@bwaidelich
Copy link
Member

@Sebobo that makes a lot of sense, thanks!
I didn't think of that and I wonder if other users will think of it. Wouldn't it make sense to make that the default behavior (i.e. only fall back to async if in backend or explicitly activated)?

@mficzel
Copy link
Member

mficzel commented Jan 24, 2026

For large scale & high traffic websites flushing the complete content cache is not really an option because it would kill the server

About caching: We use rolling updates, autoscaling and green/blue caches to be able to start each release with fresh caches (not all caches). Since eventually fusion caches sometimes need flushing after code evolved we built the rollout process to survive that in the first place.

For the async topic here I would be totally fine with changing the default for async to off again but would not do automagic for the backend and also consider this breaky.

Can you explain to me why the suggestion to precalculate the dimensions of async images is not a possible solution to the mentioned problem of missing width and height for not yet rendered async images? That would be a clear feature that can be added in a minor release.

@Sebobo
Copy link
Member Author

Sebobo commented Jan 25, 2026

Can you explain to me why the suggestion to precalculate the dimensions of async images is not a possible solution to the mentioned problem of missing width and height for not yet rendered async images? That would be a clear feature that can be added in a minor release.

I didn't mean it's not possible. Just not implemented, but of course it would be great next addition 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants