From 56fedca8e32ca86a3054fe3be7c1955a88d4e02b Mon Sep 17 00:00:00 2001 From: Rishabh Date: Wed, 5 Aug 2026 14:49:10 +0530 Subject: [PATCH] fix: keep every frame when optimizing animated images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sharp decodes only the first frame unless constructed with `{ animated: true }`, so animated GIFs were served as still images by both image pipelines — the /api/image handler (dev and server builds) and the static build's .webp generation, which MDXImage always prefers via . Detect multi-frame sources with a header-only metadata probe, gated on the two formats that can animate, and pass the flag through to sharp. AVIF output is skipped for animated input because sharp flattens it; those requests get animated WebP instead, which every AVIF-capable browser decodes. The flag is folded into the cache key and ETag only when set, so still-image keys stay stable while stale single-frame entries are busted. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/cli/commands/static-generate.ts | 7 +- .../chronicle/src/lib/image-animation.test.ts | 49 +++++++++++++ packages/chronicle/src/lib/image-animation.ts | 20 ++++++ .../chronicle/src/lib/image-utils.test.ts | 22 ++++++ packages/chronicle/src/lib/image-utils.ts | 7 ++ .../chronicle/src/server/api/image.test.ts | 72 ++++++++++++++++++- packages/chronicle/src/server/api/image.ts | 47 +++++++----- 7 files changed, 205 insertions(+), 19 deletions(-) create mode 100644 packages/chronicle/src/lib/image-animation.test.ts create mode 100644 packages/chronicle/src/lib/image-animation.ts diff --git a/packages/chronicle/src/cli/commands/static-generate.ts b/packages/chronicle/src/cli/commands/static-generate.ts index eb29bd5..e2e63bb 100644 --- a/packages/chronicle/src/cli/commands/static-generate.ts +++ b/packages/chronicle/src/cli/commands/static-generate.ts @@ -20,6 +20,7 @@ import { loadApiSpec, resolveDocument, type ApiSpec } from '@/lib/openapi'; import { buildApiRoutes, getSpecSlug } from '@/lib/api-routes'; import { buildLlmsTxt, type LlmsPage } from '@/lib/llms'; import { DEFAULT_WIDTH, DEFAULT_QUALITY, isLocalImage, isSvg, splitVersion } from '@/lib/image-utils'; +import { isAnimatedImage } from '@/lib/image-animation'; import { getAssetVersion } from '@/lib/asset-version'; import type { VersionContext } from '@/lib/version-source'; import type { Frontmatter, PageNavLink } from '@/types'; @@ -717,7 +718,11 @@ async function optimizeImages( try { await fs.mkdir(path.dirname(destPath), { recursive: true }); const source = await fs.readFile(srcPath); - const optimizedBuf = await sharp(source) + // Animated sources must keep every frame — the element in + // MDXImage always prefers this .webp, so a flattened one would render + // an animated GIF as a still image + const animated = await isAnimatedImage(srcPath); + const optimizedBuf = await sharp(source, { animated }) .resize({ width: DEFAULT_WIDTH, withoutEnlargement: true }) .webp({ quality: DEFAULT_QUALITY }) .toBuffer(); diff --git a/packages/chronicle/src/lib/image-animation.test.ts b/packages/chronicle/src/lib/image-animation.test.ts new file mode 100644 index 0000000..4066e0d --- /dev/null +++ b/packages/chronicle/src/lib/image-animation.test.ts @@ -0,0 +1,49 @@ +import { afterAll, beforeAll, describe, expect, test } from 'bun:test'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import sharp from 'sharp'; +import { isAnimatedImage } from './image-animation'; + +let dir: string; +let animatedGif: string; +let stillGif: string; +let stillPng: string; + +async function solidPng(color: string): Promise { + return sharp({ create: { width: 8, height: 8, channels: 3, background: color } }).png().toBuffer(); +} + +beforeAll(async () => { + dir = await fs.mkdtemp(path.join(os.tmpdir(), 'chronicle-anim-')); + animatedGif = path.join(dir, 'tour.gif'); + stillGif = path.join(dir, 'still.gif'); + stillPng = path.join(dir, 'photo.png'); + + const frames = await Promise.all([solidPng('#f00'), solidPng('#00f'), solidPng('#0f0')]); + await fs.writeFile(animatedGif, await sharp(frames, { join: { animated: true } }).gif().toBuffer()); + await fs.writeFile(stillGif, await sharp(frames[0]).gif().toBuffer()); + await fs.writeFile(stillPng, frames[0]); +}); + +afterAll(async () => { + await fs.rm(dir, { recursive: true, force: true }); +}); + +describe('isAnimatedImage', () => { + test('returns true for a multi-frame gif', async () => { + expect(await isAnimatedImage(animatedGif)).toBe(true); + }); + + test('returns false for a single-frame gif', async () => { + expect(await isAnimatedImage(stillGif)).toBe(false); + }); + + test('returns false for formats that cannot animate', async () => { + expect(await isAnimatedImage(stillPng)).toBe(false); + }); + + test('returns false for a missing file', async () => { + expect(await isAnimatedImage(path.join(dir, 'nope.gif'))).toBe(false); + }); +}); diff --git a/packages/chronicle/src/lib/image-animation.ts b/packages/chronicle/src/lib/image-animation.ts new file mode 100644 index 0000000..fa3fe41 --- /dev/null +++ b/packages/chronicle/src/lib/image-animation.ts @@ -0,0 +1,20 @@ +import sharp from 'sharp'; +import { isAnimatable } from './image-utils'; + +// Server-only: keeps the `sharp` import out of image-utils.ts, which is shared +// with the client bundle. + +/** + * True when the file holds more than one frame. sharp decodes only the first + * frame unless constructed with `{ animated: true }`, so callers need this to + * avoid flattening animated GIF/WebP sources. + */ +export async function isAnimatedImage(filePath: string): Promise { + if (!isAnimatable(filePath)) return false; + try { + const { pages } = await sharp(filePath).metadata(); + return (pages ?? 1) > 1; + } catch { + return false; + } +} diff --git a/packages/chronicle/src/lib/image-utils.test.ts b/packages/chronicle/src/lib/image-utils.test.ts index ac9d903..235d854 100644 --- a/packages/chronicle/src/lib/image-utils.test.ts +++ b/packages/chronicle/src/lib/image-utils.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from 'bun:test'; import { isLocalImage, isSvg, + isAnimatable, buildOptimizedUrl, webpUrl, splitVersion, @@ -38,6 +39,27 @@ describe('isSvg', () => { }); }); +describe('isAnimatable', () => { + test('returns true for .gif and .webp', () => { + expect(isAnimatable('/_content/tour.gif')).toBe(true); + expect(isAnimatable('/_content/tour.webp')).toBe(true); + }); + + test('ignores case and query strings', () => { + expect(isAnimatable('/_content/TOUR.GIF?v=abc123')).toBe(true); + }); + + test('returns false for still-only formats', () => { + expect(isAnimatable('/_content/photo.png')).toBe(false); + expect(isAnimatable('/_content/photo.jpg')).toBe(false); + expect(isAnimatable('/_content/logo.svg')).toBe(false); + }); + + test('works on filesystem paths', () => { + expect(isAnimatable('/abs/path/.content/docs/tour.gif')).toBe(true); + }); +}); + describe('buildOptimizedUrl', () => { test('builds URL with width and default quality', () => { const url = buildOptimizedUrl('/_content/img.png', 640); diff --git a/packages/chronicle/src/lib/image-utils.ts b/packages/chronicle/src/lib/image-utils.ts index 87efc3f..fa174f7 100644 --- a/packages/chronicle/src/lib/image-utils.ts +++ b/packages/chronicle/src/lib/image-utils.ts @@ -11,6 +11,13 @@ export function isSvg(url: string): boolean { return url.split('?')[0].endsWith('.svg'); } +// Only these formats can hold multiple frames — everything else skips the +// metadata probe in isAnimatedImage() +export function isAnimatable(url: string): boolean { + const base = url.split('?')[0].toLowerCase(); + return base.endsWith('.gif') || base.endsWith('.webp'); +} + export function buildOptimizedUrl(url: string, width: number, quality = DEFAULT_QUALITY, version?: string): string { const base = `/api/image?url=${encodeURIComponent(url)}&w=${width}&q=${quality}`; return version ? `${base}&v=${version}` : base; diff --git a/packages/chronicle/src/server/api/image.test.ts b/packages/chronicle/src/server/api/image.test.ts index f4ea601..2f8fae1 100644 --- a/packages/chronicle/src/server/api/image.test.ts +++ b/packages/chronicle/src/server/api/image.test.ts @@ -1,5 +1,9 @@ -import { describe, expect, test } from 'bun:test'; -import { negotiateFormat, cacheKey, MIME } from './image'; +import { afterAll, beforeAll, describe, expect, test } from 'bun:test'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import sharp from 'sharp'; +import { negotiateFormat, cacheKey, MIME, optimizeImage } from './image'; describe('negotiateFormat', () => { test('returns avif when Accept includes image/avif', () => { @@ -21,6 +25,22 @@ describe('negotiateFormat', () => { test('prefers avif over webp when both present', () => { expect(negotiateFormat('image/webp,image/avif')).toBe('avif'); }); + + test('downgrades avif to webp for animated sources', () => { + expect(negotiateFormat('image/avif,image/webp,*/*', true)).toBe('webp'); + }); + + test('returns webp for animated sources when only avif is advertised', () => { + expect(negotiateFormat('image/avif,*/*', true)).toBe('webp'); + }); + + test('returns webp for animated sources when webp is advertised', () => { + expect(negotiateFormat('image/webp,image/png,*/*', true)).toBe('webp'); + }); + + test('returns original for animated sources with neither format', () => { + expect(negotiateFormat('image/png,*/*', true)).toBe('original'); + }); }); describe('cacheKey', () => { @@ -66,6 +86,18 @@ describe('cacheKey', () => { expect(a).toBe(b); }); + test('returns different keys for animated and still variants', () => { + const still = cacheKey('/_content/tour.gif', 1024, 75, 'webp', 'aaaa111111'); + const animated = cacheKey('/_content/tour.gif', 1024, 75, 'webp', 'aaaa111111', true); + expect(animated).not.toBe(still); + }); + + test('leaves still-image keys unchanged when animated defaults to false', () => { + const implicit = cacheKey('/_content/img.png', 1024, 75, 'webp', 'aaaa111111'); + const explicit = cacheKey('/_content/img.png', 1024, 75, 'webp', 'aaaa111111', false); + expect(explicit).toBe(implicit); + }); + test('key ends with format extension', () => { expect(cacheKey('/_content/img.png', 640, 75, 'webp')).toMatch(/\.webp$/); expect(cacheKey('/_content/img.png', 640, 75, 'avif')).toMatch(/\.avif$/); @@ -73,6 +105,42 @@ describe('cacheKey', () => { }); }); +describe('optimizeImage with animated sources', () => { + let dir: string; + let gif: string; + + beforeAll(async () => { + dir = await fs.mkdtemp(path.join(os.tmpdir(), 'chronicle-image-')); + gif = path.join(dir, 'tour.gif'); + const frames = await Promise.all( + ['#f00', '#00f', '#0f0'].map(background => + sharp({ create: { width: 8, height: 8, channels: 3, background } }).png().toBuffer(), + ), + ); + await fs.writeFile(gif, await sharp(frames, { join: { animated: true } }).gif().toBuffer()); + }); + + afterAll(async () => { + await fs.rm(dir, { recursive: true, force: true }); + }); + + async function pages(buf: Buffer): Promise { + return (await sharp(buf).metadata()).pages ?? 1; + } + + test('keeps every frame on webp output', async () => { + expect(await pages(await optimizeImage(gif, 320, 75, 'webp', true))).toBe(3); + }); + + test('keeps every frame on original-format output', async () => { + expect(await pages(await optimizeImage(gif, 320, 75, 'original', true))).toBe(3); + }); + + test('flattens to one frame when animated is not set', async () => { + expect(await pages(await optimizeImage(gif, 320, 75, 'webp'))).toBe(1); + }); +}); + describe('MIME', () => { test('maps common image extensions', () => { expect(MIME['.png']).toBe('image/png'); diff --git a/packages/chronicle/src/server/api/image.ts b/packages/chronicle/src/server/api/image.ts index 311ebb2..2550e9a 100644 --- a/packages/chronicle/src/server/api/image.ts +++ b/packages/chronicle/src/server/api/image.ts @@ -9,6 +9,7 @@ import { safePath } from '@/server/utils/safe-path' import { assetCacheControl, etagFor, isNotModified, REVALIDATE_CACHE } from '@/server/utils/asset-cache' import { getAssetVersion } from '@/lib/asset-version' import { ALLOWED_WIDTHS, ALLOWED_QUALITIES, DEFAULT_WIDTH, DEFAULT_QUALITY, isLocalImage, isSvg, splitVersion } from '@/lib/image-utils' +import { isAnimatedImage } from '@/lib/image-animation' export const STORAGE_KEY = 'image-cache' @@ -16,9 +17,14 @@ const inflight = new Map>() export type OutputFormat = 'avif' | 'webp' | 'original' -export function negotiateFormat(accept: string | null): OutputFormat { - if (accept?.includes('image/avif')) return 'avif' - if (accept?.includes('image/webp')) return 'webp' +export function negotiateFormat(accept: string | null, animated = false): OutputFormat { + const wantsAvif = accept?.includes('image/avif') ?? false + const wantsWebp = accept?.includes('image/webp') ?? false + // sharp flattens animated sources to a single frame on AVIF output, so + // animated images fall back to WebP — every AVIF-capable browser decodes + // animated WebP + if (wantsAvif && !animated) return 'avif' + if (wantsWebp || (animated && wantsAvif)) return 'webp' return 'original' } @@ -30,8 +36,11 @@ export const MIME: Record = { '.webp': 'image/webp', } -export function cacheKey(url: string, w: number, q: number, format: OutputFormat, version?: string | number): string { - const hash = crypto.createHash('sha256').update(`${url}:${w}:${q}:${format}:${version ?? 0}`).digest('hex').slice(0, 16) +export function cacheKey(url: string, w: number, q: number, format: OutputFormat, version?: string | number, animated = false): string { + // The animated marker is only appended when set, so keys for still images + // stay stable across the animation fix and the on-disk cache survives + const suffix = animated ? ':animated' : '' + const hash = crypto.createHash('sha256').update(`${url}:${w}:${q}:${format}:${version ?? 0}${suffix}`).digest('hex').slice(0, 16) return `${hash}.${format}` } @@ -48,9 +57,10 @@ export async function optimizeImage( w: number, q: number, format: OutputFormat, + animated = false, ): Promise { const source = await fs.readFile(filePath); - const pipeline = sharp(source).resize({ width: w, withoutEnlargement: true }); + const pipeline = sharp(source, { animated }).resize({ width: w, withoutEnlargement: true }); if (format === 'avif') return pipeline.avif({ quality: q }).toBuffer(); if (format === 'webp') return pipeline.webp({ quality: q }).toBuffer(); return pipeline.toBuffer(); @@ -91,24 +101,28 @@ export default defineHandler(async event => { throw new HTTPError({ status: StatusCodes.NOT_FOUND, message: 'Not Found' }) } + const stat = await fs.stat(filePath).catch(() => null) + if (!stat) { + throw new HTTPError({ status: StatusCodes.NOT_FOUND, message: 'Not Found' }) + } + + const animated = await isAnimatedImage(filePath) const accept = event.headers.get('accept') - const format = negotiateFormat(accept) + const format = negotiateFormat(accept, animated) const ext = path.extname(filePath).toLowerCase() const originalMime = MIME[ext] ?? 'application/octet-stream' const contentType = format === 'original' ? originalMime : `image/${format}` - const stat = await fs.stat(filePath).catch(() => null) - if (!stat) { - throw new HTTPError({ status: StatusCodes.NOT_FOUND, message: 'Not Found' }) - } const currentVersion = await getAssetVersion(filePath) - const key = cacheKey(url, w, q, format, currentVersion ?? stat.mtimeMs) + const key = cacheKey(url, w, q, format, currentVersion ?? stat.mtimeMs, animated) const requestedVersion = event.url.searchParams.get('v') const cacheControl = import.meta.dev ? REVALIDATE_CACHE : assetCacheControl(requestedVersion, currentVersion) - const etag = etagFor(currentVersion ?? String(stat.mtimeMs), String(w), String(q), format) + // `animated` is part of the ETag so browsers holding a pre-fix single-frame + // response revalidate instead of getting a 304 + const etag = etagFor(currentVersion ?? String(stat.mtimeMs), String(w), String(q), format, ...(animated ? ['animated'] : [])) const headers = { 'Content-Type': contentType, 'Cache-Control': cacheControl, @@ -132,7 +146,7 @@ export default defineHandler(async event => { } const work = (async () => { - const optimized = await optimizeImage(filePath, w, q, format) + const optimized = await optimizeImage(filePath, w, q, format, animated) await storage.setItemRaw(key, optimized) return optimized })() @@ -183,12 +197,13 @@ export async function warmupImageCache() { const stat = await fs.stat(filePath).catch(() => null); if (!stat) continue; - const key = cacheKey(base, w, q, format, (await getAssetVersion(filePath)) ?? stat.mtimeMs); + const animated = await isAnimatedImage(filePath); + const key = cacheKey(base, w, q, format, (await getAssetVersion(filePath)) ?? stat.mtimeMs, animated); const cached = await storage.getItemRaw(key); if (cached) continue; try { - const optimized = await optimizeImage(filePath, w, q, format); + const optimized = await optimizeImage(filePath, w, q, format, animated); await storage.setItemRaw(key, optimized); warmed++; } catch { /* skip unprocessable */ }