Skip to content

fix(images): make the fallbackFormat rule per-source-format - #61

Open
dmurko wants to merge 2 commits into
mainfrom
fix/fallbackformat-rule-corrected
Open

fix(images): make the fallbackFormat rule per-source-format#61
dmurko wants to merge 2 commits into
mainfrom
fix/fallbackformat-rule-corrected

Conversation

@dmurko

@dmurko dmurko commented Aug 7, 2026

Copy link
Copy Markdown
Member

Supersedes the closed #60. Same core finding, with the .png half corrected and enforcement added.

The scaffold says "Do NOT set fallbackFormat="webp"" unconditionally — a rule #46 introduced from a .png repro. It is wrong for .webp/.avif sources, where following it ships a multi-megabyte PNG fallback.

Ground truth

Identical in astro@6.4.8 (the scaffold's pin) and astro@7.1.3node_modules/astro/components/Picture.astro:

let resultFallbackFormat = fallbackFormat ?? defaultFallbackFormat; // 'png'
if (!fallbackFormat && isESMImportedImage(clonedSrc) &&
    specialFormatsFallback.includes(clonedSrc.format)) {            // ['gif','svg','jpg','jpeg']
  resultFallbackFormat = clonedSrc.format;
}

Neither webp nor avif is on that list, so both fall through to defaultFallbackFormat = 'png'.

The corrected rule

Source fallbackFormat Why
.webp / .avif set "webp" not in specialFormatsFallback → fallback becomes PNG. The only genuinely wrong combination.
.jpg / .jpeg / .gif / .svg omit already fall back to themselves
.png optional — setting it is usually better see below
not known statically compute it fallbackFormat={img.format === "webp" ? "webp" : undefined}

Where this differs from #60

#60 said .png must omit it because setting it fails the build with ENOENT … dist/_astro/<name>.png. That no longer reproduces.

Tested on both 6.4.8 and 7.1.3 — webp-only <Picture> on a .png source, with and without widths, and with a second raw <img> on the same asset. All built clean. Astro does delete an unreferenced original (dist/assets/build/generate.js), but only when globalThis.astroAsset.referencedImages lacks the path, and the unlink is wrapped in try {} catch {}.

So .png is documented as optional, with the measured tradeoff. Same 2400px PNG at widths={[300, 600]}:

files emitted total
omitted 4 (webp ladder + PNG ladder) 109 kB
fallbackFormat="webp" 2 (fallback reuses the webp) 27 kB

The troubleshooting row is requalified to "you are on an Astro older than 6.4 — upgrade", rather than "change the prop".

Enforcement, not just documentation

Added to skills/code-review, keyed to the resolved source extension and explicitly told not to flag .png + fallbackFormat as build-breaking. A rule that lives only in CLAUDE.md is how the image config drifted in the first place.

Two further code-review gaps

Both found while applying this to site-hakuto (site-hakuto#31):

  • The markdown-cap check looked only for markdown.rehypePlugins. Astro 7 takes markdown.processor: unified({ rehypePlugins }) and rejects the legacy key with unified is not a function — so the check false-positived on every Astro 7 project.
  • Content images referenced by an absolute /images/… public path never enter the pipeline at all, and no rehype cap can fix them: they're absent from file.data.astro.localImagePaths, which such plugins key off. Now Critical.

Also carried over from #60, re-verified

All three resize() branches in astro/dist/assets/services/sharp.js pass withoutEnlargement: true, so width never upscales either — the right-sizing section implied only widths were clamped.

Scope

Docs-only — no scaffold file or plugin version change, following 4078700's precedent.

Context: teamniteo/ops#2831.

🤖 Generated with Claude Code

dmurko and others added 2 commits August 7, 2026 18:29
Supersedes the closed #60. Same core finding, with the `.png` half corrected
and enforcement added.

The scaffold said "Do NOT set `fallbackFormat=\"webp\"`" unconditionally, a rule
introduced by #46 from a `.png` repro. It is wrong for `.webp`/`.avif` sources,
where following it ships a multi-megabyte PNG fallback.

Ground truth, identical in astro@6.4.8 (the scaffold's pin) and astro@7.1.3 —
`node_modules/astro/components/Picture.astro`:

  let resultFallbackFormat = fallbackFormat ?? defaultFallbackFormat; // 'png'
  if (!fallbackFormat && isESMImportedImage(clonedSrc) &&
      specialFormatsFallback.includes(clonedSrc.format)) {  // gif,svg,jpg,jpeg
    resultFallbackFormat = clonedSrc.format;
  }

Neither `webp` nor `avif` is on that list, so both default to a PNG fallback.
That is the one genuinely wrong combination, and it is now a Critical check in
`skills/code-review` keyed to the resolved source extension rather than flagged
blanket — a rule that lives only in CLAUDE.md is how this drifted in the first
place.

Where this differs from #60: the `ENOENT … dist/_astro/<name>.png` failure it
cited for `.png` sources **no longer reproduces**. Tested on both 6.4.8 and
7.1.3 — webp-only `<Picture>` on a `.png`, with and without `widths`, and with
a second raw `<img>` on the same asset. All build clean. Astro deletes an
unreferenced original in `dist/assets/build/generate.js`, but only when
`globalThis.astroAsset.referencedImages` lacks the path, and the `unlink` is
wrapped in `try {} catch {}`. So `.png` is now documented as optional, with the
measured tradeoff: on a 2400px PNG at widths=[300,600], omitting emits 4 files
/ 109kB (a full second PNG ladder), setting it emits 2 files / 27kB. The
troubleshooting row is requalified as "upgrade Astro", not "change the prop".

Also carried over from #60, re-verified here: all three `resize()` branches in
`astro/dist/assets/services/sharp.js` pass `withoutEnlargement: true`, so
`width` never upscales either — the section previously implied only `widths`
were clamped.

Two further code-review gaps found while applying this to site-hakuto:

- The markdown-cap check looked only for `markdown.rehypePlugins`. Astro 7
  takes `markdown.processor: unified({ rehypePlugins })` and rejects the legacy
  key outright, so the check false-positived on every Astro 7 project.
- Content images referenced by an absolute `/images/…` public path never enter
  the pipeline at all, and no rehype cap can fix them — they are absent from
  `localImagePaths`, which such plugins key off. Now a Critical check.

Docs-only; no scaffold file or plugin version change, following 4078700's
precedent for the analogous stale-guidance fix.
@dmurko
dmurko requested a review from dz0ny August 7, 2026 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants