feat(image): support images.loaderFile and emit srcSet from custom loaders - #2700
feat(image): support images.loaderFile and emit srcSet from custom loaders#2700NathanDrake2406 wants to merge 10 commits into
Conversation
vinext replaces next/image with a from-scratch shim, so Next's build-time machinery for `images.loaderFile` — a bundler alias that swaps the default loader module — had no equivalent here. The option was accepted by config and then silently ignored, leaving remote images unoptimized. Generate a `virtual:vinext-image-loader` module from next.config so a configured loaderFile becomes the default loader for every image, local or remote. `loader: "custom"` without a loaderFile now reports the missing per-image `loader` prop instead of quietly falling back to /_next/image, and a missing or default-export-less loaderFile fails at config time. Custom loaders also never produced a srcSet: the loader ran once with `imgWidth ?? 0`, so `fill` images — which have no intrinsic width — sent `width: 0` into user loader code. Port Next's getWidths/generateImgAttrs so the loader is invoked once per candidate width, with the width-less case mapping to the deviceSizes ladder rather than 0. This also applies to getImageProps, which shared the same defect. Two existing tests asserted the old behavior of `src` being the declared width; upstream makes `src` the largest candidate so non-srcSet browsers get the highest-fidelity image. They are updated with the derivation. Fixes cloudflare#2699
commit: |
Performance benchmarksCompared 0 improved · 0 regressed · 6 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
|
Strong PR — the diagnosis matches what I found independently while chasing the same issue on a real project (Next 16 + One concrete thing worth tightening before merge — the
So for a remote Minor, optional: Happy to see this land — it unblocks CDN/CMS images through |
Production getImageProps calls blocked remote sources before a custom loader could translate them, diverging from the Image component and breaking picture elements built from the returned props. Let an effective loader own remote URL generation in both paths, and preserve overrideSrc as the fallback source without discarding loader-generated candidates.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3707766599
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Four review findings, all where loader selection and image rendering were
ordered wrongly relative to each other.
A named `images.loader` ("imgix", "cloudinary", "akamai") was only rejected
when paired with a `loaderFile`. On its own it fell past the early return and
nothing downstream caught it, so every image was quietly served from
/_next/image instead of the CDN the config named. Validate the loader value
before `loaderFile` is required.
`images.loader: "custom"` with no `loaderFile` generates a loader that reports
the missing per-image `loader` prop. The shim selected it only after the
`unoptimized` early return, so `<Image unoptimized>` and
`getImageProps({ unoptimized: true })` rendered the original URL from a config
that cannot produce URLs at all. Upstream throws above `generateImgAttrs`,
which is what handles `unoptimized`; mark the generated stub so the shim can
tell it from a working loader and honour that order. A valid loader is still
bypassed for unoptimized images.
Selecting a configured loader entered an <img> branch that ignored
`placeholder="blur"`, so an image that showed a placeholder under the built-in
loader silently lost it — and disagreed with `getImageProps`, which still
returned the blur style. Upstream merges `placeholderStyle` on every path.
Custom loaders received a forced `quality: 75` because the default was applied
at the loader seam rather than inside the built-in optimizer. A loader written
as `quality ?? 80`, or one letting a CDN choose, generated different URLs under
vinext than under Next.js. Pass the absence through; `imageOptimizationUrl`
still applies 75 at its own boundary.
Also drops the hand-authored changeset: CI generates changesets from
Conventional Commit subjects, so keeping it risks a duplicate release entry.
The shim's `images.loaderFile` / bare-`custom` branches were covered by a unit test that swapped `virtual:vinext-image-loader` with `vi.doMock`, which is exactly what `image/image-loader-unconfigured.ts` tells tests not to do. Move them into the existing plugin integration test, where a throwaway project with a real next.config is served by a real dev server, so the loader under test is the one the plugin generated. `getImageProps` runs from a client page: `next/image` is a "use client" module, so its exports are client references in the RSC environment and a route handler cannot call it, and the `loader` prop is a function that would otherwise have to cross the server/client boundary. The fixture loader now emits `quality=auto` when quality is absent. With `?? 75` it produced the same URL whether or not the shim forced a default, so the case could not fail.
The bare-`custom` loader carried its state as a property stamped on the exported function, which made the property name an implicit runtime protocol duplicated across the generator, the ambient declaration, the test stand-in, and the shim. Replace it with a `requiresLoaderProp` named export. Every branch of the generator emits it, so the shim's named import links in all three configurations rather than reading an optional property that happens to be absent. Drops the augmented function type from both type surfaces.
|
Addressed the structural feedback on the function-property marker in
export default customImageLoader;
export const requiresLoaderProp = true;All three generator branches emit Removed along with it: the augmented function type in both Test change: the two marker tests (one string-matching the generated source, one asserting the property survived export) collapse into one that imports all three generated variants and asserts Verified: |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff11d0b76c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <img | ||
| ref={mergedRef} | ||
| src={resolvedSrc} | ||
| src={renderedSrc} |
There was a problem hiding this comment.
When a custom-loader image renders, this JSX inserts src before srcSet, despite generateImgAttrs intentionally returning src last. Browsers may begin fetching the largest fallback candidate before discovering the responsive set, so lower-DPR clients can download both the largest image and their selected candidate; getImageProps similarly reconstructs its object with src before srcSet. Preserve the generated attribute order in both paths.
AGENTS.md reference: AGENTS.md:L84-L84
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b4cd9582d. Confirmed: generateImgAttrs returns src last on purpose, but both call sites then wrote it first — the component in JSX, getImageProps in its object literal — so the intent never reached the DOM. src now comes after srcSet/sizes in both, matching upstream, which spreads imgAttributes wholesale and so inherits the object order.
{...rest} still comes last in the component, so caller overrides keep the precedence they had.
| const effectiveLoader = loader ?? configuredImageLoader; | ||
| if (effectiveLoader) { | ||
| const resolvedQuality = typeof quality === "string" ? Number(quality) : quality; | ||
| const attributes = generateImgAttrs({ |
There was a problem hiding this comment.
When images.loaderFile is configured and an image uses a data: or blob: source, this unconditional call sends the source through the loader for every candidate width and emits transformed src/srcSet URLs. Next.js automatically treats these sources as unoptimized, leaving the original URL untouched with no srcSet; without that guard, inline images and browser-created blob URLs can become invalid CDN requests.
AGENTS.md reference: AGENTS.md:L84-L84
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b4cd9582d. Verified against Next.js 16.2.7 get-img-props.js:270:
if (!src || src.startsWith('data:') || src.startsWith('blob:')) {
unoptimized = true;
isLazy = false;
}with generateImgAttrs returning early on unoptimized (:97), so the loader is never invoked. Added isInlineSrc() alongside the other unoptimized triggers in both paths.
Placement follows upstream's ordering rather than being an early return: the missing-loader-prop throw happens earlier still (:184), so the guard sits behind reportsMissingLoaderProp — a bare images.loader: "custom" is still reported for an inline source, it just no longer gets rewritten.
Regression test leaves data: and blob: sources untouched in image-loader-plugin.test.ts. I confirmed it fails without the guard, producing exactly the breakage you described:
OK https://images.example.comdata:image/gif;base64,R0lGODlhAQAB...?width=640&quality=auto
| ...width=384 1x, ...width=640 2x
Not included: upstream also sets isLazy = false in the same block. That is a separate loading difference which already applies to inline sources today, so I left it out rather than widen this PR — flagging it as a known gap.
| // loader is used. A per-image `loader` prop always takes precedence. | ||
| // Config cannot travel through the `process.env.__VINEXT_IMAGE_*` defines used | ||
| // below — those carry JSON, and a loader is a function. | ||
| import configuredImageLoader, { requiresLoaderProp } from "virtual:vinext-image-loader"; |
There was a problem hiding this comment.
When an App Router project sets ssr.external: true, the SSR environment configuration in index.ts deliberately omits noExternal because dependencies are loaded directly by Node. The newly externalized image shim then evaluates this top-level virtual: import without Vite's resolver and fails with ERR_UNSUPPORTED_ESM_URL_SCHEME before any <Image> can render, even when no custom loader is configured. Keep this shim or its loader dependency in Vite's transform graph, or use a Node-resolvable module boundary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not reproducible — I believe this one is incorrect, and I tested it directly rather than reasoning about it.
The premise is that the shim is reached as a bare specifier and so gets externalized. It isn't. next/image is mapped through resolve.alias to an absolute path (index.ts:2409-2417, nextShimMap → path.join(shimsDir, definition.shim)). Vite's SSR externalization only applies to bare imports, so by the time that decision is made the id is already an absolute file path and the module stays in the transform graph — where the plugin's own resolveId/load hooks resolve virtual:vinext-image-loader. The vinext/shims/* specifier is likewise intercepted at index.ts:3615 and returned as an absolute path.
Empirical check — tests/fixtures/app-basic (which uses next/image) with ssr: { external: true } added:
$ npx vinext build # succeeds
$ npx vinext start
$ curl -s -o /dev/null -w '%{http_code}' /nextjs-compat/image-blur-placeholder
200
Rendered output:
<img src="/transparent-image.svg" ... srcSet="/transparent-image.svg 64w" data-nimg="1" style="background-image:url(data:image/png;base64,...)"/>No ERR_UNSUPPORTED_ESM_URL_SCHEME, no server errors. Grepping the build output, there is no unresolved import ... from "virtual:" statement in any server chunk. The only occurrences are inside the SSR-manifest JSON, under a key carrying Rollup's resolved-virtual-module NUL prefix — which is itself the evidence that Vite processed the module internally and emitted it into a client chunk rather than leaving it for Node.
Happy to be shown otherwise if there is a config path I missed — but under ssr.external: true on App Router this renders correctly today. (The fixture change was reverted; it was only for this check.)
| onLoad={handleLoad} | ||
| onError={handleError} | ||
| style={fill ? getFillStyle(style) : style} | ||
| style={fill ? getFillStyle(style, blurStyle) : { ...blurStyle, ...style }} |
There was a problem hiding this comment.
When a custom-loader image uses placeholder="blur" together with a user style containing backgroundImage, backgroundSize, or another placeholder property, this merge applies the user style after blurStyle, so the placeholder is hidden before the image loads. Next.js merges its placeholder style last, and getImageProps repeats the same reversed ordering; apply the blur properties after the user style in both custom-loader paths.
AGENTS.md reference: AGENTS.md:L84-L84
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct about upstream, but deliberately not fixed here — this is pre-existing, not something this PR introduced.
Verified the divergence is real. Next.js 16.2.7 get-img-props.js:575-576 merges placeholder last:
style: {
...imgStyle,
...placeholderStyle
}vinext has it reversed. But { ...blurStyle, ...style } is the existing convention at every site, four of which predate this PR — on 05eee9fe they are at lines 571, 772, 837 and 909, covering the unoptimized, remote, and both getImageProps paths. This PR added a fifth instance following the same pattern.
So the choice is: fix only the two custom-loader paths, which leaves the loader paths silently disagreeing with the other three and makes the inconsistency harder to spot; or fix all five, which changes rendered output on paths this PR does not otherwise touch. Neither belongs in a feature PR under the repo's "smallest coherent change / avoid unrelated refactors" rule, and the second is a behavior change deserving its own regression tests.
Filing as a follow-up instead: flip all five sites to { ...style, ...blurStyle } (and the getFillStyle argument order to match) with a test that a user backgroundImage no longer suppresses the placeholder.
Worth noting the comment on blurBackgroundStyle already documents upstream's ordering, so the divergence is recorded in-tree rather than silent.
Two custom-loader defects found in review. Inline sources carry their own bytes, so no loader can produce a fetchable URL for them. Upstream forces `unoptimized` for `data:`/`blob:` before any loader runs (`get-img-props.ts:270`), which makes `generateImgAttrs` return the original `src` with no `srcSet`. Without that guard a configured `loaderFile` rewrote a data URI into `https://cdn.example.com/data:image/gif;base64,...?width=640` and emitted a srcSet of the same. The guard sits with the other `unoptimized` triggers and behind the missing-loader-prop check, matching upstream's ordering: the bare `images.loader: "custom"` error is raised earlier still (`:184`), so it is still reported for an inline source. `generateImgAttrs` deliberately returns `src` last so the browser sees the responsive set before the fallback candidate, but both custom-loader call sites then wrote `src` first — the component in JSX, `getImageProps` in its object literal — which defeats it, since React applies attributes in prop order. Move `src` after `srcSet`/`sizes` in both.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4cd9582db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // image would quietly be served from `/_next/image` instead of the CDN the | ||
| // config names. | ||
| const loader: string | undefined = images.loader; | ||
| if (loader !== undefined && loader !== "default" && loader !== "custom") { |
There was a problem hiding this comment.
When an existing Next.js app configures images.loader as imgix, cloudinary, or akamai without loaderFile, this branch now throws during config resolution and prevents the app from starting. These are current valid loader values—the repository's vendored Next 16.2.7 declaration in packages/types/next/upstream/dist/shared/lib/image-config.d.ts includes all three in VALID_LOADERS—so the claim that they exist only for next/legacy/image is incorrect. Implement their URL generation or preserve a nonfatal fallback rather than rejecting valid Next.js configuration.
AGENTS.md reference: AGENTS.md:L293-L297
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Disagreeing on this one — Next.js rejects this configuration too, so the current behavior is parity, not a regression.
VALID_LOADERS does list all three, but that list gates both next/image and next/legacy/image. What decides the outcome is the config validation. Next 16.2.7, dist/server/config.js:540-541:
if (images.loader !== 'default' && images.loader !== 'custom' && images.path === imageConfigDefault.path) {
throw new Error(
`Specified images.loader property (${images.loader}) also requires images.path property to be assigned to a URL prefix.\n` +
`See more info here: https://nextjs.org/docs/api-reference/next/legacy/image#loader-configuration`
)
}So images.loader: "imgix" with no loaderFile and no custom images.path throws during config resolution and prevents the app from starting — in Next.js, today. That is precisely the case this branch rejects. Note also where upstream's own error message points: the next/legacy/image loader-configuration docs.
The implementations back that up. In 16.2.7 the named loaders exist only in dist/client/legacy/image.js:
45: function imgixLoader({ config, src, width, quality })
58: function akamaiLoader({ config, src, width })
61: function cloudinaryLoader({ config, src, width, quality })
dist/shared/lib/image-loader.js — what modern next/image actually uses — contains only defaultLoader (:13, marked __next_img_default at :110). There is no imgix/cloudinary/akamai code path in modern next/image to implement or fall back to.
The one real difference: upstream permits a named loader when images.path is set to a custom prefix, because legacy image consumes it. vinext implements neither next/legacy/image nor custom images.path, so there is no configuration under which a named loader could work — which is why this rejects unconditionally and says so in the message.
A non-fatal fallback is specifically what the earlier round of this review asked to remove, and for a good reason: silently serving /_next/image when the config names a CDN is the failure mode that is hardest to notice in production.
Reverting would restore that. Leaving as-is unless there's a concrete case where Next.js starts an app in this configuration — happy to look at one.
| it("resolves a relative loaderFile against the project root", async () => { | ||
| const root = makeProjectRoot(); | ||
| const config = await resolveNextConfig({ images: { loaderFile: "./loader.mjs" } }, root); | ||
| expect(config.images?.loaderFile).toBe(`${root}/loader.mjs`); |
There was a problem hiding this comment.
On Windows, root contains backslashes while resolveImageLoaderFile() returns a path normalized by toSlash, so this expected value becomes a mixed C:\.../loader.mjs string and the test fails despite correct production behavior. Keep the native node:path fixture input, but canonicalize the expected path.join(root, "loader.mjs") with toSlash as required for source-output comparisons.
AGENTS.md reference: AGENTS.md:L332-L332
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 741fb3ec3.
resolveImageLoaderFile returns toSlash(path.resolve(root, loaderFile)) while root comes from mkdtempSync with native separators, so on Windows the expectation was a mixed C:\...\xxx/loader.mjs against an actual C:/.../xxx/loader.mjs. Now toSlash(path.join(root, "loader.mjs")), keeping the native fixture input as you suggested. Follows the existing convention in tests/app-route-graph.test.ts.
| onLoad={handleLoad} | ||
| onError={handleError} | ||
| style={fill ? getFillStyle(style) : style} | ||
| style={fill ? getFillStyle(style, blurStyle) : { ...blurStyle, ...style }} |
There was a problem hiding this comment.
When a custom or configured loader is combined with placeholder="blur" and the caller's style defines backgroundImage, backgroundSize, backgroundRepeat, or backgroundPosition, this merge applies the caller style after blurStyle, immediately overwriting the placeholder and making it invisible. Next.js merges its placeholder style last—the new helper's own comment documents that ordering—so reverse the precedence for both the fill and non-fill custom-loader paths until loading completes.
AGENTS.md reference: AGENTS.md:L293-L297
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 741fb3ec3 — you were right to re-raise it, and the point that decided it was the one about the helper's own comment.
I deferred this last round as pre-existing, which it is: the reversed merge is at every site, four of them predating this PR. But this PR added blurBackgroundStyle, whose doc states upstream merges placeholderStyle last, while every caller then did the opposite. A helper that documents an ordering its callers contradict is this PR's problem regardless of where the callers came from, so the choice was to fix the behavior or delete the claim. The behavior is the one worth having.
Verified against 16.2.7 get-img-props.js:
const imgStyle = Object.assign(fill ? {position:'absolute', ...} : {}, showAltText ? {} : {color:'transparent'}, style)
...
style: { ...imgStyle, ...placeholderStyle }Two orderings to preserve, and I kept both: the caller's style wins over the fill defaults, and placeholderStyle wins over everything while it is showing. So getFillStyle became {...fillDefaults, ...style, ...backgroundStyle} and the five non-fill sites became {...style, ...blurStyle} — all of them, not just the two custom-loader paths, since fixing those alone would have left the loader paths disagreeing with the other three.
Regression test keeps the blur placeholder above a caller background style in image-component.test.ts, covering fill and non-fill. It asserts the blur URL wins rather than merely appearing — the old behavior still emitted the placeholder into the style object, just invisibly, so a containment check would have passed against the bug. Confirmed it fails on the pre-fix ordering.
Also re-ran tests/e2e/app-router/nextjs-compat/image.spec.ts, which covers placeholder removal after load: passes, so the caller's style still takes over once blurComplete drops the placeholder.
Upstream merges `placeholderStyle` after `imgStyle` — which already contains the caller's `style` — so the placeholder wins for as long as it is showing (`get-img-props.ts:574-577`). vinext had the merge reversed at every site, so an `<Image placeholder="blur">` whose caller also set `backgroundImage`, `backgroundSize`, `backgroundRepeat` or `backgroundPosition` emitted the placeholder into the style object but rendered it invisible. Pre-existing on the unoptimized, remote and both `getImageProps` paths rather than introduced here, but this PR added `blurBackgroundStyle`, whose own comment documents the upstream ordering the callers then contradicted. Fixing only the two custom-loader paths would leave the loader paths disagreeing with the other three, so all five move together, along with `getFillStyle`, which had the same inversion for `fill` images. The caller's style still wins over the fill defaults, matching upstream, and takes over completely once `blurComplete` drops the placeholder. Also normalize the `loaderFile` path expectation with `toSlash`: the resolver returns a normalized path while the fixture root carries native separators, which compares a half-backslash path against a forward-slash one on Windows.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…ader calls Cleanup pass over the shim, no behavior change. `generateImgAttrs` invoked the loader once per candidate width and then a final time to rebuild the widest URL the map had already produced — N+1 calls where N suffice, per image, per render. Loaders are arbitrary user code and a signed-URL loader hashes per call, so materialize the URLs once and reuse the last. Verified: a 400px image now makes 2 loader calls for its 2 candidates, previously 3. `generateImageAttributes` had become a pass-through around `generateImgAttrs` whose only effect was discarding `sizes`; both call sites now call `generateImgAttrs` directly. The blur placeholder repeated the same sanitize-and-guard pair at six sites, of which this branch added two. `blurPlaceholder(blurDataURL, show)` absorbs it and returns the sanitized url for the one caller that needs it bare. `showBlurPlaceholder` answers the `!blurComplete && placeholder === "blur"` question once per render instead of four times. The missing-loader-prop ordering was stated twice, once per path. It is a rule about upstream's ordering, so it gets one home in `bypassesLoaders()`. In `getImageProps`, `skipOpt` evaluated `isSvgUrl` — the only URL parse in the function — before the cheap terms that decide the result anyway. With a configured `loaderFile` the loader term is always true, so every image paid a parse that could not change the outcome; reordered to short-circuit. Also: reattach the `resolveCacheHandlerPathToFilesystem` doc comment, which this branch had stranded above `resolveImageLoaderFile`; drop the unused typed `undefined` in the test stand-in; reuse `startFixtureServer` instead of hand-rolling the same dev server; and shorten the rationale emitted into the generated module, which ships to users' bundles and duplicates the TS doc.
2 participants
Overview
images.loaderFile/images.loaderwork, and make custom loaders emit a realsrcSetvirtual:vinext-image-loadermodule from next.config, and port Next'sgetWidths/generateImgAttrsinto the shim<img>; misconfiguration now fails loudlyCloses #2699.
Why
vinext does not wrap Next's
<Image>.public-shim-map.jsonreplacesnext/imagewholesale withshims/image.tsx. That means none of Next's build-time image machinery carries over automatically, and anext.configkey only works if the shim re-implements it.Upstream implements
loaderFileas a bundler alias:create-compiler-aliases.tsswapsnext/dist/shared/lib/image-loaderfor the user's file. There is no such module left to alias here, so the option was parsed and then dropped. A remotefillimage landed in the plain-<img>branch and rendered<img src="{original}" data-nimg="fill">with no optimization at all.The second defect is independent of config. The
loaderprop path invoked the loader exactly once, asloader({ src, width: imgWidth ?? 0, quality }). Afillimage has nowidth, soimgWidthisundefinedand the?? 0converted "unknown width" into a concrete but wrong0, which flowed straight into user loader code. That path also never built asrcSet.loaderFileis resolved and existence-checked at config time; an unsupportedloadervalue throwsfillpassesundefined, which maps to thedeviceSizesladder withwdescriptorsgetImagePropsmust agree with what<Image>rendersgenerateImgAttrsWhat changed
images.loaderFileset/_next/imageusedloaderFilepath does not existloaderFilemodule has no default exportimages.loader: "custom", noloaderFile, noloaderprop/_next/imageloader-prop errorimages.loader: "imgix"withloaderFilenext/legacy/imageupstreamimages.loader: "imgix"alone, noloaderFile/_next/imageimages.loader: "custom", noloaderFile,unoptimizedplaceholder="blur"getImagePropsqualityomitted75undefinedand applies its own defaultloaderprop, fixed widthsrcSetsrcSetwith1x/2xcandidates;srcis the largest candidateloaderprop,fillwidth: 0, nosrcSetdeviceSizesentry,wdescriptors,sizesdefaults to100vwgetImagePropswith a loaderwidth: 0defect,srcSet: undefined/_next/imagepath is byte-identicalRemote-image optimization through
/_next/imageis unchanged and still unsupported. That path stays reserved for a separate change.Maintainer review path
packages/vinext/src/shims/image.tsxforgetWidths/generateImgAttrsand the loader branch, which is where both defects lived.packages/vinext/src/image/image-loader-virtual.tsfor the generated module and its three states.packages/vinext/src/config/next-config.ts(resolveImageLoaderFile) for path resolution and validation.packages/vinext/src/index.tsfor theresolveId/loadwiring.tests/image-loader-plugin.test.tsfor proof that a real next.config reaches the generated module.Validation
getWidthsandgenerateImgAttrswere checked line by line against the vendored.refs/nextjs-v16.2.6/packages/next/src/shared/lib/get-img-props.ts, including thewvsxdescriptor rule, thewidths[last]choice forsrc, the!sizes && kind === "w"default to100vw, and the attribute ordering comment about Safari prefetchingsrcbeforesrcSetlands.tests/image-loader-config.test.ts: codegen across the three states, plus three tests that write the generated source to disk andimport()it, so quoting and escaping are verified by execution rather than string matching.tests/image-loader-plugin.test.ts: resolves and loadsvirtual:vinext-image-loaderthrough a real Vite server with a realnext.config.mjs, then serves throwaway projects from a dev server so the configured-loader branches run against the module the plugin actually generated. Three of those cases fail against the pre-review commit.getImagePropsregression tests assertingfillnever sendswidth: 0and produces a device-sizesrcSet.Commands and extended results
Neither unit failure is related to this change:
tests/commonjs-loader.test.ts > delegates packages with native addons to Node's loaderglobs for a compiledbetter_sqlite3.nodeaddon that was never built in this working tree. Confirmed failing onmainas well.tests/dynamic-requests-build.test.ts > serves guarded fully dynamic requests in pages and route handlers during developmenttimes out under suite parallelism and passes in isolation (43/43).Risk / compatibility
loader,srcis now the largest candidate width rather than the declared width, and asrcSetis emitted. This is upstream behaviour. Two existing tests asserted the previous output and are updated with the derivation of the expected values.images.loaderandimages.loaderFile. Previously these were accepted by the type and ignored, so no working configuration breaks. Configurations that were silently broken now fail at config time, which is the intent./_next/imagepath produces identical output.getImageWidthsis preserved and reused rather than reimplemented, so the fixed-widthx-descriptor behaviour is untouched.Non-goals
/_next/imageoptimizer.parseImageParamsstill accepts only path-relative URLs, and the SSRF surface that opens deserves its own change.sizes-lessfillcase for the built-in loader, which is next/image: fixed-size images emit w-descriptor srcSet without sizes (Next.js emits 1x/2x) #1966 and shares this code but has a wider blast radius.imgix,cloudinary,akamai). They exist only innext/legacy/imageupstream and are now rejected rather than partially honoured.overrideSrcprop is still not applied in the loader branch. Pre-existing, unrelated, left alone to keep the diff reviewable.References
.refs/nextjs-v16.2.6/packages/next/src/shared/lib/get-img-props.tsgetWidths/generateImgAttrs.refs/nextjs-v16.2.6/packages/next/src/build/create-compiler-aliases.tsloaderFilealiasing this replaces.refs/nextjs-v16.2.6/packages/next/src/server/config.tsloaderFilevalidation and error messagessrcSetbehaviour in the same file, deliberately out of scopeNote on a separate parity gap
While verifying widths I found an unrelated pre-existing divergence: vinext defaults
images.imageSizesto[16, 32, 48, 64, 96, 128, 256, 384]inindex.tsandshims/image.tsx, but upstreamimageConfigDefaultis[32, 48, 64, 96, 128, 256, 384]with no16. That extra entry changesallSizesand therefore srcSet candidate selection. Not touched here; happy to open a separate issue.