fix(image): emit x-descriptor srcSet for fixed-size images without sizes (#1966) by Divkix · Pull Request #2062 · cloudflare/vinext

Divkix · 2026-06-16T03:24:41Z

Problem

Closes #1966.

For a fixed-size <Image width={N} /> with no sizes prop, the shim emitted a w-descriptor srcSet (… 640w, 750w, …) and left the sizes attribute undefined. Per the HTML spec, a w-descriptor srcSet with no sizes defaults to sizes=100vw, so the browser selects an image sized to the full viewport width rather than the element width — over-fetching on high-DPR / large-viewport displays (e.g. a 1000px image fetched at 1920w) and hurting LCP. Next.js handles this with an x-descriptor (1x/2x) srcSet.

Fix

Mirror Next.js getWidths (get-img-props.ts):

No sizes + numeric width → x-descriptor srcSet. Build [width, width*2] quantized up to the nearest allowed size from allSizes (deviceSizes ∪ imageSizes), deduped, emitted as 1x / 2x. e.g. width=100 → w=128 1x, w=256 2x.
w-descriptors are kept for the sizes-provided and fill paths. The sizes attribute logic was already correct (fill → 100vw), so it is unchanged.
Read images.imageSizes (already injected as __VINEXT_IMAGE_SIZES, but the shim never read it) to build the allSizes union used for quantization.
Fallback src now uses the largest srcSet candidate (Next.js sets src = loader(widths[last])). This also fixes a pre-existing latent 400: the old src used the raw declared width, which the optimizer rejects when it isn't in the allowed-widths list (e.g. w=100).

Implemented by replacing generateSrcSet with getImageWidths + buildResponsiveSrc, wired into both the component render and getImageProps.

Edge cases

Width clamping past the largest size, dedup to a single 1x entry, tiny widths (16 → 16 1x, 32 2x), quality propagation, and custom-configured device/image sizes.

Test plan

Added a regression test porting Next.js' unit test: width:100, height:200, no sizes → srcSet w=128 1x, w=256 2x, src=w=256, no sizes attribute.
Updated existing assertions to the Next.js-parity output across tests/image-component.test.ts, tests/shims.test.ts, and tests/image-optimization-parity.test.ts.
✅ image-component.test.ts 60/60 · shims.test.ts 1130/1130 · full unit suite green · integration image-optimization-parity 6/6 (confirms the server serves the new w=128 src with a 200).

pkg-pr-new · 2026-06-16T03:25:56Z

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2062

npm i https://pkg.pr.new/create-vinext-app@2062

npm i https://pkg.pr.new/vinext@2062

commit: 867fd17

james-elicx · 2026-06-16T14:04:23Z

/bigbonk review for issues

ask-bonk

Reviewed against the authoritative Next.js source (packages/next/src/shared/lib/get-img-props.ts, getWidths + generateImgAttrs).

The core fix is correct. The no-sizes x-descriptor path faithfully mirrors Next.js:

[width, width*2].map(w => ALL_SIZES.find(p => p >= w) ?? largest) matches Next's allSizes.find(p => p >= w) || allSizes[last].
ALL_SIZES = deviceSizes ∪ imageSizes sorted ascending matches Next's allSizes.
src = loader(widths[last]) matches Next (largest candidate), and reading __VINEXT_IMAGE_SIZES is wired correctly (index.ts:1385, deploy.ts:491, defaults match DEFAULT_IMAGE_SIZES).

The "latent 400" claim is verified. parseImageParams rejects any w not in allowedWidths (server/image-optimization.ts:142), so the old fallback src with the raw declared width (e.g. w=100) would 400. The new quantized width always lands in the allowlist. Good catch.

Two non-blocking notes below. Overall LGTM.

ask-bonk · 2026-06-16T14:07:35Z

+): { widths: number[]; kind: "w" | "x" } {
+  if (sizes) {
+    const widths = RESPONSIVE_WIDTHS.filter((w) => w <= originalWidth * 2);
+    return { widths: widths.length ? widths : [originalWidth], kind: "w" };


Non-blocking parity note: this sizes-present branch does not mirror Next.js's getWidths. Next parses the vw percentages out of the sizes string and returns allSizes.filter(s => s >= deviceSizes[0] * smallestRatio) (or the full allSizes when no vw is present), and it does not cap at 2x. Here it returns deviceSizes-only filtered by w <= originalWidth * 2.

This divergence is pre-existing (the old generateSrcSet did the same), so it's out of scope for this PR. But the PR description states it "mirror[s] Next.js getWidths" and that the sizes path is "kept" / "already correct" — that's only true for the no-sizes path you actually changed. Consider softening that wording, or filing a follow-up for the sizes-path divergence so it isn't lost.

ask-bonk · 2026-06-16T14:07:35Z

-            .join(", ") || `${src} ${imgWidth}w`
-        : undefined;
+      ? buildResponsiveSrc(src, imgWidth, imgQuality, sizes)
+      : null;


Heads up: routing the sizes-present case through buildResponsiveSrc also changes the fallback src for that path. Previously src = imageOptimizationUrl(src, imgWidth, ...) (the declared width); now it's widths[last] from the sizes branch (largest device width <= imgWidth*2). e.g. width=500 + sizes was w=500, now w=828.

This is arguably closer to Next.js (which always uses widths[last]), so it's likely an improvement — but it's an undocumented behavior change beyond the no-sizes fix described in the PR, and the existing sizes-prop test (image-component.test.ts:165) only asserts the sizes attribute, not the srcSet/src. Worth a one-line mention in the PR body and ideally a test asserting the srcSet+src for the sizes-provided case so this path is no longer untested.

ask-bonk · 2026-06-16T14:07:45Z

Review posted successfully to PR #2062.

Summary

I reviewed PR #2062 against the authoritative Next.js source (get-img-props.ts → getWidths + generateImgAttrs) and verified the implementation, the "latent 400" claim, env wiring, and ran the test suite.

Conclusion: LGTM (with 2 non-blocking notes).

What's correct:

The no-sizes x-descriptor path faithfully mirrors Next.js: [width, width*2].map(w => ALL_SIZES.find(p => p >= w) ?? largest) matches Next's logic exactly, ALL_SIZES is the right deviceSizes ∪ imageSizes union, and src = widths[last] matches.
The "latent 400" fix is verified — parseImageParams (image-optimization.ts:142) rejects widths not in allowedWidths, so the old w=100 fallback src would 400. The new quantized width always lands in the allowlist.
__VINEXT_IMAGE_SIZES is correctly injected (index.ts:1385, deploy.ts:491) and defaults match DEFAULT_IMAGE_SIZES.
image-component.test.ts passes 60/60.

Two non-blocking notes I flagged inline:

The sizes-present branch in getImageWidths does not actually mirror Next.js (deviceSizes-only capped at 2x, vs Next's allSizes filtered by vw ratio). Pre-existing, but the PR's "mirrors Next.js getWidths" wording overstates it for that path.
Routing the sizes-present case through buildResponsiveSrc silently changes the fallback src for that path (declared width → widths[last]), which is untested (the existing sizes test only asserts the sizes attribute). Suggested documenting it and adding a test.

github run

james-elicx · 2026-06-16T14:20:55Z

/bigbonk review for issues

ask-bonk

Independently verified the core fix against the authoritative Next.js source (packages/next/src/shared/lib/get-img-props.ts → getWidths + generateImgAttrs). The no-sizes x-descriptor path is correct and the fix is sound.

What I confirmed:

[width, width*2].map(w => ALL_SIZES.find(p => p >= w) ?? largest) is a faithful port of Next's allSizes.find(p => p >= w) || allSizes[last], including the new Set dedup and kind: 'x'. ALL_SIZES = deviceSizes ∪ imageSizes sorted ascending matches Next's allSizes.
src = loader(widths[last]) matches Next (largest candidate). The sizes attribute stays undefined for the fixed/no-sizes case and only becomes 100vw for fill — matches Next's !sizes && kind === 'w' ? '100vw' : sizes (our fixed path is kind === 'x', so no 100vw).
The "latent 400" claim is verified. parseImageParams returns null (→ 400) for any w not in allowedWidths (server/image-optimization.ts:142), and allowedWidths is the same deviceSizes ∪ imageSizes union. The old fallback src used the raw declared width (e.g. w=100/w=500), which is not in the allowlist, so it would 400. Good catch.
__VINEXT_IMAGE_SIZES wiring is correct (index.ts:1385, deploy.ts:491, global.d.ts:458) and defaults match DEFAULT_IMAGE_SIZES. The shim previously never read it.
image-component.test.ts 60/60 and shims.test.ts image cases pass locally.

Adding one point that strengthens the PR beyond what the description claims: routing the sizes-present case through buildResponsiveSrc also fixes a latent 400 there. Previously that path set src = imageOptimizationUrl(src, imgWidth) with the raw declared width (e.g. width=500 → w=500), and 500 is not in allowedWidths either — so it would 400 today. The new widths[last] (828 for width=500) always lands on an allowed width. Worth calling out in the PR body since it's a second real fix, not just a cosmetic change.

Non-blocking notes below. LGTM.

ask-bonk · 2026-06-16T14:24:01Z

+): { widths: number[]; kind: "w" | "x" } {
+  if (sizes) {
+    const widths = RESPONSIVE_WIDTHS.filter((w) => w <= originalWidth * 2);
+    return { widths: widths.length ? widths : [originalWidth], kind: "w" };


Confirming the prior note: this sizes-present branch does not mirror Next's getWidths. Next parses the vw percentages from the sizes string and returns allSizes.filter(s => s >= deviceSizes[0] * smallestRatio) (or the full allSizes when no vw is present), with no 2x cap. Here it returns deviceSizes-only filtered by w <= originalWidth*2.

This is pre-existing (old generateSrcSet did the same), so out of scope — but the PR description's "mirror[s] Next.js getWidths" / sizes path "already correct" wording only holds for the no-sizes path you actually changed. Suggest softening the wording or filing a follow-up so the divergence isn't lost.

Also note the empty-filter fallback [originalWidth]: for a tiny sizes-present image (e.g. width=10), widths[last] would be 10, which is not in allowedWidths and would 400. Edge case, but it's the same class of bug this PR is fixing for the no-sizes path.

ask-bonk · 2026-06-16T14:24:01Z

-            .join(", ") || `${src} ${imgWidth}w`
-        : undefined;
+      ? buildResponsiveSrc(src, imgWidth, imgQuality, sizes)
+      : null;


Routing the sizes-present case through buildResponsiveSrc silently changes the fallback src for that path (declared width → widths[last]), e.g. width=500 + sizes goes from w=500 to w=828. As noted in the summary this is actually a fix (the old w=500 would 400), but it's untested — the existing sizes-prop test (tests/image-component.test.ts:165) only asserts the sizes attribute, not src/srcSet.

Worth adding an assertion so this path is covered:

Suggested change

: null;

it("renders with custom sizes prop", () => {

const html = ReactDOMServer.renderToString(

React.createElement(Image, {

alt: "sized",

src: "/img.png",

width: 500,

height: 300,

sizes: "(max-width: 768px) 100vw, 50vw",

}),

);

expect(html).toContain('sizes="(max-width: 768px) 100vw, 50vw"');

// `sizes` present -> w-descriptor srcSet; fallback src is the largest

// candidate (<= width*2 = 1000 -> 828), always an allowed width.

expect(html).toContain(`src="${optUrlHtml("/img.png", 828)}"`);

});

(Adjust the line number target if image-component.test.ts:165 shifted.)

ask-bonk · 2026-06-16T14:24:12Z

Review posted successfully to PR #2062.

Summary

I independently verified PR #2062 against the authoritative Next.js source (get-img-props.ts → getWidths + generateImgAttrs), checked the optimizer's width allowlist, confirmed env wiring, and ran the test suite.

Conclusion: LGTM (with 2 non-blocking notes).

Verified correct:

The no-sizes x-descriptor path is a faithful port: [width, width*2].map(w => ALL_SIZES.find(p => p >= w) ?? largest) matches Next's logic exactly, including dedup and kind: 'x'. ALL_SIZES = deviceSizes ∪ imageSizes is correct, and src = widths[last] matches Next.
The sizes attribute stays undefined for the fixed/no-sizes case (only 100vw for fill), matching Next.
The "latent 400" claim is verified — parseImageParams (image-optimization.ts:142) rejects widths not in allowedWidths, so the old w=100 fallback would 400.
__VINEXT_IMAGE_SIZES is correctly injected and defaults match DEFAULT_IMAGE_SIZES.
Tests pass locally (image-component.test.ts 60/60, shims image cases green).

New finding that strengthens the PR (beyond the existing review notes):

Routing the sizes-present case through buildResponsiveSrc also fixes a second latent 400. The old sizes path set src = w=<declared width> (e.g. w=500), and 500 is not in allowedWidths either — so it would 400 today. The new widths[last] (828) always lands on an allowed width. This is a real fix worth documenting, not a cosmetic change.

Two non-blocking inline notes:

The sizes-present branch still doesn't mirror Next's getWidths (pre-existing divergence), and the empty-filter fallback [originalWidth] can produce a 400-causing src for tiny images.
The sizes-present src/srcSet change is untested — suggested a suggestion adding src/srcSet assertions to the existing test.

github run

github-actions · 2026-06-18T22:52:25Z

Performance benchmarks

Compared 867fd17 against base 8d0a18d using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	126.6 KB	126.6 KB	⚫ -0.0%
Client entry size (gzip)	vinext	120.6 KB	120.6 KB	⚫ +0.0%
Dev server cold start	vinext	2.65 s	2.68 s	⚫ +1.4%
Production build time	vinext	2.81 s	2.80 s	⚫ -0.3%
RSC entry closure size (gzip)	vinext	98.6 KB	98.6 KB	⚫ -0.0%
Server bundle size (gzip)	vinext	165.3 KB	165.3 KB	⚫ +0.0%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

Divkix · 2026-06-24T14:45:54Z

Both nits from the review are addressed:

The sizes-present SSR test now asserts srcSet and src (largest candidate, always an allowed width).
Added a sizes-present getImageProps test covering srcSet, src, and sizes passthrough.

…zes (cloudflare#1966) For a fixed-size <Image> with a numeric `width` and no `sizes`, the shim emitted a w-descriptor srcSet and left `sizes` undefined. Per the HTML spec, a w-descriptor srcSet with no `sizes` defaults to `sizes=100vw`, so the browser fetches a viewport-sized image instead of an element-sized one — over-fetching on high-DPR/large viewports and hurting LCP. Mirror Next.js getWidths: when there is no `sizes` prop and a numeric width, emit a 1x/2x x-descriptor srcSet built from [width, width*2] quantized up to the nearest allowed size (deviceSizes ∪ imageSizes). w-descriptors are kept for the `sizes`-provided and `fill` paths. Also set the fallback `src` to the largest srcSet candidate (Next.js sets `src = loader(widths[last])`). This fixes a latent 400: the old `src` used the raw declared width, which is rejected by the optimizer when it is not in the allowed-widths list. - read images.imageSizes (already injected as __VINEXT_IMAGE_SIZES) to build the allSizes union used for x-descriptor quantization - replace generateSrcSet with getImageWidths + buildResponsiveSrc, wired into both the component render and getImageProps - update tests to the Next.js-parity output and add a regression test (width 100 → "w=128 1x, w=256 2x", src w=256, no sizes attribute)

ask-bonk Bot reviewed Jun 16, 2026

View reviewed changes

Divkix force-pushed the fix/issue-1966-image-srcset branch from 8508baf to 867fd17 Compare July 13, 2026 03:51

-      : null;
+  it("renders with custom sizes prop", () => {
+    const html = ReactDOMServer.renderToString(
+      React.createElement(Image, {
+        alt: "sized",
+        src: "/img.png",
+        width: 500,
+        height: 300,
+        sizes: "(max-width: 768px) 100vw, 50vw",
+      }),
+    );
+    expect(html).toContain('sizes="(max-width: 768px) 100vw, 50vw"');
+    // `sizes` present -> w-descriptor srcSet; fallback src is the largest
+    // candidate (<= width*2 = 1000 -> 828), always an allowed width.
+    expect(html).toContain(`src="${optUrlHtml("/img.png", 828)}"`);
+  });

Uh oh!

Conversation

Divkix commented Jun 16, 2026

Problem

Fix

Edge cases

Test plan

Uh oh!

pkg-pr-new Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

james-elicx commented Jun 16, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Jun 16, 2026

Summary

Uh oh!

james-elicx commented Jun 16, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Jun 16, 2026

Summary

Uh oh!

github-actions Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance benchmarks

Uh oh!

Divkix commented Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pkg-pr-new Bot commented Jun 16, 2026 •

edited

Loading

github-actions Bot commented Jun 18, 2026 •

edited

Loading