ReviewOS

also looking at this

stacks/stx

fix(images): name delivery variants independently of the output path

#1961
Merged chris wants to merge fix/stable-image-delivery-names into main
2 files +102 -2
packages/stx/src/builtins/image-delivery.tsmodified+59-1
Changes to packages/stx/src/builtins/image-delivery.ts
@@ -1,4 +1,4 @@
1import type { ImageDeliveryManifest } from 'ts-images/delivery'
1import type { ImageDeliveryManifest, ImageDeliveryStorage } from 'ts-images/delivery'
22import { createHash } from 'node:crypto'
33import fs from 'node:fs'
44import path from 'node:path'
@@ -25,6 +25,63 @@ function catalogName(relativePath: string): string {
2525 return `${readable || 'image'}-${pathHash}`
2626}
2727
28/**
29 * Where delivery variants are written, and — crucially — under what identity.
30 *
31 * ts-images folds `storage.cacheNamespace` into the content hash that names
32 * every variant, and its built-in local adapter builds that namespace out of
33 * the absolute output directory. That is stable for a checkout that lives in
34 * one place forever and wrong for anything deploying atomic releases, where
35 * the same tree is served from a new absolute path every time:
36 *
37 * releases/<sha-1>/…/_stx/images -> hero-6f21e0aa-44338d78039b57e4-640.webp
38 * releases/<sha-2>/…/_stx/images -> hero-6f21e0aa-301f828bdc402f4a-640.webp
39 *
40 * Same bytes, same encode options, different filename — so `stat()` misses for
41 * every variant, a warm cache carried over from the previous release is never
42 * read, and each deploy re-encodes the entire public directory while the old
43 * generation stays on disk forever. On a site with a couple of hundred source
44 * images that is minutes of CPU during which the server is bound but cannot
45 * answer, repeated per deploy, plus unbounded disk growth.
46 *
47 * The namespace below describes what the variant *is* — the URL space it is
48 * published into — and not where this particular release happens to write it.
49 * Two builds of identical bytes therefore agree on the filename, which is what
50 * makes the on-disk cache reusable at all.
51 */
52function deliveryStorage(outDir: string): ImageDeliveryStorage {
53 const url = (key: string) => `${DELIVERY_URL}/${key.split('/').map(encodeURIComponent).join('/')}`
54
55 return {
56 cacheNamespace: `stx:${DELIVERY_URL}`,
57 async stat(key) {
58 const file = path.join(outDir, key)
59 try {
60 const stats = await fs.promises.stat(file)
61 // A zero-byte file is a half-finished write from a previous run that
62 // was killed mid-encode. Treat it as absent so it gets rewritten
63 // rather than served as a broken image forever.
64 return stats.size > 0 ? { bytes: stats.size, path: file, url: url(key) } : null
65 }
66 catch {
67 return null
68 }
69 },
70 async write(key, bytes) {
71 const file = path.join(outDir, key)
72 await fs.promises.mkdir(path.dirname(file), { recursive: true })
73 // Write-then-rename: a reader that arrives mid-write sees either the
74 // old file or the new one, never a truncated one. The delivery directory
75 // is commonly shared between a running server and a build.
76 const pending = `${file}.${process.pid}.tmp`
77 await fs.promises.writeFile(pending, bytes)
78 await fs.promises.rename(pending, file)
79 return { bytes: bytes.byteLength, path: file, url: url(key) }
80 },
81 url,
82 }
83}
84
2885async function collectRasterImages(root: string, directory = root): Promise<Array<{ absolutePath: string, relativePath: string }>> {
2986 const entries = await fs.promises.readdir(directory, { withFileTypes: true })
3087 entries.sort((a, b) => a.name.localeCompare(b.name))
@@ -122,6 +179,7 @@ export async function prepareImageDelivery(
122179
123180 const catalogOptions = {
124181 outDir: path.join(outputDir, '_stx', 'images'),
182 storage: deliveryStorage(path.join(outputDir, '_stx', 'images')),
125183 baseUrl: DELIVERY_URL,
126184 widths: DEFAULT_WIDTHS,
127185 formats: ['avif', 'webp'] as const,
packages/stx/test/builtins/image-delivery.test.tsmodified+43-1
Changes to packages/stx/test/builtins/image-delivery.test.ts
@@ -1,5 +1,5 @@
11import { afterAll, beforeAll, describe, expect, it } from 'bun:test'
2import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'
2import { mkdir, mkdtemp, readdir, rm, stat, writeFile } from 'node:fs/promises'
33import { tmpdir } from 'node:os'
44import { join } from 'node:path'
55import { encode } from 'ts-images'
@@ -103,4 +103,46 @@ describe('build-time Image delivery', () => {
103103 expect(html).toContain(':src="avatarUrl"')
104104 expect(html).not.toContain('<picture')
105105 })
106
107 it('names variants the same from any output directory, so releases reuse the cache', async () => {
108 // An atomic-release deploy runs the exact same build from a new absolute
109 // path each time. If the output directory leaks into the variant names,
110 // every release re-encodes the whole public directory and the previous
111 // release's output is dead weight on disk.
112 const releaseOne = join(tempDir, 'releases', 'aaaaaaa', 'delivery')
113 const releaseTwo = join(tempDir, 'releases', 'bbbbbbb', 'delivery')
114
115 clearImageDeliveryCatalog()
116 const first = await prepareImageDelivery(publicDir, releaseOne)
117 const firstNames = (await readdir(join(releaseOne, '_stx', 'images'))).sort()
118
119 clearImageDeliveryCatalog()
120 const second = await prepareImageDelivery(publicDir, releaseTwo)
121 const secondNames = (await readdir(join(releaseTwo, '_stx', 'images'))).sort()
122
123 expect(firstNames.length).toBeGreaterThan(0)
124 expect(secondNames).toEqual(firstNames)
125 expect(second.fingerprint).toBe(first.fingerprint)
126 })
127
128 it('reuses variants already on disk instead of re-encoding them', async () => {
129 const outDir = join(tempDir, 'releases', 'ccccccc', 'delivery')
130
131 clearImageDeliveryCatalog()
132 await prepareImageDelivery(publicDir, outDir)
133 const written = join(outDir, '_stx', 'images')
134 const before = await Promise.all(
135 (await readdir(written)).sort().map(async name => [name, (await stat(join(written, name))).mtimeMs] as const),
136 )
137
138 clearImageDeliveryCatalog()
139 await prepareImageDelivery(publicDir, outDir)
140 const after = await Promise.all(
141 (await readdir(written)).sort().map(async name => [name, (await stat(join(written, name))).mtimeMs] as const),
142 )
143
144 // Same names and untouched mtimes: the second pass found every variant and
145 // wrote nothing. A miss would rewrite the file and move the mtime.
146 expect(after).toEqual(before)
147 })
106148})