Fall back to the original image before the default image - #4007
Merged
Merged
Conversation
OpenAPI ChangesNo changes detected Unexpected changes? Ensure your branch is up-to-date with |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The passive stage reset can briefly load a newly supplied source unoptimized or fetch it twice.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Adds direct-browser image loading between Next.js optimization failure and the default fallback.
Changes:
- Extends
useImageWithFallbackwith optimized, original, and fallback stages. - Propagates
unoptimizedthrough affected image components. - Updates query typings and fallback tests.
| File | Description |
|---|---|
frontends/ol-utilities/src/hooks/useImageWithFallback.ts |
Implements the fallback state machine. |
frontends/ol-utilities/src/hooks/useImageWithFallback.test.ts |
Tests fallback stages and source changes. |
frontends/ol-test-utilities/src/domQueries/byImageSrc.ts |
Types image-query options. |
frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx |
Passes unoptimized state to list cards. |
frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx |
Tests list-card fallback sequence. |
frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.tsx |
Passes unoptimized state to cards. |
frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.test.tsx |
Tests card and SVG fallbacks. |
frontends/ol-components/src/components/BaseLearningResourceCard/BaseLearningResourceCard.tsx |
Adds image optimization control. |
frontends/main/src/page-components/LearningResourceExpanded/CallToActionSection.tsx |
Applies fallback behavior to drawer images. |
frontends/main/src/page-components/LearningResourceExpanded/CallToActionSection.test.tsx |
Tests drawer fallback sequence. |
frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoCard.tsx |
Reuses the shared fallback hook. |
frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx |
Applies fallback behavior to sidebar images. |
frontends/main/src/app-pages/ProductPages/ProductPageTemplate.test.tsx |
Tests sidebar fallback sequence. |
frontends/main/src/app-pages/ProductPages/MitxOnlineResourceCard.tsx |
Applies fallback behavior to MITx cards. |
frontends/main/src/app-pages/ProductPages/MitxOnlineResourceCard.test.tsx |
Tests MITx card fallback sequences. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The image optimizer fetches remote images server-side, and some hosts block that (e.g. bot protection answering 403) while still serving the same image to browsers. useImageWithFallback went straight from the failed optimized image to the default image, so those resources lost their images. It now retries the original with next/image's `unoptimized` first, and only falls back to the default if that fails too. Every image using the hook passes the new `unoptimized` value through. Also type byImageSrc's error functions with their real arguments so the queries accept the `nextJsOriginalSrc` option in TypeScript. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Next.js serves some images unoptimized regardless of the prop, e.g. SVGs and data: URLs. For those, the "original" retry rendered the same <img>, no second error fired, and the image stayed broken instead of falling back to the default. onError now checks the failed element's src and goes straight to the fallback when it was already the original. Also move VideoCard's hand-rolled optimized-then-placeholder fallback onto useImageWithFallback so its thumbnails get the original-image retry. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gumaerc
force-pushed
the
cg/nextjs-image-opt-fallback-to-original
branch
from
September 29, 2026 16:21
876b27e to
827b35b
Compare
ChristopherChudzicki
approved these changes
Sep 29, 2026
ChristopherChudzicki
left a comment
ChristopherChudzicki
left a comment
Contributor
There was a problem hiding this comment.
👍 But with one suggestion that I do think is worth doing.
Replace the useEffect that reset the stage when src changed with state that records which src failed. Any other src starts at its initial stage in the same render, so a new src can no longer render once with the previous src's stage (e.g. loading unoptimized, or twice). Adds a test that records every render to check this. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What are the relevant tickets?
Part of https://github.com/mitodl/hq/issues/10240 (follow-up to #3983)
Description (What does it do?)
Since image optimization went live, some resources show the default image even though their real image loads fine in a browser. The optimizer fetches remote images server-side, and some hosts block that. climate.mit.edu (behind Akamai) answers the optimizer with 403: the production nextjs pod logs had 1,047 of these across 48 images between 14:05 and 16:41 UTC on 2026-09-28.
useImageWithFallbackwent straight from the failed optimized image to the default.The fallback chain is now: optimized image, then the original loaded directly by the browser (
next/imagewithunoptimized), then the default image. This covers any host that blocks server-side fetches, with no list of domains to maintain.useImageWithFallbackreturnsunoptimizedalongsidesrcandonError, and every image that uses it passes that through: the resource cards (via a newimageUnoptimizedprop onBaseLearningResourceCard), the resource drawer image,MitxOnlineResourceCard, and the product page sidebar image.data:URLs. For those the retry would render the same<img>and never fire another error, soonErrorchecks the failed element'ssrcand skips straight to the default.VideoCardhad its own copy of the old optimized-then-placeholder logic; it now uses the hook.byImageSrctest queries now accept theirnextJsOriginalSrcoption in TypeScript. It already worked at runtime, but the error functions were typed with the default single-argument signature.How can this be tested?
yarn testcovers the chain: new unit tests for the hook, and the existing fallback tests in the card, list card, drawer, MITx Online card and product page tests now check each step.*/_next/image*(Network request blocking) and load a search page. Card images should load from their original URLs instead of/_next/image. Also block an image's source host and it should show the default image. (I haven't run this check; the chain is covered by the tests above.)Additional Context
maximumResponseBodycap) now load the full original instead of the default image. That's what they did before optimization was turned on.FeaturedVideo,MoreFromPlaylist, testimonial avatars, organization logos, instructor photos, the homepage news section). None of these had an original-image fallback before this either. That's left for a follow-up./_next/imagerequest per page view before the browser loads the original, and Next doesn't cache failed fetches, so those requests reach the pods. Caching/_next/image4xx responses briefly at Fastly would stop that; that's a separate change in ol-infrastructure.🤖 Generated with Claude Code