From b1eb1d29c5948adb5b3ea9e74cde4594e652a6d5 Mon Sep 17 00:00:00 2001 From: dangreen Date: Thu, 3 Sep 2026 14:33:07 +0400 Subject: [PATCH 1/2] fix(imgproxy,cloudflare): select `src` the way a generation rule does `src`, and with it `url`, was the largest width of the last format of the rule, so the format list had to be written in the opposite order to a build-time rule: the widely supported format first there, last here. It is now the variant of the source format - read from the url extension - at the largest width, falling back to the first format of the list when the source format is not in it. That is the selection a generation rule makes, minus the source width, which is unknown at runtime. --- packages/cloudflare/src/image.spec.ts | 21 ++++++++++++++++++++- packages/cloudflare/src/image.ts | 17 +++++++++++------ packages/cloudflare/src/types.ts | 5 +++-- packages/imgproxy/src/image.spec.ts | 20 +++++++++++++++++++- packages/imgproxy/src/image.ts | 22 +++++++++++----------- packages/imgproxy/src/types.ts | 5 +++-- 6 files changed, 67 insertions(+), 23 deletions(-) diff --git a/packages/cloudflare/src/image.spec.ts b/packages/cloudflare/src/image.spec.ts index 9b17a83..0fadd5d 100644 --- a/packages/cloudflare/src/image.spec.ts +++ b/packages/cloudflare/src/image.spec.ts @@ -121,6 +121,7 @@ describe('cloudflare', () => { }) expect(image.srcSet.map(({ id }) => id)).toEqual(['webp600', 'png600']) + expect(image.src.id).toBe('png600') expect(image.srcMap.png600).toBe('/cdn-cgi/image/width=600/https://cdn.example.com/picture.png') }) @@ -131,7 +132,7 @@ describe('cloudflare', () => { }).srcSet.length).toBe(1) }) - it('should select fallback src of the last format with the largest width', () => { + it('should select fallback src of the source format with the largest width', () => { const image = cloudflare.image(sourceUrl, { width: [300, 1200, 600], format: ['avif', 'webp', 'jpg'] @@ -140,6 +141,24 @@ describe('cloudflare', () => { expect(image.src.id).toBe('jpg1200') }) + it('should select fallback src of the source format wherever it is in the rule', () => { + const image = cloudflare.image(sourceUrl, { + width: [1200, 600], + format: ['jpg', 'webp'] + }) + + expect(image.src.id).toBe('jpg1200') + }) + + it('should select fallback src of the first format without the source format', () => { + const image = cloudflare.image(sourceUrl, { + width: [1200, 600], + format: ['webp', 'avif'] + }) + + expect(image.src.id).toBe('webp1200') + }) + it('should use custom endpoint', () => { const zone = new Cloudflare({ endpoint: 'https://example.com/cdn-cgi/image' diff --git a/packages/cloudflare/src/image.ts b/packages/cloudflare/src/image.ts index 9e29fd1..5a93a39 100644 --- a/packages/cloudflare/src/image.ts +++ b/packages/cloudflare/src/image.ts @@ -39,7 +39,8 @@ export interface CloudflareImage { */ url: string /** - * Fallback image variant: the last format, the largest width. + * Fallback image variant: the source format, or the first format + * of the rule when the source format is not in it, at the largest width. */ src: SrcSetEntry /** @@ -91,13 +92,18 @@ export class Cloudflare { // Cloudflare returns svg sources as is, ignoring all transformations, // so an svg source always passes through; jpg drives the variant loop. const passthrough = this.#passthrough || isSvgSource - const formats: ImageFormat[] = isSvgSource ? ['jpg'] : toArray(rule.format, sourceFormat) + const formats: ImageFormat[] = isSvgSource + ? ['jpg'] + : [...new Set(toArray(rule.format, sourceFormat))] + // Same selection as a build-time rule: the source format, or the first + // format of the list when the source format is not in it. + const srcFormat = formats.includes(sourceFormat) ? sourceFormat : formats[0] const widths = toArray(rule.width) const srcSet: SrcSetEntry[] = [] const srcMap: Record = {} let src: SrcSetEntry | undefined - for (const format of new Set(formats)) { + for (const format of formats) { if (!canOutputFormat(format, sourceFormat)) { throw new TypeError(`Cloudflare can not force the ${format} output format.`) } @@ -127,9 +133,8 @@ export class Cloudflare { srcMap[entry.id] = entry.url } - // Formats go first: on format change the entry starts the next format - // group, so the src candidate ends up in the last group, the largest width. - if (!src || src.format !== entry.format || entry.width > src.width) { + // The `src` variant is the largest width of the selected format. + if (entry.format === srcFormat && (!src || entry.width > src.width)) { src = entry } } diff --git a/packages/cloudflare/src/types.ts b/packages/cloudflare/src/types.ts index b6533d7..6831d15 100644 --- a/packages/cloudflare/src/types.ts +++ b/packages/cloudflare/src/types.ts @@ -49,8 +49,9 @@ export interface CloudflareOptions { */ export interface CloudflareRule { /** - * Output image format(s). The last one is used as the `src` fallback. - * Defaults to the url file extension. + * Output image format(s). Defaults to the url file extension. + * The `src` fallback is the variant of the source format, or of the + * first format of the list when the source format is not in it. */ format?: ImageFormat | ImageFormat[] /** diff --git a/packages/imgproxy/src/image.spec.ts b/packages/imgproxy/src/image.spec.ts index 062fee0..0b3de48 100644 --- a/packages/imgproxy/src/image.spec.ts +++ b/packages/imgproxy/src/image.spec.ts @@ -102,7 +102,7 @@ describe('imgproxy', () => { }).srcSet.length).toBe(1) }) - it('should select fallback src of the last format with the largest width', () => { + it('should select fallback src of the source format with the largest width', () => { const image = imgproxy.image(sourceUrl, { width: [300, 1200, 600], format: ['avif', 'webp', 'jpg'] @@ -111,6 +111,24 @@ describe('imgproxy', () => { expect(image.src.id).toBe('jpg1200') }) + it('should select fallback src of the source format wherever it is in the rule', () => { + const image = imgproxy.image(sourceUrl, { + width: [1200, 600], + format: ['jpg', 'webp'] + }) + + expect(image.src.id).toBe('jpg1200') + }) + + it('should select fallback src of the first format without the source format', () => { + const image = imgproxy.image(sourceUrl, { + width: [1200, 600], + format: ['webp', 'avif'] + }) + + expect(image.src.id).toBe('webp1200') + }) + it('should use custom processing builder', () => { const presets = new Imgproxy({ endpoint: 'https://imgproxy.example.com', diff --git a/packages/imgproxy/src/image.ts b/packages/imgproxy/src/image.ts index 6562418..fb20de0 100644 --- a/packages/imgproxy/src/image.ts +++ b/packages/imgproxy/src/image.ts @@ -50,7 +50,8 @@ export interface ImgproxyImage { */ url: string /** - * Fallback image variant: the last format, the largest width. + * Fallback image variant: the source format, or the first format + * of the rule when the source format is not in it, at the largest width. */ src: SrcSetEntry /** @@ -99,18 +100,18 @@ export class Imgproxy { * @returns Image srcset object. */ image(sourceUrl: string, rule: ImgproxyRule): ImgproxyImage { - const formats = toArray(rule.format, outputFormatFromUrl(sourceUrl)) + const sourceFormat = outputFormatFromUrl(sourceUrl) + // Raster image can't be converted to SVG. + const formats = [...new Set(toArray(rule.format, sourceFormat))].filter(format => format !== 'svg') + // Same selection as a build-time rule: the source format, or the first + // format of the list when the source format is not in it. + const srcFormat = formats.includes(sourceFormat) ? sourceFormat : formats[0] const widths = toArray(rule.width) const srcSet: SrcSetEntry[] = [] const srcMap: Record = {} let src: SrcSetEntry | undefined - for (const format of new Set(formats)) { - // Raster image can't be converted to SVG. - if (format === 'svg') { - continue - } - + for (const format of formats) { for (const width of new Set(widths)) { // The source size is unknown on the client, so multipliers can't be // resolved, and `w` descriptors need integer pixel widths. @@ -136,9 +137,8 @@ export class Imgproxy { srcMap[entry.id] = entry.url } - // Formats go first: on format change the entry starts the next format - // group, so the src candidate ends up in the last group, the largest width. - if (!src || src.format !== entry.format || entry.width > src.width) { + // The `src` variant is the largest width of the selected format. + if (entry.format === srcFormat && (!src || entry.width > src.width)) { src = entry } } diff --git a/packages/imgproxy/src/types.ts b/packages/imgproxy/src/types.ts index 7d12ca3..be637b8 100644 --- a/packages/imgproxy/src/types.ts +++ b/packages/imgproxy/src/types.ts @@ -60,8 +60,9 @@ export interface ImgproxyOptions { */ export interface ImgproxyRule { /** - * Output image format(s). The last one is used as the `src` fallback. - * Defaults to the url file extension. + * Output image format(s). Defaults to the url file extension. + * The `src` fallback is the variant of the source format, or of the + * first format of the list when the source format is not in it. */ format?: ImageFormat | ImageFormat[] /** From 3549ed1678f05498f8b7f934b89cddb6bd295502 Mon Sep 17 00:00:00 2001 From: dangreen Date: Thu, 3 Sep 2026 14:33:34 +0400 Subject: [PATCH 2/2] refactor(imgproxy,cloudflare): deduplicate in `toUniqArray` `toArray` left the deduplication to every call site, which spelled it as `[...new Set(toArray(...))]` and `for (const width of new Set(widths))`. --- packages/cloudflare/src/image.ts | 10 ++++------ packages/cloudflare/src/utils.spec.ts | 18 ++++++++++-------- packages/cloudflare/src/utils.ts | 10 +++++----- packages/imgproxy/src/image.ts | 8 ++++---- packages/imgproxy/src/utils.spec.ts | 18 ++++++++++-------- packages/imgproxy/src/utils.ts | 8 ++++---- 6 files changed, 37 insertions(+), 35 deletions(-) diff --git a/packages/cloudflare/src/image.ts b/packages/cloudflare/src/image.ts index 5a93a39..a29d4c9 100644 --- a/packages/cloudflare/src/image.ts +++ b/packages/cloudflare/src/image.ts @@ -14,7 +14,7 @@ import { buildCloudflareUrl, createDefaultProcessing } from './url.ts' -import { toArray } from './utils.ts' +import { toUniqArray } from './utils.ts' const FORMAT_PATTERN = /\.(\w+)$/ const QUERY_OR_FRAGMENT_PATTERN = /[?#]/ @@ -92,13 +92,11 @@ export class Cloudflare { // Cloudflare returns svg sources as is, ignoring all transformations, // so an svg source always passes through; jpg drives the variant loop. const passthrough = this.#passthrough || isSvgSource - const formats: ImageFormat[] = isSvgSource - ? ['jpg'] - : [...new Set(toArray(rule.format, sourceFormat))] + const formats: ImageFormat[] = isSvgSource ? ['jpg'] : toUniqArray(rule.format, sourceFormat) // Same selection as a build-time rule: the source format, or the first // format of the list when the source format is not in it. const srcFormat = formats.includes(sourceFormat) ? sourceFormat : formats[0] - const widths = toArray(rule.width) + const widths = toUniqArray(rule.width) const srcSet: SrcSetEntry[] = [] const srcMap: Record = {} let src: SrcSetEntry | undefined @@ -108,7 +106,7 @@ export class Cloudflare { throw new TypeError(`Cloudflare can not force the ${format} output format.`) } - for (const width of new Set(widths)) { + for (const width of widths) { // The source size is unknown on the client, so multipliers can't be // resolved, and `w` descriptors need integer pixel widths. if (!Number.isInteger(width) || width <= 1) { diff --git a/packages/cloudflare/src/utils.spec.ts b/packages/cloudflare/src/utils.spec.ts index 7d85640..aa39c69 100644 --- a/packages/cloudflare/src/utils.spec.ts +++ b/packages/cloudflare/src/utils.spec.ts @@ -3,24 +3,26 @@ import { it, expect } from 'vitest' -import { toArray } from './utils.ts' +import { toUniqArray } from './utils.ts' describe('cloudflare', () => { describe('utils', () => { - describe('toArray', () => { - it('should return array as is', () => { - const value = [1, 2] + describe('toUniqArray', () => { + it('should deduplicate an array', () => { + expect(toUniqArray([1, 2, 1])).toEqual([1, 2]) + }) - expect(toArray(value)).toBe(value) + it('should keep the order of the first occurrences', () => { + expect(toUniqArray([2, 1, 2, 3])).toEqual([2, 1, 3]) }) it('should wrap single value', () => { - expect(toArray(1)).toEqual([1]) + expect(toUniqArray(1)).toEqual([1]) }) it('should fall back for undefined', () => { - expect(toArray(undefined, 1)).toEqual([1]) - expect(toArray(undefined)).toEqual([]) + expect(toUniqArray(undefined, 1)).toEqual([1]) + expect(toUniqArray(undefined)).toEqual([]) }) }) }) diff --git a/packages/cloudflare/src/utils.ts b/packages/cloudflare/src/utils.ts index ff4a2aa..74dbc51 100644 --- a/packages/cloudflare/src/utils.ts +++ b/packages/cloudflare/src/utils.ts @@ -1,12 +1,12 @@ /** - * Normalize an optional single-or-array value to an array. + * Normalize an optional single-or-array value to an array of unique values. * @param value - Single value, array, or `undefined`. - * @param fallback - Value for the `undefined` case. - * @returns Array of values. + * @param fallback - Value for the `undefined` case, omitted for an empty array. + * @returns Array of unique values. */ -export function toArray(value: T | T[] | undefined, fallback?: T): T[] { +export function toUniqArray(value: T | T[] | undefined, fallback?: T): T[] { if (Array.isArray(value)) { - return value + return [...new Set(value)] } if (value !== undefined) { diff --git a/packages/imgproxy/src/image.ts b/packages/imgproxy/src/image.ts index fb20de0..60b5226 100644 --- a/packages/imgproxy/src/image.ts +++ b/packages/imgproxy/src/image.ts @@ -13,7 +13,7 @@ import { buildImgproxyUrl, createDefaultProcessing } from './url.ts' -import { toArray } from './utils.ts' +import { toUniqArray } from './utils.ts' const FORMAT_PATTERN = /\.(\w+)$/ const QUERY_OR_FRAGMENT_PATTERN = /[?#]/ @@ -102,17 +102,17 @@ export class Imgproxy { image(sourceUrl: string, rule: ImgproxyRule): ImgproxyImage { const sourceFormat = outputFormatFromUrl(sourceUrl) // Raster image can't be converted to SVG. - const formats = [...new Set(toArray(rule.format, sourceFormat))].filter(format => format !== 'svg') + const formats = toUniqArray(rule.format, sourceFormat).filter(format => format !== 'svg') // Same selection as a build-time rule: the source format, or the first // format of the list when the source format is not in it. const srcFormat = formats.includes(sourceFormat) ? sourceFormat : formats[0] - const widths = toArray(rule.width) + const widths = toUniqArray(rule.width) const srcSet: SrcSetEntry[] = [] const srcMap: Record = {} let src: SrcSetEntry | undefined for (const format of formats) { - for (const width of new Set(widths)) { + for (const width of widths) { // The source size is unknown on the client, so multipliers can't be // resolved, and `w` descriptors need integer pixel widths. if (!Number.isInteger(width) || width <= 1) { diff --git a/packages/imgproxy/src/utils.spec.ts b/packages/imgproxy/src/utils.spec.ts index 653b9e0..05db457 100644 --- a/packages/imgproxy/src/utils.spec.ts +++ b/packages/imgproxy/src/utils.spec.ts @@ -3,24 +3,26 @@ import { it, expect } from 'vitest' -import { toArray } from './utils.ts' +import { toUniqArray } from './utils.ts' describe('imgproxy', () => { describe('utils', () => { - describe('toArray', () => { - it('should return array as is', () => { - const value = [1, 2] + describe('toUniqArray', () => { + it('should deduplicate an array', () => { + expect(toUniqArray([1, 2, 1])).toEqual([1, 2]) + }) - expect(toArray(value)).toBe(value) + it('should keep the order of the first occurrences', () => { + expect(toUniqArray([2, 1, 2, 3])).toEqual([2, 1, 3]) }) it('should wrap single value', () => { - expect(toArray(1)).toEqual([1]) + expect(toUniqArray(1)).toEqual([1]) }) it('should fall back for undefined', () => { - expect(toArray(undefined, 1)).toEqual([1]) - expect(toArray(undefined)).toEqual([]) + expect(toUniqArray(undefined, 1)).toEqual([1]) + expect(toUniqArray(undefined)).toEqual([]) }) }) }) diff --git a/packages/imgproxy/src/utils.ts b/packages/imgproxy/src/utils.ts index 9066388..74dbc51 100644 --- a/packages/imgproxy/src/utils.ts +++ b/packages/imgproxy/src/utils.ts @@ -1,12 +1,12 @@ /** - * Normalize an optional single-or-array value to an array. + * Normalize an optional single-or-array value to an array of unique values. * @param value - Single value, array, or `undefined`. * @param fallback - Value for the `undefined` case, omitted for an empty array. - * @returns Array of values. + * @returns Array of unique values. */ -export function toArray(value: T | T[] | undefined, fallback?: T): T[] { +export function toUniqArray(value: T | T[] | undefined, fallback?: T): T[] { if (Array.isArray(value)) { - return value + return [...new Set(value)] } if (value !== undefined) {