diff --git a/packages/bundler-utils/src/generate.spec.ts b/packages/bundler-utils/src/generate.spec.ts index a75f445..f17db9a 100644 --- a/packages/bundler-utils/src/generate.spec.ts +++ b/packages/bundler-utils/src/generate.spec.ts @@ -1,7 +1,8 @@ import { describe, it, - expect + expect, + vi } from 'vitest' import sharp from 'sharp' import { generateSrcSetModule } from './generate.ts' @@ -118,6 +119,26 @@ describe('bundler-utils', () => { expect(module).toContain('"jpg160"') }) + it('should deduplicate variants of overlapping fallthrough rules', async () => { + const image = await createImage() + const emitImage = vi.fn(emitToPath) + const module = await generateSrcSetModule(image, {}, { + rules: [ + { + fallthrough: true, + width: [0.5, 1] + }, + { + width: [320, 0.25] + } + ] + }, emitImage) + + expect(emitImage).toHaveBeenCalledTimes(3) + expect(module.match(/"jpg320":/g)?.length).toBe(1) + expect(module).toContain('"jpg160"') + }) + it('should inline placeholder data-url', async () => { const image = await createImage() const module = await generateSrcSetModule( @@ -133,6 +154,119 @@ describe('bundler-utils', () => { expect(module).toMatch(/export const placeholder = "data:image\/webp;base64,[^"]+";/) }) + it('should keep the configured placeholder options for the query flag', async () => { + const image = await createImage() + const module = await generateSrcSetModule(image, { + placeholder: true + }, { + skipOptimization: true, + placeholder: { + format: 'jpg' + } + }, emitToPath) + + expect(module).toMatch(/export const placeholder = "data:image\/jpeg;base64,/) + }) + + it('should disable the placeholder from the query', async () => { + const image = await createImage() + const module = await generateSrcSetModule(image, { + placeholder: false + }, { + skipOptimization: true, + placeholder: { + format: 'jpg' + } + }, emitToPath) + + expect(module).toContain('export const placeholder = undefined;') + }) + + it('should select the requested format at the first width of the rule', async () => { + const image = await createImage() + const module = await generateSrcSetModule(image, { + select: { + format: 'webp' + } + }, { + rules: [ + { + format: ['jpg', 'webp'], + width: [1, 0.5] + } + ] + }, emitToPath) + + expect(module).toContain('const url = "/images/image.webp";') + }) + + it('should select the requested width in the first format of the rule', async () => { + const image = await createImage() + const module = await generateSrcSetModule(image, { + select: { + width: 320 + } + }, { + rules: [ + { + format: ['webp', 'jpg'], + width: [1, 0.5] + } + ] + }, emitToPath) + + expect(module).toContain('const url = "/images/image@320w.webp";') + }) + + it('should prefer the select from the query over the options', async () => { + const image = await createImage() + const module = await generateSrcSetModule(image, { + select: { + id: 'webp320' + } + }, { + select: { + id: 'jpg320' + }, + rules: [ + { + format: ['jpg', 'webp'], + width: [0.5] + } + ] + }, emitToPath) + + expect(module).toContain('const url = "/images/image@320w.webp";') + }) + + it('should select the original variant without an explicit select', async () => { + const image = await createImage() + const module = await generateSrcSetModule(image, {}, { + rules: [ + { + format: ['webp', 'jpg'], + width: [1, 0.5] + } + ] + }, emitToPath) + + expect(module).toContain('const url = "/images/image.jpg";') + }) + + it('should fall back to the first variant when nothing matches the select', async () => { + const image = await createImage() + const module = await generateSrcSetModule(image, {}, { + rules: [ + { + format: ['webp', 'jpg'], + width: [0.5] + } + ] + }, emitToPath) + + expect(module).toContain('const url = "/images/image@320w.webp";') + }) + it('should build public path expression without a plain public path', async () => { const image = await createImage() const module = await generateSrcSetModule(image, {}, { diff --git a/packages/bundler-utils/src/generate.ts b/packages/bundler-utils/src/generate.ts index 6c82f71..5a834fe 100644 --- a/packages/bundler-utils/src/generate.ts +++ b/packages/bundler-utils/src/generate.ts @@ -3,7 +3,6 @@ import { type ImageSource, SrcSetGenerator, getImageMetadata, - matchImage, mimeTypes } from '@srcset/core' import type { QueryOptions } from './query.ts' @@ -45,41 +44,46 @@ export async function generateSrcSetModule( limit }) const metadata = await getImageMetadata(source) + // The query flag only switches the placeholder on and off: the configured + // options stay, so `?placeholder` does not fall back to the defaults. + const placeholderOptions = query.placeholder === undefined + ? options.placeholder + : query.placeholder && (options.placeholder ?? true) const placeholder = await createPlaceholder( source, metadata, - query.placeholder ?? options.placeholder, + placeholderOptions, limit, options.cache ) - const select = { - format: metadata.format, - width: metadata.width, + const userSelect = { ...options.select, ...query.select } - const srcSet: SrcSetModuleEntry[] = [] - - for (const rule of rules) { - if (!await matchImage(source, rule.match)) { - continue - } - - for await (const image of generator.generate(source, rule)) { - srcSet.push({ - id: resourceId(image.width, image.originMultiplier ?? image.width, image.format), - format: image.format, - type: mimeTypes[image.format], - width: image.width, - height: image.height, - originMultiplier: image.originMultiplier, - url: emitImage(image) - }) + const hasUserSelect = userSelect.id !== undefined + || userSelect.format !== undefined + || userSelect.width !== undefined + // The implicit selection describes the original image and applies only when + // nothing is selected explicitly: mixed into a partial selection it would + // defeat the half the user did not specify. + const select = hasUserSelect + ? userSelect + : { + format: metadata.format, + width: metadata.width } + const srcSet: SrcSetModuleEntry[] = [] - if (!rule.fallthrough) { - break - } + for await (const image of generator.generateAll(source, rules)) { + srcSet.push({ + id: resourceId(image.width, image.originMultiplier ?? image.width, image.format), + format: image.format, + type: mimeTypes[image.format], + width: image.width, + height: image.height, + originMultiplier: image.originMultiplier, + url: emitImage(image) + }) } return createModuleString(select, srcSet, placeholder) diff --git a/packages/bundler-utils/src/generate.types.ts b/packages/bundler-utils/src/generate.types.ts index 876b79a..c4c568b 100644 --- a/packages/bundler-utils/src/generate.types.ts +++ b/packages/bundler-utils/src/generate.types.ts @@ -1,11 +1,9 @@ import type { SrcSetImage, - SrcSetGeneratorOptions + SrcSetGeneratorOptions, + SrcSetRule } from '@srcset/core' -import type { - SrcSetRule, - SrcSetImagePaths -} from './types.ts' +import type { SrcSetImagePaths } from './types.ts' import type { SrcSetEntrySelect, ResourceIdFormatter diff --git a/packages/bundler-utils/src/placeholder.ts b/packages/bundler-utils/src/placeholder.ts index e1c9186..3681a0a 100644 --- a/packages/bundler-utils/src/placeholder.ts +++ b/packages/bundler-utils/src/placeholder.ts @@ -77,7 +77,9 @@ export async function createPlaceholder( const createImage = async (): Promise => { const contents = await limit(() => { - const pipeline = sharp(source.contents).resize({ + const pipeline = sharp(source.contents, { + autoOrient: true + }).resize({ width, withoutEnlargement: true }) diff --git a/packages/bundler-utils/src/query.ts b/packages/bundler-utils/src/query.ts index 5fe5085..f01c687 100644 --- a/packages/bundler-utils/src/query.ts +++ b/packages/bundler-utils/src/query.ts @@ -1,4 +1,4 @@ -import type { SrcSetRule } from './types.ts' +import type { SrcSetRule } from '@srcset/core' import type { SrcSetEntrySelect } from './module.ts' export interface QueryOptions { diff --git a/packages/bundler-utils/src/types.ts b/packages/bundler-utils/src/types.ts index 2c49981..8fe67cd 100644 --- a/packages/bundler-utils/src/types.ts +++ b/packages/bundler-utils/src/types.ts @@ -1,8 +1,3 @@ -import type { - Matcher, - GenerateOptions -} from '@srcset/core' - /** * Paths of an image emitted on the bundler side. */ @@ -22,18 +17,3 @@ export interface SrcSetImagePaths { */ publicPathExpression?: string } - -/** - * Rule to generate image variants: match options plus generate options. - */ -export interface SrcSetRule extends GenerateOptions { - /** - * Rule(s) to match the image: glob, media query or matcher function. - */ - match?: Matcher | Matcher[] - /** - * Keep matching the rest of the rules after this rule matched. - * By default the first matched rule is the only one applied. - */ - fallthrough?: boolean -} diff --git a/packages/cli/src/args.spec.ts b/packages/cli/src/args.spec.ts index 666f5bf..e226164 100644 --- a/packages/cli/src/args.spec.ts +++ b/packages/cli/src/args.spec.ts @@ -9,6 +9,12 @@ import { parseCliArgs } from './args.ts' describe('cli', () => { describe('args', () => { describe('parseCliArgs', () => { + it('should reject an unrecognised option', () => { + setArgs('images/**/*.jpg', '--formats', 'webp', '--dest', 'dist') + + expect(() => parseCliArgs()).toThrow('Unknown option: "--formats"') + }) + it('should parse sources and options', () => { setArgs( 'images/**/*.jpg', diff --git a/packages/cli/src/args.ts b/packages/cli/src/args.ts index 4347f27..13c2d65 100644 --- a/packages/cli/src/args.ts +++ b/packages/cli/src/args.ts @@ -1,4 +1,7 @@ -import type { ImageFormat } from '@srcset/core' +import type { + ImageFormat, + SrcSetRule +} from '@srcset/core' import { rest, alias, @@ -7,7 +10,6 @@ import { option, readOptions } from 'argue-cli' -import type { SrcSetCliRule } from './types.ts' export const usage = `srcset [...sources] [...options] @@ -28,7 +30,7 @@ export interface CliArgs { help: boolean verbose: boolean | undefined sources: string[] - rule: SrcSetCliRule | null + rule: SrcSetRule | null skipOptimization: boolean | undefined scalingUp: boolean | undefined dest: string | undefined @@ -64,7 +66,7 @@ export function parseCliArgs(): CliArgs { option(alias('config', 'c'), String), option('concurrency', Number) ) - const rule: SrcSetCliRule = { + const rule: SrcSetRule = { ...match && { match }, @@ -75,11 +77,19 @@ export function parseCliArgs(): CliArgs { format: format as ImageFormat[] } } + const sources = rest() + // Whatever the readers did not take stays in `argv`: a leftover flag is a + // typo, and treating it as a glob would quietly match nothing. + const unknownOption = sources.find(source => source.startsWith('-')) + + if (unknownOption) { + throw new Error(`Unknown option: "${unknownOption}".`) + } return { help: Boolean(help), verbose, - sources: rest(), + sources, rule: Object.keys(rule).length ? rule : null, skipOptimization, scalingUp, diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index a709796..04fe76e 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -7,14 +7,14 @@ import { import { loadConfig } from './config.ts' import { run } from './run.ts' -const args = parseCliArgs() +try { + const args = parseCliArgs() -if (args.help) { - console.info(usage) - process.exit(0) -} + if (args.help) { + console.info(usage) + process.exit(0) + } -try { const config = await loadConfig(args.config) const options: SrcSetCliOptions = { ...config, diff --git a/packages/cli/src/run.spec.ts b/packages/cli/src/run.spec.ts index fb38954..6c76340 100644 --- a/packages/cli/src/run.spec.ts +++ b/packages/cli/src/run.spec.ts @@ -128,6 +128,52 @@ describe('cli', () => { expect(written.some(file => file.includes('photo@160w.jpg'))).toBe(true) }) + it('should write a variant of overlapping fallthrough rules once', async () => { + const dir = await createProject() + const written = await runIn(dir, { + src: 'images/**/*.jpg', + dest: 'dist', + skipOptimization: true, + rules: [ + { + fallthrough: true, + width: [0.5] + }, + { + width: [0.5, 0.25] + } + ] + }) + + expect(written.length).toBe(2) + expect(new Set(written).size).toBe(2) + }) + + it('should throw on an output path collision', async () => { + const dir = await createProject() + const contents = await sharp({ + create: { + width: 320, + height: 240, + channels: 3, + background: '#d53a7b' + } + }).jpeg().toBuffer() + + await mkdir(join(dir, 'other')) + await writeFile(join(dir, 'other/photo.jpg'), contents) + + // Both sources sit outside the cwd, so both keep the file name only. + await expect(runIn(join(dir, 'images'), { + src: [join(dir, 'images/photo.jpg'), join(dir, 'other/photo.jpg')], + dest: 'dist', + skipOptimization: true, + rules: [{ + width: [1] + }] + })).rejects.toThrow('collision') + }) + it('should throw without matched sources', async () => { const dir = await createProject() diff --git a/packages/cli/src/run.ts b/packages/cli/src/run.ts index e9f5806..7191965 100644 --- a/packages/cli/src/run.ts +++ b/packages/cli/src/run.ts @@ -12,10 +12,7 @@ import { resolve, sep } from 'node:path' -import { - SrcSetGenerator, - matchImage -} from '@srcset/core' +import { SrcSetGenerator } from '@srcset/core' import { glob } from 'tinyglobby' import type { SrcSetCliOptions } from './types.ts' @@ -49,33 +46,32 @@ export async function run(options: SrcSetCliOptions) { const generator = new SrcSetGenerator(generatorOptions) const written: string[] = [] + const outputPaths = new Set() const processFile = async (file: string) => { const source = { path: resolve(file), contents: await readFile(file) } - for (const rule of rules) { - if (!await matchImage(source, rule.match)) { - continue - } - - for await (const image of generator.generate(source, rule)) { - const outputPath = toOutputPath(dest, image.path) + for await (const image of generator.generateAll(source, rules)) { + const outputPath = toOutputPath(dest, image.path) - await mkdir(dirname(outputPath), { - recursive: true - }) - await writeFile(outputPath, image.contents) - written.push(outputPath) - - if (verbose) { - console.info(`${file} -> ${outputPath}`) - } + // Sources outside the cwd keep their file name only, so two of them + // can resolve to one output path - losing an image without a word + // is worse than stopping. + if (outputPaths.has(outputPath)) { + throw new Error(`Output path collision: "${outputPath}". Run from a directory containing every source, or process them separately.`) } - if (!rule.fallthrough) { - break + outputPaths.add(outputPath) + await mkdir(dirname(outputPath), { + recursive: true + }) + await writeFile(outputPath, image.contents) + written.push(outputPath) + + if (verbose) { + console.info(`${file} -> ${outputPath}`) } } } diff --git a/packages/cli/src/types.ts b/packages/cli/src/types.ts index 412c6c8..4f967bb 100644 --- a/packages/cli/src/types.ts +++ b/packages/cli/src/types.ts @@ -1,24 +1,8 @@ import type { - Matcher, - GenerateOptions, + SrcSetRule, SrcSetGeneratorOptions } from '@srcset/core' -/** - * Rule to generate image variants: match options plus generate options. - */ -export interface SrcSetCliRule extends GenerateOptions { - /** - * Rule(s) to match the image: glob, media query or matcher function. - */ - match?: Matcher | Matcher[] - /** - * Keep matching the rest of the rules after this rule matched. - * By default the first matched rule is the only one applied. - */ - fallthrough?: boolean -} - export interface SrcSetCliOptions extends SrcSetGeneratorOptions { /** * Source image(s) glob patterns. @@ -31,7 +15,7 @@ export interface SrcSetCliOptions extends SrcSetGeneratorOptions { /** * Rules to generate image variants. */ - rules?: SrcSetCliRule[] + rules?: SrcSetRule[] /** * Print processed images. */ diff --git a/packages/core/src/cache.spec.ts b/packages/core/src/cache.spec.ts index 66f2155..6659ff4 100644 --- a/packages/core/src/cache.spec.ts +++ b/packages/core/src/cache.spec.ts @@ -75,14 +75,21 @@ describe('core', () => { const image = createImage() const fn = vi.fn(() => Promise.resolve(image)) const generated = await storage.memo(context, variant, fn) + const { key } = storage.getKey(context, variant) expect(fn).toHaveBeenCalledTimes(1) - expect(generated).toEqual(image) + expect(generated).toEqual({ + ...image, + cacheKey: key + }) const cached = await new SrcSetCacheStorage(dir).memo(context, variant, fn) expect(fn).toHaveBeenCalledTimes(1) - expect(cached).toEqual(image) + expect(cached).toEqual({ + ...image, + cacheKey: key + }) }) it('should miss on different variant or source', async () => { @@ -120,7 +127,7 @@ describe('core', () => { expect(fn).toHaveBeenCalledTimes(2) }) - it('should miss when the stored file is overwritten by a colliding name', async () => { + it('should miss when the stored file is damaged', async () => { const { storage } = await createStorage() const context = createContext() const variant = { @@ -175,7 +182,10 @@ describe('core', () => { const regenerated = await storage.memo(context, variant, fn) expect(fn).toHaveBeenCalledTimes(2) - expect(regenerated).toEqual(createImage()) + expect(regenerated).toEqual({ + ...createImage(), + cacheKey: storage.getKey(context, variant).key + }) }) }) @@ -190,14 +200,33 @@ describe('core', () => { const address = storage.getKey(context, variant) expect(address.key).toMatch(/^[0-9a-f]{64}$/) - expect(address.path).toBe('image.webp') + expect(address.path).toBe(`${address.key}-image.webp`) expect(storage.getKey(context, variant)).toEqual(address) }) it('should use the source file name for the svg passthrough', async () => { const { storage } = await createStorage() + const address = storage.getKey(createContext(), null) - expect(storage.getKey(createContext(), null).path).toBe('image.jpg') + expect(address.path).toBe(`${address.key}-image.jpg`) + }) + + it('should give colliding variant names distinct stored paths', async () => { + const { storage } = await createStorage() + const variant = { + format: 'webp' as const, + width: 0.5 + } + const moved = createContext() + + moved.source.path = '/other/image.jpg' + + const address = storage.getKey(createContext(), variant) + const movedAddress = storage.getKey(moved, variant) + + expect(address.path).not.toBe(movedAddress.path) + expect(address.path.endsWith('-image.webp')).toBe(true) + expect(movedAddress.path.endsWith('-image.webp')).toBe(true) }) }) diff --git a/packages/core/src/cache.ts b/packages/core/src/cache.ts index 9def819..b0ff5d8 100644 --- a/packages/core/src/cache.ts +++ b/packages/core/src/cache.ts @@ -17,7 +17,10 @@ import type { ImageVariant, SrcSetImage } from './types.ts' -import { resolveVariant } from './path.ts' +import { + toPosixPath, + resolveVariant +} from './path.ts' import { assertStoredPath, getTemporaryName, @@ -26,6 +29,20 @@ import { } from './cache.utils.ts' import { environment } from './cache.version.ts' +const storedPathSeparator = '-' + +/** + * Make the stored file path of a variant: the storage is flat, so the + * manifest key prefixes the variant file name - names alone are not + * unique across sources and options. + * @param key - Manifest key of the variant. + * @param name - Variant file name. + * @returns Stored file path. + */ +export function getStoredPath(key: string, name: string) { + return `${key}${storedPathSeparator}${name}` +} + /** * Address of a cached variant: the manifest key and the stored file path. */ @@ -35,7 +52,7 @@ export interface CacheAddress { */ key: string /** - * Stored file path of the variant: the variant file name. + * Stored file path of the variant: the key-prefixed variant file name. */ path: string } @@ -57,8 +74,9 @@ interface CacheEntry { * together with its manifest, and the repeated generation with the same * source, options and variant reads it back instead of processing. * Function options, like custom optimizers, are keyed by their source text. - * The stored files are named by the variant file name from `SrcSetImage.path`, - * and can be read back with `read` and `readStream`. + * The stored files are named by the manifest key and the variant file name + * from `SrcSetImage.path`, and can be read back with `read` and `readStream` + * at the path made by `getStoredPath`. */ export class SrcSetCacheStorage { private readonly dir: string @@ -87,8 +105,7 @@ export class SrcSetCacheStorage { .update(environment) .update(source.contents) .update(serialize({ - // Posix separators, so the keys are stable across platforms. - path: source.path.replaceAll('\\', '/'), + path: toPosixPath(source.path), variant, processing, optimization, @@ -100,7 +117,7 @@ export class SrcSetCacheStorage { return { key, - path: parse(resolveVariant(context, variant).path).base + path: getStoredPath(key, parse(resolveVariant(context, variant).path).base) } } @@ -112,14 +129,15 @@ export class SrcSetCacheStorage { this.read(address.path) ]) - // Variant names are not unique across sources and options: - // a file overwritten by a colliding name is a miss. + // The stored path is keyed, so a mismatch means a damaged + // or half-written file rather than another entry: regenerate. if (entry.hash !== getContentsHash(contents)) { return null } return { path: entry.path, + cacheKey: address.key, contents, format: entry.format, width: entry.width, @@ -176,6 +194,11 @@ export class SrcSetCacheStorage { if (image) { await this.writeEntry(address, image) + + return { + ...image, + cacheKey: address.key + } } return image @@ -183,8 +206,7 @@ export class SrcSetCacheStorage { /** * Write contents to the storage. An existing file is overwritten: - * variant names are not unique across option changes, - * stale contents must not survive. + * a repeated write of the same path carries the same contents. * @param path - Stored file path. * @param contents - File contents. * @returns Stored file path. diff --git a/packages/core/src/generator.spec.ts b/packages/core/src/generator.spec.ts index ea43650..1270d2b 100644 --- a/packages/core/src/generator.spec.ts +++ b/packages/core/src/generator.spec.ts @@ -20,8 +20,10 @@ import type { import { createImage, createAnimatedImage, + createOrientedImage, createSvg, - fixtureWidth + fixtureWidth, + fixtureHeight } from '../test/image.mock.ts' async function generateAll(generator: SrcSetGenerator, source: ImageSource, options?: GenerateOptions) { @@ -207,15 +209,14 @@ describe('core', () => { skipOptimization: true }) const image = await createImage('jpg') - const [w64, x2, x1, w320] = await generateAll(generator, image, { + const [w64, x1, xHalf] = await generateAll(generator, image, { scalingUp: false, - width: [64, 1, 0.5, 320] + width: [64, 1, 0.5] }) expect(w64.originMultiplier).toBeNull() - expect(x2.originMultiplier).toBe(1) - expect(x1.originMultiplier).toBe(0.5) - expect(w320.originMultiplier).toBeNull() + expect(x1.originMultiplier).toBe(1) + expect(xHalf.originMultiplier).toBe(0.5) }) it('should skip scaling up', async () => { @@ -246,6 +247,52 @@ describe('core', () => { expect(images[1].postfix).toBe(`@${fixtureWidth}w`) }) + it('should keep a multiplier and an absolute width of the same target apart', async () => { + const generator = new SrcSetGenerator({ + skipOptimization: true + }) + const image = await createImage('jpg') + const images = await generateAll(generator, image, { + width: [1, fixtureWidth] + }) + + expect(images.map(({ path }) => path)).toEqual([ + '/images/image.jpg', + `/images/image@${fixtureWidth}w.jpg` + ]) + }) + + it('should keep the multiplier variant of a collision in either order', async () => { + const generator = new SrcSetGenerator({ + skipOptimization: true + }) + const image = await createImage('jpg') + const half = fixtureWidth / 2 + const [before] = await generateAll(generator, image, { + width: [0.5, half] + }) + const [after] = await generateAll(generator, image, { + width: [half, 0.5] + }) + + expect(before.originMultiplier).toBe(0.5) + expect(after.originMultiplier).toBe(0.5) + expect(after.path).toBe(before.path) + }) + + it('should deduplicate widths resolving to the same target', async () => { + const generator = new SrcSetGenerator({ + skipOptimization: true + }) + const image = await createImage('jpg') + const images = await generateAll(generator, image, { + width: [1920, 1280, fixtureWidth] + }) + + expect(images.length).toBe(1) + expect(images[0].width).toBe(fixtureWidth) + }) + it('should deduplicate identical variants', async () => { const generator = new SrcSetGenerator({ skipOptimization: true @@ -494,6 +541,125 @@ describe('core', () => { expect(files.length).toBe(4) }) }) + + describe('orientation', () => { + it('should rotate the pixels of an oriented source', async () => { + const generator = new SrcSetGenerator() + const image = await createOrientedImage() + const [variant] = await generateAll(generator, image, { + width: [0.5] + }) + const metadata = await sharp(variant.contents).metadata() + + expect(variant.width).toBe(fixtureHeight / 2) + expect(variant.height).toBe(fixtureWidth / 2) + expect(metadata.width).toBe(fixtureHeight / 2) + expect(metadata.height).toBe(fixtureWidth / 2) + expect(metadata.orientation).toBeUndefined() + }) + + it('should keep every variant of an oriented source in one orientation', async () => { + const generator = new SrcSetGenerator({ + skipOptimization: true + }) + const image = await createOrientedImage() + const images = await generateAll(generator, image, { + width: [1, 0.5] + }) + const rendered = await Promise.all(images.map(async ({ contents }) => { + const { + width, + height, + orientation + } = await sharp(contents).metadata() + + // A browser applies the tag: transposed for orientation 6. + return orientation === 6 ? width < height : width > height + })) + + expect(images.length).toBe(2) + expect(rendered).toEqual([false, false]) + expect(images.map(({ width }) => width)).toEqual([fixtureHeight, fixtureHeight / 2]) + }) + }) + + describe('generateAll', () => { + it('should apply the first matching rule only', async () => { + const generator = new SrcSetGenerator({ + skipOptimization: true + }) + const image = await createImage('jpg') + const images = [] + + for await (const generated of generator.generateAll(image, [ + { + match: '**/*.png', + width: [0.25] + }, + { + width: [0.5] + }, + { + width: [0.75] + } + ])) { + images.push(generated) + } + + expect(images.map(({ path }) => path)).toEqual(['/images/image@320w.jpg']) + }) + + it('should keep matching after a fallthrough rule', async () => { + const generator = new SrcSetGenerator({ + skipOptimization: true + }) + const image = await createImage('jpg') + const images = [] + + for await (const generated of generator.generateAll(image, [ + { + fallthrough: true, + width: [0.5] + }, + { + width: [0.25] + } + ])) { + images.push(generated) + } + + expect(images.map(({ path }) => path)).toEqual([ + '/images/image@320w.jpg', + '/images/image@160w.jpg' + ]) + }) + + it('should produce a file of overlapping rules once', async () => { + const generator = new SrcSetGenerator({ + skipOptimization: true + }) + const image = await createImage('jpg') + const images = [] + + for await (const generated of generator.generateAll(image, [ + { + fallthrough: true, + width: [0.5, 1] + }, + { + width: [320, 0.25] + } + ])) { + images.push(generated) + } + + expect(images.map(({ path }) => path)).toEqual([ + '/images/image@320w.jpg', + '/images/image.jpg', + '/images/image@160w.jpg' + ]) + }) + }) }) }) }) diff --git a/packages/core/src/generator.ts b/packages/core/src/generator.ts index 1b2dbce..2443bbe 100644 --- a/packages/core/src/generator.ts +++ b/packages/core/src/generator.ts @@ -14,7 +14,8 @@ import type { Postfix, SrcSetGeneratorOptions, GenerateOptions, - GenerateContext + GenerateContext, + SrcSetRule } from './types.ts' import { isSupportedFormat, @@ -26,22 +27,54 @@ import { mergeProcessingOptions } from './defaults.ts' import { getImageMetadata } from './metadata.ts' +import { matchImage } from './match.ts' import { resolveVariant } from './path.ts' import { parallel } from './parallel.ts' const animatableFormats = new Set(['gif', 'webp']) -function createVariants(formats: ImageFormat[], widths: number[]) { +function createVariants(context: GenerateContext, formats: ImageFormat[], widths: number[]) { + const { metadata } = context const uniqueFormats = new Set(formats) const uniqueWidths = new Set(widths) const variants: ImageVariant[] = [] + const targets = new Map() for (const format of uniqueFormats) { for (const width of uniqueWidths) { - variants.push({ + const variant = { format, width - }) + } + const { + requestedWidth, + targetWidth, + postfix + } = resolveVariant(context, variant) + + if (!context.scalingUp && requestedWidth > metadata.width) { + continue + } + + // Requested widths above the original resolve to the same target width: + // without this the same image is encoded, stored and emitted twice, + // under one name and with one `w` descriptor. The postfix is a part of + // the identity: a multiplier and an absolute width can resolve to the + // same pixels under different names, and both names are wanted. + const target = `${format}|${postfix}|${targetWidth}` + const index = targets.get(target) + + if (index === undefined) { + targets.set(target, variants.length) + variants.push(variant) + continue + } + + // One file requested both ways: keep the multiplier, it is what + // carries `originMultiplier` for the variant selection. + if (width <= 1 && variants[index].width > 1) { + variants[index] = variant + } } } @@ -175,7 +208,7 @@ export class SrcSetGenerator { return } - const variants = createVariants(formats, widths) + const variants = createVariants(context, formats, widths) yield* parallel( variants, @@ -184,6 +217,42 @@ export class SrcSetGenerator { ) } + /** + * Create set of image variants from the source image by the rules: + * the first matching rule is applied, `fallthrough` keeps matching + * the rest. Rules resolving to one file produce it once, the first + * rule wins - as it does without `fallthrough`. + * @param source - Image file. + * @param rules - Rules to generate image variants. + * @yields Generated image variants. + */ + async* generateAll( + source: ImageSource, + rules: SrcSetRule[] + ): AsyncGenerator { + const paths = new Set() + + for (const rule of rules) { + if (!await matchImage(source, rule.match)) { + continue + } + + for await (const image of this.generate(source, rule)) { + if (paths.has(image.path)) { + continue + } + + paths.add(image.path) + + yield image + } + + if (!rule.fallthrough) { + break + } + } + } + /** * Memoize the variant generation in the cache storage, when configured. * @param context - Generate context. @@ -248,16 +317,10 @@ export class SrcSetGenerator { } = context const isMultiplier = width <= 1 const { - requestedWidth, targetWidth, postfix, path } = resolveVariant(context, variant) - - if (!context.scalingUp && requestedWidth > metadata.width) { - return null - } - const willResize = targetWidth < metadata.width const passthrough = !willResize && format === metadata.format && context.skipOptimization let contents: Buffer @@ -268,8 +331,11 @@ export class SrcSetGenerator { contents = source.contents } else { const animated = metadata.animated && animatableFormats.has(format) + // Browsers honour the EXIF orientation, and re-encoding drops it: + // rotate the pixels instead, so every variant renders the same way. const pipeline = sharp(source.contents, { - animated + animated, + autoOrient: true }) if (willResize) { diff --git a/packages/core/src/match.spec.ts b/packages/core/src/match.spec.ts index c59fa15..a63ad3b 100644 --- a/packages/core/src/match.spec.ts +++ b/packages/core/src/match.spec.ts @@ -51,6 +51,17 @@ describe('core', () => { expect(await matchImage(image, '**/*.png')).toBe(false) }) + it('should match a windows path by path glob', async () => { + const image = await createImage('jpg') + const onWindows = { + ...image, + path: 'C:\\project\\src\\images\\image.jpg' + } + + expect(await matchImage(onWindows, '**/images/*.jpg')).toBe(true) + expect(await matchImage(onWindows, '**/*.png')).toBe(false) + }) + it('should match by function', async () => { const image = await createImage('jpg') diff --git a/packages/core/src/match.ts b/packages/core/src/match.ts index dc8a8f7..9991acc 100644 --- a/packages/core/src/match.ts +++ b/packages/core/src/match.ts @@ -2,6 +2,7 @@ import { match as matchMediaQuery } from 'css-mediaquery' import picomatch from 'picomatch' import type { ImageSource } from './types.ts' import { getImageMetadata } from './metadata.ts' +import { toPosixPath } from './path.ts' /** * Image size in pixels. @@ -59,6 +60,9 @@ export async function matchImage(source: ImageSource, matcher?: Matcher | Matche } const matchers = Array.isArray(matcher) ? matcher : [matcher] + // One glob matches on every platform: the loader and the cli + // feed native paths, vite feeds posix ones. + const path = toPosixPath(source.path) return matchers.every((matcherToApply) => { if (typeof matcherToApply === 'string') { @@ -66,7 +70,7 @@ export async function matchImage(source: ImageSource, matcher?: Matcher | Matche return matchMediaQuery(matcherToApply, size) } - return picomatch(matcherToApply)(source.path) + return picomatch(matcherToApply)(path) } if (typeof matcherToApply === 'function') { diff --git a/packages/core/src/metadata.spec.ts b/packages/core/src/metadata.spec.ts index e703e35..2954d20 100644 --- a/packages/core/src/metadata.spec.ts +++ b/packages/core/src/metadata.spec.ts @@ -7,6 +7,7 @@ import { getImageMetadata } from './metadata.ts' import { createImage, createAnimatedImage, + createOrientedImage, createSvg, fixtureWidth, fixtureHeight @@ -51,6 +52,14 @@ describe('core', () => { expect(metadata.animated).toBe(true) }) + it('should report the size a browser renders for an oriented image', async () => { + const image = await createOrientedImage() + const metadata = await getImageMetadata(image) + + expect(metadata.width).toBe(fixtureHeight) + expect(metadata.height).toBe(fixtureWidth) + }) + it('should throw error on unsupported contents', async () => { await expect(getImageMetadata({ path: '/file.txt', diff --git a/packages/core/src/metadata.ts b/packages/core/src/metadata.ts index 089bf62..f4ccf29 100644 --- a/packages/core/src/metadata.ts +++ b/packages/core/src/metadata.ts @@ -29,7 +29,8 @@ export async function getImageMetadata(source: ImageSource): Promise 1 + // Sharp reports the stored size, but browsers honour the EXIF orientation + // and the generator auto-orients, so the oriented pair is the real size. + // Animated frames keep the page height: orientation does not apply to them. + const metadataWidth = animated ? width : autoOrient.width || width + const metadataHeight = animated ? pageHeight ?? height : autoOrient.height || height - if (!width || !metadataHeight) { + if (!metadataWidth || !metadataHeight) { throw new Error(`Cannot read image dimensions: ${source.path}`) } const metadata: ImageMetadata = { format: normalizedFormat, - width, + width: metadataWidth, height: metadataHeight, - animated: (pages ?? 1) > 1 + animated } cache.set(source.contents, metadata) diff --git a/packages/core/src/path.ts b/packages/core/src/path.ts index bfaad8c..be69a90 100644 --- a/packages/core/src/path.ts +++ b/packages/core/src/path.ts @@ -6,6 +6,16 @@ import type { Postfix } from './types.ts' +/** + * Convert a file path to posix separators, so paths are stable + * across platforms: bundlers feed native and posix paths alike. + * @param filePath - File path in any platform form. + * @returns File path with posix separators. + */ +export function toPosixPath(filePath: string) { + return filePath.replaceAll('\\', '/') +} + /** * Add postfix and format extension to the image file path. * @param imagePath - Source image file path. @@ -14,11 +24,10 @@ import type { * @returns Image variant file path. */ export function renameImagePath(imagePath: string, postfix: string, format: ImageFormat) { - // Posix separators, so variant paths are stable across platforms. const { dir, name - } = path.parse(imagePath.replaceAll('\\', '/')) + } = path.parse(toPosixPath(imagePath)) return path.format({ dir, @@ -84,7 +93,7 @@ export function resolveVariant(context: GenerateContext, variant: ImageVariant | requestedWidth: metadata.width, targetWidth: metadata.width, postfix: '', - path: source.path.replaceAll('\\', '/') + path: toPosixPath(source.path) } } diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 858f578..a126e6e 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -7,6 +7,7 @@ import type { } from 'sharp' import type { LimitFunction } from 'p-limit' import type { ImageFormat } from './formats.ts' +import type { Matcher } from './match.ts' import type { SrcSetCacheStorage } from './cache.ts' /** @@ -77,6 +78,12 @@ export interface SrcSetImage { * Width multiplier relative to the original image, if the variant was requested with one. */ originMultiplier: number | null + /** + * Manifest key of the variant, set when it went through the cache storage. + * Addresses the stored file, which the variant file name alone does not: + * names are not unique across sources and options. + */ + cacheKey?: string } /** @@ -176,6 +183,21 @@ export interface GenerateOptions extends Omit { + const contents = await createNoisePipeline(width, height) + .withMetadata({ + orientation: 6 + }) + .jpeg({ + quality: 90 + }) + .toBuffer() + + return { + path: '/images/oriented.jpg', + contents + } +} + /** * Create an animated gif fixture in memory. * @param width - Frame width. diff --git a/packages/loader/README.md b/packages/loader/README.md index 865f849..1cb0805 100644 --- a/packages/loader/README.md +++ b/packages/loader/README.md @@ -60,9 +60,10 @@ export default { loader: '@srcset/loader', options: { rules: [ + // First format is the fallback: default export and src { width: [1, 0.5], - format: ['avif', 'webp', 'jpg'] + format: ['jpg', 'webp', 'avif'] } ], placeholder: true diff --git a/packages/loader/src/index.ts b/packages/loader/src/index.ts index 5974c06..be740d9 100644 --- a/packages/loader/src/index.ts +++ b/packages/loader/src/index.ts @@ -3,9 +3,9 @@ export { default, raw } from './loader.ts' +export type { SrcSetRule } from '@srcset/core' export type * from './types.ts' export type { - SrcSetRule, SrcSetEntrySelect, ResourceIdFormatter, PlaceholderOptions diff --git a/packages/loader/src/loader.spec.ts b/packages/loader/src/loader.spec.ts index d27a1f6..0ba3086 100644 --- a/packages/loader/src/loader.spec.ts +++ b/packages/loader/src/loader.spec.ts @@ -140,6 +140,21 @@ describe('loader', () => { expect(assets).toEqual(['image.jpg']) }) + it('should keep the source extension of a converted image in development mode', async () => { + const dir = await createFixtureProject(defaultEntry) + const { assets } = await compile(createCompiler, dir, { + skipOptimization: true, + rules: [ + { + width: [1], + format: ['webp'] + } + ] + }, 'development') + + expect(assets).toEqual(['image.jpg.webp']) + }) + it('should export placeholder data-url when enabled', async () => { const dir = await createFixtureProject(defaultEntry) const { exports } = await compile(createCompiler, dir, { diff --git a/packages/loader/src/loader.ts b/packages/loader/src/loader.ts index 97edeab..3bac385 100644 --- a/packages/loader/src/loader.ts +++ b/packages/loader/src/loader.ts @@ -1,11 +1,10 @@ /* oxlint-disable import/no-default-export */ -import type { LoaderContext } from 'webpack' import type { SrcSetImage } from '@srcset/core' import { parseResourceQuery, generateSrcSetModule } from '@srcset/bundler-utils' -import type { SrcSetLoaderOptions } from './types.ts' +import type { SrcSetLoaderContext } from './types.ts' import { interpolateName, getDefaultName @@ -16,7 +15,7 @@ import { } from './paths.ts' import { getSharedLimit } from './limit.ts' -async function generateModule(ctx: LoaderContext, contents: Buffer) { +async function generateModule(ctx: SrcSetLoaderContext, contents: Buffer) { const options = ctx.getOptions() const { context = ctx.rootContext, @@ -67,7 +66,7 @@ async function generateModule(ctx: LoaderContext, contents: * Webpack and Rspack loader for generating responsive images. * @param contents - Source image contents. */ -export default function srcSetLoader(this: LoaderContext, contents: Buffer) { +export default function srcSetLoader(this: SrcSetLoaderContext, contents: Buffer) { const callback = this.async() generateModule(this, contents).then( diff --git a/packages/loader/src/template.spec.ts b/packages/loader/src/template.spec.ts index 3fcf81c..9b8e592 100644 --- a/packages/loader/src/template.spec.ts +++ b/packages/loader/src/template.spec.ts @@ -36,6 +36,33 @@ describe('loader', () => { })).toBe('photo.webp') }) + it('should replace sourceext token for a converted image', () => { + expect(interpolateName('[name][postfix][sourceext].[ext]', context)).toBe('photo@320w.jpg.webp') + }) + + it('should drop sourceext token when the format is kept', () => { + expect(interpolateName('[name][postfix][sourceext].[ext]', { + ...context, + format: 'jpg' + })).toBe('photo@320w.jpg') + expect(interpolateName('[name][sourceext].[ext]', { + ...context, + resourcePath: '/project/src/photo.jpeg', + format: 'jpg' + })).toBe('photo.jpg') + }) + + it('should keep converted siblings apart with the default development name', () => { + const jpg = interpolateName(defaultDevelopmentName, context) + const png = interpolateName(defaultDevelopmentName, { + ...context, + resourcePath: '/project/src/images/photo.png' + }) + + expect(jpg).toBe('images/photo@320w.jpg.webp') + expect(png).toBe('images/photo@320w.png.webp') + }) + it('should replace contenthash token with given length', () => { const name = interpolateName('[contenthash:12].[ext]', context) diff --git a/packages/loader/src/template.ts b/packages/loader/src/template.ts index c61f34f..4cfcc41 100644 --- a/packages/loader/src/template.ts +++ b/packages/loader/src/template.ts @@ -5,6 +5,7 @@ import { relative, sep } from 'node:path' +import { normalizeFormat } from '@srcset/core' export interface TemplateContext { /** @@ -30,12 +31,13 @@ export interface TemplateContext { } export const defaultProductionName = '[name][postfix].[contenthash:8].[ext]' -export const defaultDevelopmentName = '[path][name][postfix].[ext]' +export const defaultDevelopmentName = '[path][name][postfix][sourceext].[ext]' /** * Get default output file name template for the compilation mode. * Development template skips the content hash: readable and stable names, - * cache busting is not needed there. Uniqueness comes from the `[path]` prefix. + * cache busting is not needed there. Uniqueness comes from the `[path]` + * prefix and, for converted images, from `[sourceext]`. * @param mode - Compilation mode. * @returns File name template. */ @@ -48,14 +50,16 @@ const hashPattern = /\[(?:content)?hash(?::(\d+))?\]/g /** * Interpolate output file name template. - * @param template - File name template with `[name]`, `[postfix]`, `[ext]`, `[path]`, `[hash]`/`[contenthash]` tokens. + * @param template - File name template with `[name]`, `[postfix]`, `[ext]`, + * `[path]`, `[sourceext]`, `[hash]`/`[contenthash]` tokens. * @param context - Template context. * @returns Interpolated file name. */ export function interpolateName(template: string, context: TemplateContext) { const { dir, - name + name, + ext } = parse(context.resourcePath) const relativeDir = relative(context.context, dir) // Resources outside the context get no path prefix: no parent @@ -63,11 +67,16 @@ export function interpolateName(template: string, context: TemplateContext) { const dirPath = relativeDir && !relativeDir.startsWith('..') && !isAbsolute(relativeDir) ? `${relativeDir.replaceAll(sep, '/')}/` : '' + // Converted variants of same-named siblings would share a name without it, + // e.g. `hero.jpg` and `hero.png` both become `hero.webp`. Without a + // conversion it would only repeat the output extension, so it stays empty. + const sourceExt = normalizeFormat(ext.slice(1).toLowerCase()) === context.format ? '' : ext let hash: string | null = null return template .replaceAll('[path]', dirPath) .replaceAll('[name]', name) + .replaceAll('[sourceext]', sourceExt) .replaceAll('[postfix]', context.postfix) .replaceAll('[ext]', context.format) .replace(hashPattern, (_, length: string | undefined) => { diff --git a/packages/loader/src/types.ts b/packages/loader/src/types.ts index 7c979b9..ad5ac11 100644 --- a/packages/loader/src/types.ts +++ b/packages/loader/src/types.ts @@ -9,10 +9,52 @@ import type { SrcSetModuleOptions } from '@srcset/bundler-utils' */ export type PathResolver = (url: string, resourcePath: string, context: string) => string +/** + * The part of the webpack and rspack loader context the loader uses. + * Declared structurally: both compilers satisfy it, and the published + * types stay usable without `webpack` installed - it is an optional peer. + */ +export interface SrcSetLoaderContext { + /** + * Absolute path of the source image file. + */ + resourcePath: string + /** + * Import query string of the source image, with the leading `?`. + */ + resourceQuery: string + /** + * Root context directory of the compiler. + */ + rootContext: string + /** + * Compiler mode. + */ + mode: string | undefined + /** + * Read the loader options. + * @returns Loader options. + */ + getOptions(): SrcSetLoaderOptions + /** + * Emit a file to the build output. + * @param name - Output file path. + * @param content - File contents. + */ + emitFile(name: string, content: string | Buffer): void + /** + * Switch the loader to the asynchronous mode. + * @returns Completion callback. + */ + async(): (error?: Error | null, content?: string | Buffer) => void +} + export interface SrcSetLoaderOptions extends SrcSetModuleOptions { /** * Output file name template. - * Supports `[name]`, `[postfix]`, `[ext]`, `[path]` and `[hash]`/`[contenthash]` (with optional `:length`) tokens. + * Supports `[name]`, `[postfix]`, `[ext]`, `[path]`, `[sourceext]` and + * `[hash]`/`[contenthash]` (with optional `:length`) tokens. `[sourceext]` + * is the source file extension, empty when the output format matches it. */ name?: string /** diff --git a/packages/preact/package.json b/packages/preact/package.json index 90538c0..305d646 100644 --- a/packages/preact/package.json +++ b/packages/preact/package.json @@ -68,7 +68,7 @@ }, "peerDependencies": { "@srcset/runtime": "workspace:^", - "preact": ">=10.11" + "preact": ">=10.28" }, "devDependencies": { "@size-limit/preset-small-lib": "^12.0.0", @@ -77,7 +77,7 @@ "@testing-library/jest-dom": "^6.6.3", "@testing-library/preact": "^3.2.4", "happy-dom": "^20.0.0", - "preact": "^10.25.0", + "preact": "^10.28.0", "size-limit": "^12.0.0" } } diff --git a/packages/preact/src/Image.tsx b/packages/preact/src/Image.tsx index d0811ea..0f98a7a 100644 --- a/packages/preact/src/Image.tsx +++ b/packages/preact/src/Image.tsx @@ -15,7 +15,7 @@ import { function applyRef(ref: Ref | undefined, node: T | null) { if (typeof ref === 'function') { - // Preact 10.23+ callback refs may return a cleanup function: pass it through. + // Preact 10.27+ callback refs may return a cleanup function: pass it through. return ref(node) } diff --git a/packages/vite-plugin/README.md b/packages/vite-plugin/README.md index 3541cc7..fd50341 100644 --- a/packages/vite-plugin/README.md +++ b/packages/vite-plugin/README.md @@ -58,15 +58,16 @@ export default defineConfig({ plugins: [ srcset({ rules: [ + // First format is the fallback: default export and src { match: '**/*.jpg', width: [1, 0.5], - format: ['avif', 'webp', 'jpg'] + format: ['jpg', 'webp', 'avif'] }, { match: '**/*.gif', width: [1, 0.5], - format: ['webp', 'gif'] + format: ['gif', 'webp'] } ], placeholder: true diff --git a/packages/vite-plugin/src/dev.spec.ts b/packages/vite-plugin/src/dev.spec.ts index f9014f1..4a09cc5 100644 --- a/packages/vite-plugin/src/dev.spec.ts +++ b/packages/vite-plugin/src/dev.spec.ts @@ -8,6 +8,7 @@ import type { IncomingMessage, ServerResponse } from 'node:http' +import type { ReadStream } from 'node:fs' import { Writable } from 'node:stream' import { mkdtemp, @@ -15,9 +16,16 @@ import { } from 'node:fs/promises' import { tmpdir } from 'node:os' import path from 'node:path' -import { SrcSetCacheStorage } from '@srcset/core' +import { + SrcSetCacheStorage, + getStoredPath +} from '@srcset/core' import { createDevMiddleware } from './dev.ts' +const key = 'a'.repeat(64) +// Larger than the stream watermark, so the response cannot drain in one tick. +const oversizedContents = 1024 * 1024 + async function createStorage() { const dir = await mkdtemp(path.join(tmpdir(), 'srcset-dev-')) @@ -62,11 +70,11 @@ describe('vite-plugin', () => { const { storage } = await createStorage() const contents = Buffer.from('variant') - await storage.write('image.webp', contents) + await storage.write(getStoredPath(key, 'image.webp'), contents) const middleware = createDevMiddleware(storage) const request = { - url: '/@srcset/image.webp' + url: `/@srcset/${key}/image.webp` } as IncomingMessage const { response, @@ -90,12 +98,12 @@ describe('vite-plugin', () => { storage } = await createStorage() - await storage.write('image.webp', Buffer.from('variant')) - await rm(path.join(dir, 'image.webp')) + await storage.write(getStoredPath(key, 'image.webp'), Buffer.from('variant')) + await rm(path.join(dir, getStoredPath(key, 'image.webp'))) const middleware = createDevMiddleware(storage) const request = { - url: '/@srcset/image.webp' + url: `/@srcset/${key}/image.webp` } as IncomingMessage const { response, @@ -115,11 +123,11 @@ describe('vite-plugin', () => { const { storage } = await createStorage() const contents = Buffer.from('variant') - await storage.write('image.webp', contents) + await storage.write(getStoredPath(key, 'image.webp'), contents) const middleware = createDevMiddleware(storage, '/assets/') const request = { - url: '/assets/@srcset/image.webp' + url: `/assets/@srcset/${key}/image.webp` } as IncomingMessage const { response, @@ -139,11 +147,11 @@ describe('vite-plugin', () => { const { storage } = await createStorage() const contents = Buffer.from('variant') - await storage.write('image.webp', contents) + await storage.write(getStoredPath(key, 'image.webp'), contents) const middleware = createDevMiddleware(storage) const request = { - url: '/@srcset/image.webp?v=1' + url: `/@srcset/${key}/image.webp?v=1` } as IncomingMessage const { response, @@ -159,6 +167,62 @@ describe('vite-plugin', () => { expect(body()).toEqual(contents) }) + it('should destroy the stream when the request is aborted', async () => { + const { storage } = await createStorage() + const streams: ReadStream[] = [] + const readStream = storage.readStream.bind(storage) + + await storage.write(getStoredPath(key, 'image.webp'), Buffer.alloc(oversizedContents)) + + vi.spyOn(storage, 'readStream').mockImplementation((path: string) => { + const stream = readStream(path) + + streams.push(stream) + + return stream + }) + + const middleware = createDevMiddleware(storage) + const { response } = createResponse() + + middleware({ + url: `/@srcset/${key}/image.webp` + } as IncomingMessage, response, vi.fn()) + response.destroy() + + await new Promise((resolve) => { + streams[0].on('close', () => { + resolve() + }) + }) + + expect(streams[0].destroyed).toBe(true) + }) + + it('should serve same-named variants of different sources apart', async () => { + const { storage } = await createStorage() + const otherKey = 'b'.repeat(64) + + await storage.write(getStoredPath(key, 'image.webp'), Buffer.from('first')) + await storage.write(getStoredPath(otherKey, 'image.webp'), Buffer.from('second')) + + const middleware = createDevMiddleware(storage) + const first = createResponse() + const second = createResponse() + + middleware({ + url: `/@srcset/${key}/image.webp` + } as IncomingMessage, first.response, vi.fn()) + middleware({ + url: `/@srcset/${otherKey}/image.webp` + } as IncomingMessage, second.response, vi.fn()) + + await Promise.all([first.finished, second.finished]) + + expect(first.body()).toEqual(Buffer.from('first')) + expect(second.body()).toEqual(Buffer.from('second')) + }) + it('should pass foreign and unsafe urls to the next handler', async () => { const { storage } = await createStorage() const middleware = createDevMiddleware(storage) @@ -171,16 +235,19 @@ describe('vite-plugin', () => { url: '/api?next=/@srcset/image.webp' } as IncomingMessage, createResponse().response, next) middleware({ - url: '/@srcset/%' + url: `/@srcset/${key}/%` } as IncomingMessage, createResponse().response, next) middleware({ - url: '/@srcset/..%2Fsecret.jpg' + url: `/@srcset/${key}/..%2Fsecret.jpg` } as IncomingMessage, createResponse().response, next) middleware({ - url: '/@srcset/manifest.json' + url: `/@srcset/${key}/manifest.json` + } as IncomingMessage, createResponse().response, next) + middleware({ + url: '/@srcset/image.webp' } as IncomingMessage, createResponse().response, next) - expect(next).toHaveBeenCalledTimes(5) + expect(next).toHaveBeenCalledTimes(6) }) }) }) diff --git a/packages/vite-plugin/src/dev.ts b/packages/vite-plugin/src/dev.ts index 97fb570..33edf25 100644 --- a/packages/vite-plugin/src/dev.ts +++ b/packages/vite-plugin/src/dev.ts @@ -10,6 +10,7 @@ import { import { type SrcSetCacheStorage, type SrcSetImage, + getStoredPath, mimeTypes } from '@srcset/core' @@ -19,14 +20,19 @@ export const devPathPrefix = '/@srcset/' const relativeDevPathPrefix = devPathPrefix.slice(1) /** - * Make the dev server path of the variant, without the leading slash. - * The name is encoded: browsers percent-encode special characters - * in requests, the middleware matches the decoded form. - * @param image - Image variant. + * Make the dev server path of the variant, without the leading slash: + * the manifest key addresses the stored file, the variant name keeps + * the url readable. The name is encoded: browsers percent-encode special + * characters in requests, the middleware matches the decoded form. + * @param image - Image variant, memoized in the cache storage. * @returns Dev server path of the variant. */ export function getDevPath(image: SrcSetImage) { - return `${relativeDevPathPrefix}${encodeURIComponent(basename(image.path))}` + if (!image.cacheKey) { + throw new Error(`Image variant "${image.path}" is not stored in the cache storage.`) + } + + return `${relativeDevPathPrefix}${image.cacheKey}/${encodeURIComponent(basename(image.path))}` } /** @@ -40,7 +46,7 @@ export function createDevMiddleware(storage: SrcSetCacheStorage, base = '/') { const prefix = base + relativeDevPathPrefix return (request: IncomingMessage, response: ServerResponse, next: () => void) => { - let fileName: string + let storedPath: string // The prefix is matched on the pathname only: a prefix inside // a query string of a foreign route is not ours. Url parsing also @@ -53,14 +59,23 @@ export function createDevMiddleware(storage: SrcSetCacheStorage, base = '/') { return } - fileName = decodeURIComponent(pathname.slice(prefix.length)) + // The address is `/`: the name alone + // is not unique across sources and options. + const [key, name, ...rest] = pathname.slice(prefix.length).split('/') + + if (!key || !name || rest.length) { + next() + return + } + + storedPath = getStoredPath(key, decodeURIComponent(name)) } catch { // Invalid url or malformed percent-encoding: not ours. next() return } - const format = extname(fileName).slice(1) as keyof typeof mimeTypes + const format = extname(storedPath).slice(1) as keyof typeof mimeTypes // The middleware serves plain variant files only. if (!Object.hasOwn(mimeTypes, format)) { @@ -71,7 +86,7 @@ export function createDevMiddleware(storage: SrcSetCacheStorage, base = '/') { let stream: ReadStream try { - stream = storage.readStream(fileName) + stream = storage.readStream(storedPath) } catch { // The storage rejects unsafe paths: not ours to serve. next() @@ -83,6 +98,11 @@ export function createDevMiddleware(storage: SrcSetCacheStorage, base = '/') { response.statusCode = 404 response.end() }) + // A client leaving mid-response would otherwise leak the file descriptor + // for the lifetime of the dev server. + response.on('close', () => { + stream.destroy() + }) response.setHeader('Content-Type', mimeTypes[format]) response.setHeader('Cache-Control', 'no-cache') stream.pipe(response) diff --git a/packages/vite-plugin/src/index.ts b/packages/vite-plugin/src/index.ts index ae4d646..1b7fb5b 100644 --- a/packages/vite-plugin/src/index.ts +++ b/packages/vite-plugin/src/index.ts @@ -1,7 +1,7 @@ export { srcset } from './plugin.ts' +export type { SrcSetRule } from '@srcset/core' export type * from './types.ts' export type { - SrcSetRule, SrcSetEntrySelect, ResourceIdFormatter, PlaceholderOptions diff --git a/packages/vite-plugin/src/plugin.spec.ts b/packages/vite-plugin/src/plugin.spec.ts index 7cd3197..69f5e06 100644 --- a/packages/vite-plugin/src/plugin.spec.ts +++ b/packages/vite-plugin/src/plugin.spec.ts @@ -207,7 +207,7 @@ export default logo const port = typeof address === 'object' && address ? address.port : 0 const exports = await server.ssrLoadModule('/entry.js') as ModuleExports - expect(exports.default).toBe('/assets/@srcset/image.jpg') + expect(exports.default).toMatch(/^\/assets\/@srcset\/[0-9a-f]{64}\/image\.jpg$/) const response = await fetch(`http://localhost:${port}${exports.default}`) @@ -217,6 +217,54 @@ export default logo } }) + it('should serve same-named sources apart', async () => { + const dir = await createFixtureProject(`import a from './a/logo.jpg' +import b from './b/logo.jpg' +export { a as default, b as src } +`) + + await mkdir(path.join(dir, 'a')) + await mkdir(path.join(dir, 'b')) + await copyFile(path.join(dir, 'image.jpg'), path.join(dir, 'a', 'logo.jpg')) + await sharp({ + create: { + width: imageWidth, + height: imageHeight, + channels: 3, + background: '#3a7bd5' + } + }).jpeg().toFile(path.join(dir, 'b', 'logo.jpg')) + + const server = await createServer({ + configFile: false, + logLevel: 'error', + root: dir, + plugins: [srcset({ + skipOptimization: true + })] + }) + + try { + await server.listen() + + const address = server.httpServer?.address() + const port = typeof address === 'object' && address ? address.port : 0 + const exports = await server.ssrLoadModule('/entry.js') as ModuleExports + const secondUrl = exports.src as unknown as string + + expect(exports.default).not.toBe(secondUrl) + + const [first, second] = await Promise.all([ + fetch(`http://localhost:${port}${exports.default}`).then(response => response.arrayBuffer()), + fetch(`http://localhost:${port}${secondUrl}`).then(response => response.arrayBuffer()) + ]) + + expect(Buffer.from(first).equals(Buffer.from(second))).toBe(false) + } finally { + await server.close() + } + }) + it('should serve stable variants across server restarts', async () => { const dir = await createFixtureProject(ruleEntry) const load = async () => { @@ -264,7 +312,7 @@ export default logo const halfWidth = imageWidth / 2 expect(exports.srcSet.length).toBe(4) - expect(exports.default).toBe('/@srcset/image.jpg') + expect(exports.default).toMatch(/^\/@srcset\/[0-9a-f]{64}\/image\.jpg$/) const webpUrl = exports.srcMap[`webp${halfWidth}`] const response = await fetch(`http://localhost:${port}${webpUrl}`) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3f9d1e2..260faad 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -219,7 +219,7 @@ importers: specifier: ^20.0.0 version: 20.11.1 preact: - specifier: ^10.25.0 + specifier: ^10.28.0 version: 10.29.8 size-limit: specifier: ^12.0.0