diff --git a/.changeset/cf-image-endpoint-fit.md b/.changeset/cf-image-endpoint-fit.md new file mode 100644 index 0000000000..757647adb2 --- /dev/null +++ b/.changeset/cf-image-endpoint-fit.md @@ -0,0 +1,6 @@ +--- +"@emdash-cms/cloudflare": patch +"emdash": patch +--- + +Fixes cropping for EmDash media on Cloudflare. An image asked to fill a fixed box — a square avatar, a fixed-ratio thumbnail — came back scaled down and letterboxed inside it, because the endpoint never passed the requested fit to the Images binding. `fit` and `position` are now honoured, so a crop crops and its focal side is respected. diff --git a/package.json b/package.json index 9f2060cde7..c4e6b42731 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "typecheck:templates": "pnpm run --workspace-concurrency=1 --filter {./templates/*} typecheck", "check": "pnpm run typecheck && pnpm run --filter {./packages/*} check", "test": "pnpm run --filter {./packages/*} test", - "test:unit": "pnpm run --filter emdash --filter @emdash-cms/auth --filter @emdash-cms/blocks --filter @emdash-cms/gutenberg-to-portable-text --filter @emdash-cms/marketplace --filter @emdash-cms/plugin-cli --filter @emdash-cms/plugin-forms --filter @emdash-cms/plugin-types --filter @emdash-cms/registry-client --filter @emdash-cms/registry-lexicons test", + "test:unit": "pnpm run --filter emdash --filter @emdash-cms/auth --filter @emdash-cms/blocks --filter @emdash-cms/cloudflare --filter @emdash-cms/gutenberg-to-portable-text --filter @emdash-cms/marketplace --filter @emdash-cms/plugin-cli --filter @emdash-cms/plugin-forms --filter @emdash-cms/plugin-types --filter @emdash-cms/registry-client --filter @emdash-cms/registry-lexicons test", "test:browser": "pnpm run --filter @emdash-cms/admin test", "test:e2e": "playwright test", "test:e2e:ui": "playwright test --ui", diff --git a/packages/cloudflare/src/image-endpoint.ts b/packages/cloudflare/src/image-endpoint.ts index c11ea91887..1ba39c8aac 100644 --- a/packages/cloudflare/src/image-endpoint.ts +++ b/packages/cloudflare/src/image-endpoint.ts @@ -21,6 +21,7 @@ import { originalMediaHeaders, parseTransformParams, resolveTransformQuality, + type ImageTransformFit, type ImageTransformFormat, } from "emdash/media/image-endpoint"; @@ -33,6 +34,45 @@ const FORMAT_MIME: Record = png: "image/png", }; +/** + * Maps Astro `fit` values to the Cloudflare Images binding's fit vocabulary. + * Unmapped values (e.g. `outside`) become `undefined`, leaving the binding's + * default behaviour unchanged. + */ +const FIT_TO_BINDING: Record = { + fill: "squeeze", + contain: "contain", + cover: "cover", + "scale-down": "scale-down", + inside: "contain", + outside: undefined, +}; + +/** + * Maps Astro `position` values to the Cloudflare Images binding's gravity + * vocabulary. Compound or unknown positions are dropped. + */ +const GRAVITY_BY_POSITION = new Map([ + ["face", "face"], + ["left", "left"], + ["right", "right"], + ["top", "top"], + ["bottom", "bottom"], + ["center", "center"], + ["centre", "center"], + ["auto", "auto"], + ["entropy", "entropy"], + ["attention", "auto"], + ["north", "top"], + ["south", "bottom"], + ["east", "right"], + ["west", "left"], +]); + +function toBindingGravity(position: string): ImageTransform["gravity"] | undefined { + return GRAVITY_BY_POSITION.get(position.trim().toLowerCase()); +} + /** Resolve the Images binding by the name the Cloudflare adapter configured. */ function resolveImagesBinding(): ImagesBinding | undefined { const configured = (globalThis as { __ASTRO_IMAGES_BINDING_NAME?: unknown }) @@ -82,11 +122,19 @@ export const GET: APIRoute = async (ctx) => { return streamOriginal(source.body, source.contentType); } - const { width, height, format, quality } = parsed.options; + const { width, height, format, quality, fit, position } = parsed.options; const outputMime = FORMAT_MIME[format] ?? "image/webp"; const transform: ImageTransform = {}; if (width) transform.width = width; if (height) transform.height = height; + if (fit) { + const bindingFit = FIT_TO_BINDING[fit]; + if (bindingFit) transform.fit = bindingFit; + } + if (position) { + const gravity = toBindingGravity(position); + if (gravity) transform.gravity = gravity; + } // Lossy formats get an explicit quality: the Images binding has no // default of its own and encodes near-losslessly without one, producing // renditions several times the size of the original. PNG is exempt — diff --git a/packages/cloudflare/tests/image-endpoint-fit.test.ts b/packages/cloudflare/tests/image-endpoint-fit.test.ts new file mode 100644 index 0000000000..55a9e7912f --- /dev/null +++ b/packages/cloudflare/tests/image-endpoint-fit.test.ts @@ -0,0 +1,122 @@ +import type { APIContext } from "astro"; +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const adapterGET = vi.fn(() => new Response("adapter", { status: 200 })); +vi.mock("@astrojs/cloudflare/image-transform-endpoint", () => ({ GET: adapterGET })); + +/** Records the transform the endpoint hands to the Images binding. */ +const transform = vi.fn(); +const output = vi.fn(() => ({ + response: () => new Response("bytes", { headers: { "Content-Type": "image/webp" } }), +})); + +const images = { + input: () => ({ + transform: (options: unknown) => { + transform(options); + return { output }; + }, + }), +}; + +vi.mock("cloudflare:workers", () => ({ env: { IMAGES: images } })); + +const { GET } = await import("../src/image-endpoint.js"); + +const storage = { + download: async () => ({ + body: new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array([1, 2, 3])); + controller.close(); + }, + }), + contentType: "image/jpeg", + }), +}; + +/** Request the endpoint the way Astro's image service does. */ +function request(params: string): Promise { + const href = encodeURIComponent("/_emdash/api/media/file/01J5ABC.webp"); + const ctx = { + request: new Request(`https://example.com/_image?href=${href}&${params}`), + locals: { emdash: { storage } }, + } as unknown as APIContext; + return GET(ctx) as Promise; +} + +/** The options passed to the Images binding for the most recent request. */ +function lastTransform(): Record { + // eslint-disable-next-line typescript/no-unsafe-type-assertion -- vitest mock call args + return transform.mock.calls.at(-1)?.[0] as Record; +} + +beforeEach(() => { + transform.mockClear(); +}); + +describe("Cloudflare image endpoint: fit and position", () => { + it("forwards fit=cover so a square crop crops instead of letterboxing", async () => { + // Astro emits `fit`/`position` for constrained images. Dropping `fit` + // left the binding on its default, so a 32x32 avatar came back + // scaled-down inside the box rather than cover-cropped to fill it. + await request("w=32&h=32&f=webp&fit=cover&position=center"); + + expect(lastTransform()).toMatchObject({ width: 32, height: 32, fit: "cover" }); + }); + + it("maps position to the binding's gravity vocabulary", async () => { + await request("w=32&h=32&fit=cover&position=top"); + expect(lastTransform().gravity).toBe("top"); + + // sharp spells it "centre" and calls saliency-based cropping + // "attention"; the binding uses "center" and "auto". + await request("w=32&h=32&fit=cover&position=centre"); + expect(lastTransform().gravity).toBe("center"); + + await request("w=32&h=32&fit=cover&position=attention"); + expect(lastTransform().gravity).toBe("auto"); + }); + + it("maps sharp's compass names onto the edges they mean", async () => { + // Astro forwards sharp's vocabulary verbatim, so `position="north"` + // reaches the endpoint and means the same edge as `top`. + for (const [position, gravity] of [ + ["north", "top"], + ["south", "bottom"], + ["east", "right"], + ["west", "left"], + ]) { + await request(`w=32&h=32&fit=cover&position=${position}`); + expect(lastTransform().gravity).toBe(gravity); + } + }); + + it("maps Astro's sharp-only inside fit onto contain", async () => { + await request("w=64&h=64&fit=inside"); + expect(lastTransform().fit).toBe("contain"); + }); + + it("maps Astro's fill to the binding's squeeze", async () => { + await request("w=64&h=64&fit=fill"); + expect(lastTransform().fit).toBe("squeeze"); + }); + + it("omits fit and gravity the binding would reject rather than forwarding them", async () => { + // `outside` resizes to exceed the box without cropping, which the binding + // cannot express, and a compound position names a corner it has no keyword + // for. Both are dropped so the rendition still resolves, and neither is + // swapped for a fit that would crop. + await request("w=64&h=64&fit=outside&position=top%20left"); + + const options = lastTransform(); + expect(options.fit).toBeUndefined(); + expect(options.gravity).toBeUndefined(); + expect(options).toMatchObject({ width: 64, height: 64 }); + }); + + it("leaves fit unset when the request carries none", async () => { + await request("w=800&f=webp"); + expect(lastTransform().fit).toBeUndefined(); + }); +}); diff --git a/packages/core/src/media/image-endpoint.ts b/packages/core/src/media/image-endpoint.ts index a524fc5eb8..54aa9871d8 100644 --- a/packages/core/src/media/image-endpoint.ts +++ b/packages/core/src/media/image-endpoint.ts @@ -37,6 +37,27 @@ export const MAX_TRANSFORM_DIMENSION = 4000; /** A format string accepted by {@link ImageTransformOptions.format}. */ export type ImageTransformFormat = (typeof ALLOWED_TRANSFORM_FORMATS)[number]; +/** + * Fit values Astro's image service may emit. Unknown values are dropped so + * backends keep their default behaviour. + */ +export const ALLOWED_TRANSFORM_FITS = [ + "fill", + "contain", + "cover", + "scale-down", + "inside", + "outside", +] as const; + +/** A fit string accepted by {@link ImageTransformOptions.fit}. */ +export type ImageTransformFit = (typeof ALLOWED_TRANSFORM_FITS)[number]; + +/** Type guard for {@link ImageTransformFit}. */ +export function isTransformFit(value: string): value is ImageTransformFit { + return (ALLOWED_TRANSFORM_FITS as readonly string[]).includes(value); +} + /** Validated options for a single transform. */ export interface ImageTransformOptions { width?: number; @@ -48,6 +69,19 @@ export interface ImageTransformOptions { * {@link DEFAULT_TRANSFORM_QUALITY}); lossless PNG deliberately gets none. */ quality?: number; + /** + * How the rendition fills the requested box, from Astro's `fit`. Left + * `undefined` when the request carried none or carried a value outside + * {@link ALLOWED_TRANSFORM_FITS}, so callers keep their backend's default. + */ + fit?: ImageTransformFit; + /** + * Which part of the image survives a crop, from Astro's `position`. Only + * meaningful for fits that crop. Kept as the raw string: the vocabulary is + * the backend's (a keyword like `center`, or coordinates), so mapping it is + * the adapter's job. + */ + position?: string; } /** Long-lived immutable cache -- transform output is deterministic per key+params. */ @@ -150,13 +184,18 @@ export function resolveTransformQuality( } /** - * Parse and validate `?w=&h=&f=&q=` query params. Width is required (it sizes - * the rendition); dimensions are bounded so a request can't ask for an - * unbounded or nonsensical transform. Format falls back to + * Parse and validate `?w=&h=&f=&q=&fit=&position=` query params. Width is + * required (it sizes the rendition); dimensions are bounded so a request can't + * ask for an unbounded or nonsensical transform. Format falls back to * {@link DEFAULT_TRANSFORM_FORMAT} when not requested. `q` is validated when * present but otherwise left `undefined` so the caller can apply a per-format * default (lossy formats get one, lossless PNG does not — see * {@link DEFAULT_TRANSFORM_QUALITY}). + * + * `fit` and `position` describe how the rendition fills its box. Unlike the + * others they are advisory: an unrecognised value is dropped rather than + * failing the request. The platform endpoint maps them onto its backend's + * vocabulary. */ export function parseTransformParams(params: URLSearchParams): ParsedTransformParams { const width = parseDimension(params.get("w")); @@ -185,7 +224,13 @@ export function parseTransformParams(params: URLSearchParams): ParsedTransformPa quality = q; } - return { ok: true, options: { width, height, format, quality } }; + const fitRaw = params.get("fit"); + const fit = fitRaw !== null && isTransformFit(fitRaw) ? fitRaw : undefined; + + const positionRaw = params.get("position")?.trim(); + const position = positionRaw ? positionRaw : undefined; + + return { ok: true, options: { width, height, format, quality, fit, position } }; } /** diff --git a/packages/core/tests/unit/media/image-endpoint.test.ts b/packages/core/tests/unit/media/image-endpoint.test.ts index f6d567d38b..79ae804adf 100644 --- a/packages/core/tests/unit/media/image-endpoint.test.ts +++ b/packages/core/tests/unit/media/image-endpoint.test.ts @@ -114,6 +114,42 @@ describe("parseTransformParams", () => { expect(parse("w=640&h=-1").ok).toBe(false); }); + it("parses fit and position from Astro's image service", () => { + const r = parse("w=32&h=32&f=webp&fit=cover&position=center"); + expect(r.ok).toBe(true); + if (r.ok) { + expect(r.options.fit).toBe("cover"); + expect(r.options.position).toBe("center"); + } + }); + + it("drops an unrecognised fit instead of failing the request", () => { + // Astro's ImageFit is open-ended (`string & {}`), so an unknown value + // must not break a rendition -- the backend keeps its own default. + const r = parse("w=32&fit=lopsided"); + expect(r.ok).toBe(true); + if (r.ok) expect(r.options.fit).toBeUndefined(); + }); + + it("accepts the fits Astro's sharp service adds beyond ImageFit", () => { + // `inside`/`outside` are absent from Astro's ImageFit union but accepted + // by its sharp service, so a site can already be using them on Node. + for (const fit of ["inside", "outside"]) { + const r = parse(`w=32&fit=${fit}`); + expect(r.ok).toBe(true); + if (r.ok) expect(r.options.fit).toBe(fit); + } + }); + + it("leaves fit and position undefined when not requested", () => { + const r = parse("w=800"); + expect(r.ok).toBe(true); + if (r.ok) { + expect(r.options.fit).toBeUndefined(); + expect(r.options.position).toBeUndefined(); + } + }); + it("rejects unsupported format and bad quality", () => { expect(parse("w=640&f=gif").ok).toBe(false); expect(parse("w=640&q=0").ok).toBe(false);