also looking at this
fix(images): name delivery variants independently of the output path
#1961
2 files
+102
-2
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -1,4 +1,4 @@ | ||
| 1 | import type { ImageDeliveryManifest } from 'ts-images/delivery' | |
| 1 | import type { ImageDeliveryManifest, ImageDeliveryStorage } from 'ts-images/delivery' | |
| 2 | 2 | import { createHash } from 'node:crypto' |
| 3 | 3 | import fs from 'node:fs' |
| 4 | 4 | import path from 'node:path' |
| @@ -25,6 +25,63 @@ function catalogName(relativePath: string): string { | ||
| 25 | 25 | return `${readable || 'image'}-${pathHash}` |
| 26 | 26 | } |
| 27 | 27 | |
| 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 | */ | |
| 52 | function 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 | ||
| 28 | 85 | async function collectRasterImages(root: string, directory = root): Promise<Array<{ absolutePath: string, relativePath: string }>> { |
| 29 | 86 | const entries = await fs.promises.readdir(directory, { withFileTypes: true }) |
| 30 | 87 | entries.sort((a, b) => a.name.localeCompare(b.name)) |
| @@ -122,6 +179,7 @@ export async function prepareImageDelivery( | ||
| 122 | 179 | |
| 123 | 180 | const catalogOptions = { |
| 124 | 181 | outDir: path.join(outputDir, '_stx', 'images'), |
| 182 | storage: deliveryStorage(path.join(outputDir, '_stx', 'images')), | |
| 125 | 183 | baseUrl: DELIVERY_URL, |
| 126 | 184 | widths: DEFAULT_WIDTHS, |
| 127 | 185 | formats: ['avif', 'webp'] as const, |
| @@ -1,5 +1,5 @@ | ||
| 1 | 1 | import { afterAll, beforeAll, describe, expect, it } from 'bun:test' |
| 2 | import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' | |
| 2 | import { mkdir, mkdtemp, readdir, rm, stat, writeFile } from 'node:fs/promises' | |
| 3 | 3 | import { tmpdir } from 'node:os' |
| 4 | 4 | import { join } from 'node:path' |
| 5 | 5 | import { encode } from 'ts-images' |
| @@ -103,4 +103,46 @@ describe('build-time Image delivery', () => { | ||
| 103 | 103 | expect(html).toContain(':src="avatarUrl"') |
| 104 | 104 | expect(html).not.toContain('<picture') |
| 105 | 105 | }) |
| 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 | }) | |
| 106 | 148 | }) |