Image and link APIs: what to fix before release

[romain-pm]

Both APIs are now implemented (#746 #766 #751 #773) and adopted in Jahia/luxe-jahia-demo (#458, #459) and on Jahia/jahia.com. A review of both together returned consistent: mostly, learning curve: acceptable — up from "no / too steep" for the image API alone (#767). Five things should be fixed before release. Four are cheap; the fifth is cheap now and breaking the day after.

1. Two published guide snippets do not compile under the scaffold's own strict tsconfig, including one in the link guide's cache-dependency section that a reader copies verbatim. Fix the snippets, then type-check every fenced tsx block in both guides as part of the build.

2. Neither API is in the library README, and the link guide's Reference section links five symbols into it — all five anchors are dead. The image guide's Reference is a plain unlinked list, so the two guides also disagree on what a Reference section is. Add the symbols, or drop the dead links rather than shipping them.

3. context is required on getImageProps and optional on getLinkProps / resolveContentLink. Same concept — the render cache dependency, and the bundleKey that selects module defaults — opposite enforcement, and the two guides state the rule in contradictory terms. Making it required on the link side is a breaking change after release.

4. Only one of the two props tiers can cross into an Island, which is the question both are documented to answer. ImgProps is flat and serializable by design. LinkState carries a JCRNodeWrapper (state.node, added by #769), so state cannot be shipped into an Island at all — and that field has zero consumers in either adoption. Move node off LinkState (a third sibling of anchor/state, or drop it in favour of readNodeReference, which is what jahia.com actually used). While there: state should carry the aria-current value, not just the isCurrent boolean behind it — 7 Island call sites across the two repos hand-write that mapping today.

5. samples/hydrogen teaches the anti-pattern the link guide calls a bug. Five of its six link sites are still <a href={buildNodeUrl(...)}>, including the NavBar and the LanguageSwitcher — the two cases the guide is written around. Two Hero backgrounds should use buildBackgroundImageUrl. This is the cheapest way to give both guides a working showcase.

Decide before merging: what happens to layout="fluid"

fluid was added by #767 as the answer to "0 of 16 luxe sites could use what the guide teaches". It now has zero call sites across all three repos, and both adopters independently reinvented it — wrongly. luxe used constrained plus a hand-written sizes at five sites, its own commit message admitting no single number describes the slot; jahia.com made full-width + sizes="auto" its wrapper default, with a docstring explaining it picked that because fill takes normal-flow images out of flow — it never learned fluid exists.

The name was not the problem. The vocabulary is still missing the commonest real slot: a maximum width, with sizes written by hand. Either let fluid take slotWidth as an optional maximum (ladder capped at 2x, sizes still required from the caller), or delete fluid and document full-width + sizes as the answer. Shipping a layout with no users and two reinventions is how the next review reaches the same verdict.

Smaller divergences, non-breaking to fix later

cacheDependency is a boolean on the image side and a four-form union on the link side. absolute exists on every image function and nowhere on the link side, though a canonical link, an og:url and a JSON-LD url all need it and jahia.com reaches past the API for it. fallback (image) and whenUnresolved (link) answer the same "the content is not there" question with different names, different defaults and non-overlapping capability, and neither guide mentions the other's.

And the highest-value warning either API could carry, which neither does: a view emitting aria-current without cache.mainResource=true. getLinkProps can see the render context and can tell.

[romain-pm]

Answered: restructure the layout set, do not extend it. Read astro@7.1.3 and next@16.3.2 from source, and verified Astro's behaviour by building 16 call sites and reading the emitted HTML.

Astro's constrained is not the case we are missing, and we did not mis-implement it. Astro derives (min-width: Wpx) Wpx, 100vw (layout.js:89-90); we derive the identical string (getImageProps.ts:184). In both, the declared width is an upper bound in the ladder but not in the sizes — that string is not "at most W", it is the precise claim "the slot is exactly W whenever the viewport is at least W". For a 1→3-column grid cell that is roughly 3x too large at desktop. We copied it faithfully, hole included.

The bug was never the vocabulary. It is that an explicit sizes does not reach the ladder.

• Astro has the same bug. widths ||= getWidths(...) and sizes ||= getSizesAttribute(...) are independent statements ten lines apart (internal.js:117, :123) and getWidths is never passed the sizes (layout.js:51-56). Verified: <Image width={800} layout="constrained" sizes="(min-width: 1024px) 33vw, 100vw"> emits a srcset byte-identical to the same call without sizes, no warning. Astro's only exit is hand-writing widths too, which nothing documents.
• Next does not. if (sizes) is the outermost branch of getWidths (get-img-props.js:51), evaluated before width is read: it flips the descriptor kind to w, drops width from the computation entirely, and grows the candidate count from 2 to 8-15. One description of the slot, and the ladder comes from it.

Measured on Jahia/luxe-jahia-demo#458, DPR 2 at the widest viewport each hand-written sizes describes:

call site	slotWidth	ladder	widest slot its own sizes claims	served
Estate/default.server.tsx:22	400	[320,400,800]	768px	0.52x
Agency/fullPage.server.tsx:131 (LCP, preload)	500	[320,500,1000]	893px	0.56x
Realtor/fullPage.server.tsx:135 (LCP, preload)	500	[320,500,1000]	893px	0.56x

The change

1. Delete fluid. Zero call sites in all three repos. Free today, a major-version rename after release.
2. constrained takes slotWidth xor sizes, as a type error, with a runtime throw for the untyped caller and the loose spread. Both is two descriptions of one slot that nothing can reconcile — Astro allows it and silently disagrees, Next makes it impossible. slotWidth alone keeps today's derived string. sizes alone is the old fluid, and is now also the honest spelling for a grid cell, an aspect-ratio box and a height-constrained slot. Neither still throws, with a message naming both exits.
3. Derive the ladder from sizes when it is given. Next's direction, not its implementation: its vw scrape misses calc(100vw - 2rem) and 33.3vw, and it never derives a ceiling. We have a real breakpoint list and a real intrinsic-width clamp, so we can do better — and the parser must fail toward the whole ladder, never toward a narrow one.
4. fixed rejects sizes — the layout's whole meaning is that the slot is one number. No current call site does it.
5. Fix the off-by-a-band filter. candidateWidths filters breakpoints.filter(c => c < width) (getImageProps.ts:166), so constrained skips every breakpoint between W and 2W; Astro keeps them (layout.js:69-76). slotWidth={960} gives us [320,640,960,1920] and Astro [320,640,960,1280,1920] — a 1.33x screen at the full slot gets the 1920 file instead of the 1280.

Height-constrained slots: nobody solves this. Astro turns height into a width through the aspect ratio and then emits a sizes about viewport width. Next's getWidths has no height parameter and fill hard-errors on any caller style.height other than 100%. Under this change the slot at least gets an honest spelling (sizes alone), which is more than either framework offers. A real answer is follow-up work, not a release blocker.

Cost. Five luxe call sites become type errors — the same five that are under-serving today. jahia.com's <Image> wrapper default needs rethinking. The guide needs more than a find-and-replace: fluid leads its layout section and both the aspect-ratio and height-constrained rows route to it.

[romain-pm]

Landed on feat/image-improvements (#766), commit 73976ab. All five points, plus the guide.

1. fluid deleted. ImageLayout is "constrained" | "fixed" | "full-width" | "fill".

2. slotWidth xor sizes, as a type. A new exported ImageSlot union carries the constraint, and both ImageOptions and JImageProps are now <base interface> & ImageSlot:

export type ImageSlot =
  | { layout?: "constrained"; slotWidth: number; sizes?: never }
  | { layout?: "constrained"; slotWidth?: never; sizes: string }
  | { layout: "fixed"; slotWidth: number; sizes?: never }
  | { layout: "full-width"; slotWidth?: never; sizes?: string }
  | { layout: "fill"; slotWidth?: never; sizes: string };

Every branch declares all three keys, so keyof JImageProps is stable and the component's Omit<ImgHTMLAttributes, "src" | "srcSet" | keyof JImageProps> still omits what it always did. One trap worth recording: the union has to sit outside the Readonly<> in the component's parameter type. Readonly<A & (B | C)> is a mapped type over an intersection, and the property lookup collapses sizes back to string — the xor silently disappears. It is now Readonly<JImageBaseProps & Omit<…>> & MarkupBox & ImageSlot.

Runtime throws live in one resolveSlot, which validates and derives the sizes in the same switch, and runs before the !node check — a view describing its slot wrongly fails the same way whether or not the content property happens to be filled. The neither-given message names both exits: "needs the slot described, and nothing in the markup describes it. Either give a slotWidth, the slot width in CSS pixels, or give a sizes — sizes="auto" lets the browser measure the real box, and sizes="(min-width: 60rem) 33vw, 100vw" describes it yourself."

3. The ladder follows the sizes. New sizesLadder.ts. Paren-aware split on commas gives the entries; paren-aware split on whitespace gives the tokens, and the last token is the source size — which is what keeps the 1024px inside (min-width: 1024px) 33vw out of the arithmetic without parsing media conditions at all. Each source size is read at the ladder's two extremes, which stand in for the viewport range:

• Nvw → { min: N/100 × narrowest, max: N/100 × widest }
• Npx → { min: N, max: N }
• calc() / min() / max() / clamp() → not evaluated; every vw/px length inside is scraped, min is the smallest single contribution at the narrowest viewport, max is the sum of them all at the widest. Both err outward on purpose.
• anything else (auto, 50%, 20em, a bare number) → give up.

Floor is the smallest min across entries, ceiling the largest max. The ladder keeps every breakpoint >= floor, up to and including the first one that reaches 2 × ceiling — inclusive, so the widest slot is still covered at DPR 2. An unreadable source size, unbalanced parentheses, an empty string, or an empty result all return the whole ladder. Failing wide is the invariant, and there is a test for each path.

Worked examples on the default ladder: 100vw → whole; (min-width: 1024px) 33vw, 100vw → whole; 33vw and 33.3vw and calc(33vw - 2rem) → [320, 640, 960, 1280, 1920]; 25vw → [320, 640, 960, 1280]; 400px → [640, 960]; min(100vw, 400px) and clamp(200px, 50vw, 600px) → whole; 33vw on [480, 960, 1440] → [480, 960].

4. fixed rejects sizes, in the type and at runtime. full-width and fill likewise reject a slotWidth, for the same "one description" reason.

5. Off-by-a-band fixed. breakpoints.filter(c => c <= 2 * width), matching Astro. slotWidth={960} now gives [320, 640, 960, 1280, 1920].

Guide. The layout section is rewritten around the two questions that decide the spelling, with one row per real slot shape. The aspect-ratio box and the height-constrained slot each get a subsection; the second says plainly that no framework has a precise answer, why (sizes is about widths, resolved before the browser knows the file), and what to write — sizes="auto" normally, and a sizes in px computed from the CSS height and the asset's own ratio when the image must load eagerly. A new subsection documents how a sizes string produces the ladder.

Verified. tsc --noEmit on the library clean; tsc -p tsconfig.spec.json clean, which is what makes the @ts-expect-error regression tests real — an assertion that stopped being a type error would fail there; vitest run 125 passing; eslint and prettier --check clean; yarn build green with publint "All good!"; tsc --noEmit on samples/hydrogen clean.

Costs, for whoever picks up the adoptions. Five luxe call sites become type errors and drop their slotWidth. jahia.com's <Image> wrapper passes layout, slotWidth and sizes as three independent optionals and needs a union of its own. And <JImage node={icon} alt="" width={48} height={48} /> no longer type-checks — the HTML box is not a slot description, since CSS can still resize the element — so it becomes layout="fixed" slotWidth={48} width={48} height={48}. Making the markup box count as a slot description is possible; it was deliberately left out of this change.