From 1271179b20a939ea4bd980ff10bdfa18663ff80b Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Sat, 25 Jul 2026 03:37:36 +1000 Subject: [PATCH 1/8] feat(image): support images.loaderFile and custom-loader srcSet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #2699 --- .changeset/image-custom-loader-support.md | 3 + packages/vinext/src/check.ts | 2 +- packages/vinext/src/config/next-config.ts | 51 +++++ .../src/image/image-loader-unconfigured.ts | 16 ++ .../vinext/src/image/image-loader-virtual.ts | 86 +++++++++ packages/vinext/src/index.ts | 12 ++ packages/vinext/src/shims/image.tsx | 180 ++++++++++++++---- .../src/virtual-vinext-image-loader.d.ts | 19 ++ scripts/check-shim-types.mjs | 1 + tests/image-component.test.ts | 60 +++++- tests/image-loader-config.test.ts | 179 +++++++++++++++++ tests/image-loader-plugin.test.ts | 81 ++++++++ tests/shims.test.ts | 15 +- vite.config.ts | 10 + 14 files changed, 669 insertions(+), 46 deletions(-) create mode 100644 .changeset/image-custom-loader-support.md create mode 100644 packages/vinext/src/image/image-loader-unconfigured.ts create mode 100644 packages/vinext/src/image/image-loader-virtual.ts create mode 100644 packages/vinext/src/virtual-vinext-image-loader.d.ts create mode 100644 tests/image-loader-config.test.ts create mode 100644 tests/image-loader-plugin.test.ts diff --git a/.changeset/image-custom-loader-support.md b/.changeset/image-custom-loader-support.md new file mode 100644 index 0000000000..ef6a76c9ed --- /dev/null +++ b/.changeset/image-custom-loader-support.md @@ -0,0 +1,3 @@ +--- +"vinext": minor +--- diff --git a/packages/vinext/src/check.ts b/packages/vinext/src/check.ts index f5d22391e8..984ea3df95 100644 --- a/packages/vinext/src/check.ts +++ b/packages/vinext/src/check.ts @@ -186,7 +186,7 @@ const CONFIG_SUPPORT: Record = { images: { status: "partial", detail: - "remotePatterns validated; on-the-fly optimization via images.optimizer (Cloudflare Images), passthrough otherwise", + "remotePatterns validated; loader/loaderFile supported; on-the-fly optimization via images.optimizer (Cloudflare Images), passthrough otherwise", }, allowedDevOrigins: { status: "supported", detail: "dev server cross-origin allowlist" }, output: { diff --git a/packages/vinext/src/config/next-config.ts b/packages/vinext/src/config/next-config.ts index 3c2b9917b3..22e5b1507e 100644 --- a/packages/vinext/src/config/next-config.ts +++ b/packages/vinext/src/config/next-config.ts @@ -230,6 +230,20 @@ export type NextConfig = { dangerouslyAllowSVG?: boolean; /** Allow image optimization for hostnames that resolve to private IP addresses. This is a security risk (SSRF) — only enable for private networks when you understand the risk. */ dangerouslyAllowLocalIP?: boolean; + /** + * Which image loader generates image URLs. Only "default" (the built-in + * `/_next/image` endpoint) and "custom" (paired with `loaderFile`) are + * implemented — the named CDN loaders exist only in `next/legacy/image` + * upstream and are rejected here rather than silently ignored. + */ + loader?: "default" | "custom"; + /** + * Path to a file whose default export is an image loader function + * `({ src, width, quality }) => string`. Resolved relative to the project + * root. When set, it replaces the built-in `/_next/image` loader for every + * image that does not pass its own `loader` prop. + */ + loaderFile?: string; /** Content-Disposition header for image responses. Defaults to "inline". */ contentDispositionType?: "inline" | "attachment"; /** Content-Security-Policy header for image responses. Defaults to "script-src 'none'; frame-src 'none'; sandbox;" */ @@ -1229,6 +1243,42 @@ export function createRscCompatibilityId( * @param filePath - Absolute path, relative path, or file:// URL (e.g. from import.meta.resolve) * @returns A filesystem path suitable for path operations */ +/** + * Resolve `images.loaderFile` to an absolute path, mirroring Next.js's checks in + * `server/config.ts`. + * + * Failing at config time is deliberate: a mistyped path that silently fell back + * to the built-in loader is indistinguishable from `loaderFile` not being + * supported at all, which is the exact failure mode this plumbing exists to fix. + * + * `loader: "custom"` without a `loaderFile` is intentionally NOT an error — + * upstream allows it, and defers to a per-image `loader` prop. The shim reports + * that case at render time instead. + */ +function resolveImageLoaderFile( + images: NonNullable, + root: string, +): string | undefined { + const { loaderFile } = images; + if (typeof loaderFile !== "string" || loaderFile === "") return undefined; + + // Widened deliberately: next.config is user-authored JavaScript, so the + // declared union is a hint, not a guarantee. A `loader: "imgix"` left over + // from a Next.js project must be reported rather than silently accepted. + const loader: string | undefined = images.loader; + if (loader !== undefined && loader !== "default" && loader !== "custom") { + throw new Error( + `Specified images.loader property (${loader}) cannot be used with images.loaderFile property. Please set images.loader to "custom".`, + ); + } + + const absolutePath = toSlash(path.resolve(root, loaderFile)); + if (!fs.existsSync(absolutePath)) { + throw new Error(`Specified images.loaderFile does not exist at "${absolutePath}".`); + } + return absolutePath; +} + function resolveCacheHandlerPathToFilesystem(filePath: string): string { // toSlash: fileURLToPath and user-supplied require.resolve() results are // backslash-separated on Windows; normalize into slash space. @@ -1876,6 +1926,7 @@ export async function resolveNextConfig( const images = config.images ? { ...config.images, + loaderFile: resolveImageLoaderFile(config.images, root), remotePatterns: config.images.remotePatterns?.map((pattern) => pattern instanceof URL ? { diff --git a/packages/vinext/src/image/image-loader-unconfigured.ts b/packages/vinext/src/image/image-loader-unconfigured.ts new file mode 100644 index 0000000000..19ac9b817a --- /dev/null +++ b/packages/vinext/src/image/image-loader-unconfigured.ts @@ -0,0 +1,16 @@ +/** + * Stand-in for the generated `virtual:vinext-image-loader` module. + * + * Tests import the `next/image` shim directly, without the vinext vite plugin, + * so the virtual module has no resolver. This file is aliased in place of it + * (see `WORKSPACE_SRC_ALIAS` in vite.config.ts) and mirrors what the generator + * emits when `images.loaderFile` is not configured. + * + * Tests that need a configured loader should assert on + * `generateImageLoaderModule()` output rather than trying to swap this module. + */ +const imageLoader: + | ((props: { src: string; width: number; quality?: number }) => string) + | undefined = undefined; + +export default imageLoader; diff --git a/packages/vinext/src/image/image-loader-virtual.ts b/packages/vinext/src/image/image-loader-virtual.ts new file mode 100644 index 0000000000..65df88af61 --- /dev/null +++ b/packages/vinext/src/image/image-loader-virtual.ts @@ -0,0 +1,86 @@ +/** + * Code generation for the `virtual:vinext-image-loader` module, resolved by the + * vinext vite plugin from `images.loaderFile` in next.config. + * + * Next.js implements `loaderFile` as a bundler alias: the module that + * `next/image` imports for its default loader + * (`next/dist/shared/lib/image-loader`) is swapped for the user's file — see + * `create-compiler-aliases.ts` upstream. vinext replaces `next/image` wholesale + * with its own shim, so there is no upstream module left to alias. Instead the + * shim imports this virtual module unconditionally and the plugin generates + * either a re-export of the user's loader or an inert stub. + * + * The generated module always has a default export, so the shim's unconditional + * import stays valid whether or not `loaderFile` is configured. + * + * This mirrors the adapter pattern in `image/image-adapters-virtual.ts`. + */ + +/** Public virtual module id imported by the `next/image` shim. */ +export const VIRTUAL_IMAGE_LOADER = "virtual:vinext-image-loader"; + +/** + * Next.js's error for a `loaderFile` whose module has no default export. + * Kept verbatim so existing troubleshooting docs and searches still apply. + */ +const MISSING_DEFAULT_EXPORT_ERROR = + "images.loaderFile detected but the file is missing default export.\n" + + "Read more: https://nextjs.org/docs/messages/invalid-images-config"; + +/** + * Generate the source of the `virtual:vinext-image-loader` module. + * + * @param images The resolved `images` block from next.config. `loaderFile` is + * expected to already be an absolute path (see `resolveImageLoaderFile`). + */ +export function generateImageLoaderModule(images?: { + loader?: "default" | "custom"; + loaderFile?: string; +}): string { + const loaderFile = images?.loaderFile; + + // `loader: "custom"` with no `loaderFile` means every image must supply its + // own `loader` prop. Export a loader that reports the omission instead of + // silently falling back to `/_next/image`, matching upstream's `customLoader`. + if (!loaderFile && images?.loader === "custom") { + return [ + '// vinext: images.loader is "custom" with no images.loaderFile — each', + "// must pass its own `loader` prop.", + "export default function customImageLoader({ src }) {", + " throw new Error(", + " 'Image with src \"' + src + '\" is missing \"loader\" prop.\\n' +", + " 'Read more: https://nextjs.org/docs/messages/next-image-missing-loader',", + " );", + "}", + "", + ].join("\n"); + } + + // Nothing configured → the shim falls back to its built-in `/_next/image` + // loader. `undefined` (not `null`) so the shim's `??` fallback reads naturally. + if (!loaderFile) { + return [ + "// vinext: no images.loaderFile configured — the built-in /_next/image loader is used.", + "export default undefined;", + "", + ].join("\n"); + } + + return [ + "// vinext: generated from `images.loaderFile` in your next.config.", + `import * as __vinextUserImageLoaderModule from ${JSON.stringify(loaderFile)};`, + "", + "const __vinextImageLoader = __vinextUserImageLoaderModule.default;", + "", + "// Configuring loaderFile is an explicit opt-in, so a module that cannot", + "// supply a loader is a config error worth failing on immediately rather", + "// than silently falling back to the built-in optimizer — which would be", + "// indistinguishable from the loaderFile being ignored.", + "if (typeof __vinextImageLoader !== 'function') {", + ` throw new Error(${JSON.stringify(MISSING_DEFAULT_EXPORT_ERROR)});`, + "}", + "", + "export default __vinextImageLoader;", + "", + ].join("\n"); +} diff --git a/packages/vinext/src/index.ts b/packages/vinext/src/index.ts index cedab094af..631e9adc50 100644 --- a/packages/vinext/src/index.ts +++ b/packages/vinext/src/index.ts @@ -58,6 +58,7 @@ import { generateImageAdaptersModule, type VinextImageConfig, } from "./image/image-adapters-virtual.js"; +import { VIRTUAL_IMAGE_LOADER, generateImageLoaderModule } from "./image/image-loader-virtual.js"; import { generateBrowserEntry, toLinkPrefetchRoutes } from "./entries/app-browser-entry.js"; import { collectRouteClassificationManifest, @@ -1109,6 +1110,8 @@ const RESOLVED_ROOT_PARAMS = VIRTUAL_PREFIX + VIRTUAL_ROOT_PARAMS; const RESOLVED_CACHE_ADAPTERS = VIRTUAL_PREFIX + VIRTUAL_CACHE_ADAPTERS; /** Virtual module that registers the config-driven image optimizer (see VinextOptions.images). */ const RESOLVED_IMAGE_ADAPTERS = VIRTUAL_PREFIX + VIRTUAL_IMAGE_ADAPTERS; +/** Virtual module exposing `images.loaderFile` to the next/image shim. */ +const RESOLVED_IMAGE_LOADER = VIRTUAL_PREFIX + VIRTUAL_IMAGE_LOADER; /** Virtual module for composed instrumentation-client bootstrap. */ const VIRTUAL_INSTRUMENTATION_CLIENT = "private-next-instrumentation-client"; const RESOLVED_INSTRUMENTATION_CLIENT = `${VIRTUAL_PREFIX}${VIRTUAL_INSTRUMENTATION_CLIENT}.mjs`; @@ -3654,6 +3657,9 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { ) { return RESOLVED_IMAGE_ADAPTERS; } + if (cleanId === VIRTUAL_IMAGE_LOADER || cleanId.endsWith("/" + VIRTUAL_IMAGE_LOADER)) { + return RESOLVED_IMAGE_LOADER; + } if (cleanId.startsWith(VIRTUAL_GOOGLE_FONTS + "?")) { return RESOLVED_VIRTUAL_GOOGLE_FONTS + cleanId.slice(VIRTUAL_GOOGLE_FONTS.length); } @@ -3817,6 +3823,12 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { if (id === RESOLVED_IMAGE_ADAPTERS) { return generateImageAdaptersModule(options.images); } + if (id === RESOLVED_IMAGE_LOADER) { + // From next.config's `images`, not the vinext() plugin `images` + // option — `loaderFile` is a Next.js config key, and the path was + // already resolved and existence-checked by resolveNextConfig(). + return generateImageLoaderModule(nextConfig?.images); + } if (id === RESOLVED_APP_SSR_ENTRY && hasAppDir) { return generateSsrEntry(hasPagesDir); } diff --git a/packages/vinext/src/shims/image.tsx b/packages/vinext/src/shims/image.tsx index 795566812a..f51dbc4143 100644 --- a/packages/vinext/src/shims/image.tsx +++ b/packages/vinext/src/shims/image.tsx @@ -26,6 +26,12 @@ import type { import { getDeploymentId } from "../utils/deployment-id.js"; import { hasRemoteMatch, isPrivateIp, type RemotePattern } from "./image-config.js"; import { useMergedRef } from "./use-merged-ref.js"; +// Generated by the vinext plugin from `images.loaderFile` in next.config. +// `undefined` when unconfigured, in which case the built-in `/_next/image` +// 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 from "virtual:vinext-image-loader"; export type { ImageLoader, StaticImageData, StaticRequire }; export type ImageLoaderProps = Parameters[0]; @@ -333,45 +339,105 @@ function getImageWidths(width: number): number[] { ]; } -function generateImageAttributes( - src: string, - width: number, - quality: number = 75, - sizes?: string, -): { src: string; srcSet: string } { +/** + * Pick the candidate widths for a srcSet and the descriptor they use. + * + * Ported from Next.js `getWidths` (shared/lib/get-img-props.ts). The three + * cases are meaningfully different, and collapsing them is what produced the + * `width: 0` bug: an image with no intrinsic width (`fill`) is not a zero-width + * image, it is an image whose width is decided by the viewport, so it gets the + * full device-size ladder with `w` descriptors rather than a single fixed URL. + */ +function getWidths( + width: number | undefined, + sizes: string | undefined, +): { widths: number[]; kind: "w" | "x" } { if (sizes) { const viewportWidthPattern = /(^|\s)(1?\d?\d)vw/g; const viewportPercentages = Array.from(sizes.matchAll(viewportWidthPattern), (match) => Number.parseInt(match[2], 10), ); - const minimumWidth = - viewportPercentages.length > 0 - ? RESPONSIVE_WIDTHS[0] * (Math.min(...viewportPercentages) * 0.01) - : 0; - const candidates = ALL_IMAGE_WIDTHS.filter((candidateWidth) => candidateWidth >= minimumWidth); - return { - src: imageOptimizationUrl(src, candidates[candidates.length - 1], quality), - srcSet: candidates - .map( - (candidateWidth) => - `${imageOptimizationUrl(src, candidateWidth, quality)} ${candidateWidth}w`, - ) - .join(", "), - }; + if (viewportPercentages.length > 0) { + const smallestRatio = Math.min(...viewportPercentages) * 0.01; + const candidates = ALL_IMAGE_WIDTHS.filter( + (candidateWidth) => candidateWidth >= RESPONSIVE_WIDTHS[0] * smallestRatio, + ); + // A deviceSizes config whose largest entry is under `smallestRatio` times + // its smallest entry filters everything out. Upstream would index into an + // empty array and hand `undefined` to the loader; keep the largest width + // instead so a pathological config degrades rather than emitting + // `width=undefined` into user-visible URLs. + return { + widths: + candidates.length > 0 ? candidates : [ALL_IMAGE_WIDTHS[ALL_IMAGE_WIDTHS.length - 1]], + kind: "w", + }; + } + return { widths: ALL_IMAGE_WIDTHS, kind: "w" }; + } + + if (typeof width !== "number") { + return { widths: RESPONSIVE_WIDTHS, kind: "w" }; } - const widths = getImageWidths(width); + return { widths: getImageWidths(width), kind: "x" }; +} + +/** + * Build `src`/`srcSet`/`sizes` by invoking `loader` once per candidate width. + * + * Ported from Next.js `generateImgAttrs`. `loader` is the seam that makes this + * work for both the built-in `/_next/image` endpoint and a user-supplied + * loader (`loader` prop or `images.loaderFile`) — upstream swaps exactly the + * same function and keeps the width selection identical. + */ +function generateImgAttrs(input: { + src: string; + width: number | undefined; + quality: number; + sizes: string | undefined; + loader: ImageLoader; +}): { src: string; srcSet: string; sizes: string | undefined } { + const { src, width, quality, sizes, loader } = input; + const { widths, kind } = getWidths(width, sizes); return { - src: imageOptimizationUrl(src, widths[widths.length - 1], quality), + // Width-descriptor srcSets are meaningless to the browser without `sizes`, + // so upstream supplies `100vw` when the caller omitted it. + sizes: !sizes && kind === "w" ? "100vw" : sizes, srcSet: widths .map( (candidateWidth, index) => - `${imageOptimizationUrl(src, candidateWidth, quality)} ${index + 1}x`, + `${loader({ src, width: candidateWidth, quality })} ${ + kind === "w" ? candidateWidth : index + 1 + }${kind}`, ) .join(", "), + // Keep `src` last: React applies attributes in order, and a `src` set + // before its `srcSet` makes the browser start a throwaway request. + src: loader({ src, width: widths[widths.length - 1], quality }), }; } +/** The built-in loader: routes through the `/_next/image` optimization endpoint. */ +const builtInImageLoader: ImageLoader = ({ src, width, quality }) => + imageOptimizationUrl(src, width, quality); + +function generateImageAttributes( + src: string, + width: number, + quality: number = 75, + sizes?: string, +): { src: string; srcSet: string } { + const { src: resolvedSrc, srcSet } = generateImgAttrs({ + src, + width, + quality, + sizes, + loader: builtInImageLoader, + }); + return { src: resolvedSrc, srcSet }; +} + const Image = forwardRef(function Image( { src: srcProp, @@ -574,27 +640,46 @@ const Image = forwardRef(function Image( ); } - // If a custom loader is provided, use basic img with loader URL - if (loader) { + // A custom loader owns URL generation entirely, so the `/_next/image` + // endpoint is bypassed — but the loader is still invoked once per candidate + // width so the browser receives a real srcSet. `fill` images deliberately + // pass `undefined` rather than a width: they have no intrinsic width, and + // substituting 0 would send a meaningless `width: 0` into user loader code. + // + // This branch sits ahead of the remote-URL handling below so a configured + // loader also owns remote sources, matching Next.js — where `loaderFile` + // replaces the default loader for every image, local or remote. + const effectiveLoader = loader ?? configuredImageLoader; + if (effectiveLoader) { const resolvedQuality = typeof quality === "string" ? Number(quality) : (quality ?? 75); - const resolvedSrc = loader({ src, width: imgWidth ?? 0, quality: resolvedQuality }); + const attributes = generateImgAttrs({ + src, + width: fill ? undefined : imgWidth, + quality: resolvedQuality, + sizes, + loader: effectiveLoader, + }); preloadImageResource({ shouldPreload, - src: resolvedSrc, - sizes, + src: attributes.src, + srcSet: attributes.srcSet, + sizes: attributes.sizes, fetchPriority: priorityFetchPriority, }); return ( {alt}` would render — callers building elements + // depend on the two agreeing. const imgQuality = typeof _quality === "string" ? Number(_quality) : (_quality ?? 75); - const resolvedSrc = blockedInProd - ? "" - : loader - ? loader({ src, width: imgWidth ?? 0, quality: imgQuality }) - : src; + const effectiveLoader = loader ?? configuredImageLoader; + const loaderAttributes = + effectiveLoader && !blockedInProd + ? generateImgAttrs({ + src, + width: fill ? undefined : imgWidth, + quality: imgQuality, + sizes, + loader: effectiveLoader, + }) + : null; + const resolvedSrc = blockedInProd ? "" : (loaderAttributes?.src ?? src); // For local images (no loader, not remote), route through optimization endpoint. // When `unoptimized` is true, bypass the endpoint entirely (Next.js compat). // SVG sources auto-skip unless dangerouslyAllowSVG is enabled. const isSvg = isSvgUrl(resolvedSrc); const skipOpt = - (isSvg && !__dangerouslyAllowSVG) || blockedInProd || !!loader || isRemoteUrl(resolvedSrc); + (isSvg && !__dangerouslyAllowSVG) || + blockedInProd || + !!effectiveLoader || + isRemoteUrl(resolvedSrc); const optimizedAttributes = imgWidth && !fill && !skipOpt ? generateImageAttributes(resolvedSrc, imgWidth, imgQuality, sizes) @@ -880,8 +978,10 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { ? optimizedAttributes.src : imageOptimizationUrl(resolvedSrc, RESPONSIVE_WIDTHS[0], imgQuality); - // Build srcSet for local images — each width points to /_next/image - const srcSet = optimizedAttributes?.srcSet; + // Build srcSet for local images — each width points to /_next/image. + // When a custom loader is in play the endpoint is bypassed, so the srcSet + // comes from the loader instead. + const srcSet = optimizedAttributes?.srcSet ?? loaderAttributes?.srcSet; // Blur placeholder styles — sanitize to prevent CSS injection const sanitizedBlurURL = imgBlurDataURL ? sanitizeBlurDataURL(imgBlurDataURL) : undefined; @@ -904,7 +1004,7 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { fetchPriority: priority ? ("high" as const) : undefined, decoding: "async" as const, srcSet, - sizes: sizes ?? (fill ? "100vw" : undefined), + sizes: loaderAttributes?.sizes ?? sizes ?? (fill ? "100vw" : undefined), className, style: fill ? getFillStyle(style, blurStyle) : { ...blurStyle, ...style }, ...rest, diff --git a/packages/vinext/src/virtual-vinext-image-loader.d.ts b/packages/vinext/src/virtual-vinext-image-loader.d.ts new file mode 100644 index 0000000000..403bfe155c --- /dev/null +++ b/packages/vinext/src/virtual-vinext-image-loader.d.ts @@ -0,0 +1,19 @@ +/** + * Type shim for the virtual image-loader module generated by vinext at build + * time from `images.loaderFile` / `images.loader` in next.config. + * + * The default export is the user's image loader, or `undefined` when nothing is + * configured — in which case the `next/image` shim falls back to its built-in + * `/_next/image` loader. See `image/image-loader-virtual.ts` for the generator. + * + * This must stay a *script* (no top-level import/export) so `declare module` is + * an ambient declaration. The same block inside `global.d.ts` would be parsed as + * a module augmentation and silently fail to apply, which is why the virtual + * imports declared there still need `@ts-expect-error` at their use sites. + */ +declare module "virtual:vinext-image-loader" { + const imageLoader: + | ((props: { src: string; width: number; quality?: number }) => string) + | undefined; + export default imageLoader; +} diff --git a/scripts/check-shim-types.mjs b/scripts/check-shim-types.mjs index 1e55c4f99e..14ed5d7971 100644 --- a/scripts/check-shim-types.mjs +++ b/scripts/check-shim-types.mjs @@ -53,6 +53,7 @@ function renderContracts(tempDir) { `/// `, `/// `, `/// `, + `/// `, "", ]; diff --git a/tests/image-component.test.ts b/tests/image-component.test.ts index 2d182b7dd7..450fd23b56 100644 --- a/tests/image-component.test.ts +++ b/tests/image-component.test.ts @@ -212,7 +212,41 @@ describe("Image SSR rendering", () => { loader, }), ); - expect(html).toContain('src="https://cdn.example.com/photo.jpg?w=200&q=75"'); + // Expected widths come from Next.js `getWidths`: with no `sizes` and a + // numeric width, candidates are [width, width * 2] each snapped up to the + // next configured size — 200 -> 256, 400 -> 640 — with `x` descriptors. + // `src` is the *largest* candidate, not the declared width, so browsers + // that ignore srcSet still get the highest-fidelity image. + expect(html).toContain( + 'srcSet="https://cdn.example.com/photo.jpg?w=256&q=75 1x, https://cdn.example.com/photo.jpg?w=640&q=75 2x"', + ); + expect(html).toContain('src="https://cdn.example.com/photo.jpg?w=640&q=75"'); + }); + + it("invokes a custom loader per device size for fill images", () => { + const widths: number[] = []; + const loader = ({ src, width }: { src: string; width: number }) => { + widths.push(width); + return `https://cdn.example.com${src}?w=${width}`; + }; + + const html = ReactDOMServer.renderToString( + React.createElement(Image, { + alt: "cdn fill", + src: "/photo.jpg", + fill: true, + loader, + }), + ); + + // A `fill` image has no intrinsic width. Regression guard for the shim + // previously collapsing that to `width: 0` and calling the loader once. + expect(widths).not.toContain(0); + expect(new Set(widths)).toEqual(new Set([640, 750, 828, 1080, 1200, 1920, 2048, 3840])); + expect(html).toContain("https://cdn.example.com/photo.jpg?w=640 640w"); + expect(html).toContain("https://cdn.example.com/photo.jpg?w=3840 3840w"); + // Width descriptors are unusable without `sizes`; Next.js defaults it. + expect(html).toContain('sizes="100vw"'); }); it("renders StaticImageData (import result)", () => { @@ -429,7 +463,29 @@ describe("getImageProps", () => { loader, }); - expect(props.src).toBe("https://cdn.example.com/photo.jpg?w=300"); + // Per Next.js `getWidths`: 300 -> 384, 600 -> 640, `x` descriptors, and + // `src` is the largest candidate. + expect(props.src).toBe("https://cdn.example.com/photo.jpg?w=640"); + expect(props.srcSet).toBe( + "https://cdn.example.com/photo.jpg?w=384 1x, https://cdn.example.com/photo.jpg?w=640 2x", + ); + }); + + it("returns a device-size srcSet for fill images with a custom loader", () => { + const widths: number[] = []; + const loader = ({ src, width }: { src: string; width: number }) => { + widths.push(width); + return `https://cdn.example.com${src}?w=${width}`; + }; + + const { props } = getImageProps({ alt: "cdn fill", src: "/photo.jpg", fill: true, loader }); + + // Regression guard: `getImageProps` shares the component's loader path, so + // a built from these props must agree with what renders. + expect(widths).not.toContain(0); + expect(props.src).toBe("https://cdn.example.com/photo.jpg?w=3840"); + expect(props.srcSet).toContain("https://cdn.example.com/photo.jpg?w=640 640w"); + expect(props.sizes).toBe("100vw"); }); it("returns blur placeholder styles", () => { diff --git a/tests/image-loader-config.test.ts b/tests/image-loader-config.test.ts new file mode 100644 index 0000000000..3df1258bfe --- /dev/null +++ b/tests/image-loader-config.test.ts @@ -0,0 +1,179 @@ +/** + * `images.loaderFile` / `images.loader` support. + * + * Covers: + * - generateImageLoaderModule() codegen for the `virtual:vinext-image-loader` + * module across the unconfigured / loaderFile / bare-"custom" permutations. + * - resolveNextConfig() path resolution and the config-time validation that + * turns a mistyped loaderFile into an error instead of a silent fallback to + * the built-in `/_next/image` loader. + */ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import { describe, it, expect } from "vite-plus/test"; +import { + generateImageLoaderModule, + VIRTUAL_IMAGE_LOADER, +} from "../packages/vinext/src/image/image-loader-virtual.js"; +import { resolveNextConfig } from "../packages/vinext/src/config/next-config.js"; + +/** + * Create a throwaway project root containing `loader.mjs` (a valid loader) and + * `no-default.mjs` (a module without a default export). `.mjs` because the temp + * dir has no package.json, so Node would treat `.js` as CommonJS. + */ +function makeProjectRoot(): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "vinext-image-loader-")); + fs.writeFileSync( + path.join(root, "loader.mjs"), + "export default ({ src, width, quality }) =>\n" + + " `https://cdn.test${src}?w=${width}&q=${quality ?? 75}`;\n", + ); + fs.writeFileSync(path.join(root, "no-default.mjs"), "export const notALoader = 1;\n"); + return root; +} + +/** Write generated source into `root` and import it as a real ES module. */ +async function importGenerated(root: string, name: string, code: string): Promise { + const modulePath = path.join(root, name); + fs.writeFileSync(modulePath, code); + return import(pathToFileURL(modulePath).href); +} + +// ─── codegen ─────────────────────────────────────────────────────────────── + +describe("generateImageLoaderModule", () => { + it("exposes the public virtual module id", () => { + expect(VIRTUAL_IMAGE_LOADER).toBe("virtual:vinext-image-loader"); + }); + + it("emits an undefined default export when nothing is configured", () => { + // The shim imports this module unconditionally and falls back to the + // built-in loader on `undefined`, so the export must exist either way. + expect(generateImageLoaderModule()).toContain("export default undefined;"); + expect(generateImageLoaderModule({})).toContain("export default undefined;"); + }); + + it("re-exports the configured loaderFile's default export", () => { + const code = generateImageLoaderModule({ loaderFile: "/project/my-loader.js" }); + expect(code).toContain( + 'import * as __vinextUserImageLoaderModule from "/project/my-loader.js";', + ); + expect(code).toContain("export default __vinextImageLoader;"); + }); + + it("fails loudly when the loaderFile has no default export", () => { + const code = generateImageLoaderModule({ loaderFile: "/project/my-loader.js" }); + expect(code).toContain("if (typeof __vinextImageLoader !== 'function')"); + // A real newline in the emitted source, not a literal backslash-n. + expect(code).toContain( + '"images.loaderFile detected but the file is missing default export.\\nRead more:', + ); + }); + + it('reports the missing loader prop for loader:"custom" with no loaderFile', () => { + // Upstream treats this as a per-image error rather than a config error, + // because a `loader` prop on each is legitimate usage. + const code = generateImageLoaderModule({ loader: "custom" }); + expect(code).toContain("export default function customImageLoader({ src })"); + expect(code).toContain('is missing "loader" prop.'); + expect(code).not.toContain("export default undefined;"); + }); + + it('prefers the loaderFile when both it and loader:"custom" are set', () => { + const code = generateImageLoaderModule({ loader: "custom", loaderFile: "/project/l.js" }); + expect(code).toContain('from "/project/l.js"'); + expect(code).not.toContain("customImageLoader"); + }); +}); + +// ─── generated modules, executed ─────────────────────────────────────────── +// +// The generator emits JavaScript source as strings, so quoting and escaping are +// only really verified by running the result. + +describe("generated virtual:vinext-image-loader module", () => { + it("re-exports a working loader from the configured loaderFile", async () => { + const root = makeProjectRoot(); + const mod = (await importGenerated( + root, + "generated-loader.mjs", + generateImageLoaderModule({ loaderFile: path.join(root, "loader.mjs") }), + )) as { default: (p: { src: string; width: number; quality?: number }) => string }; + + expect(mod.default({ src: "/photo.jpg", width: 640, quality: 80 })).toBe( + "https://cdn.test/photo.jpg?w=640&q=80", + ); + }); + + it("throws the upstream message when the loaderFile has no default export", async () => { + const root = makeProjectRoot(); + await expect( + importGenerated( + root, + "generated-nodefault.mjs", + generateImageLoaderModule({ loaderFile: path.join(root, "no-default.mjs") }), + ), + ).rejects.toThrow( + "images.loaderFile detected but the file is missing default export.\n" + + "Read more: https://nextjs.org/docs/messages/invalid-images-config", + ); + }); + + it('throws the upstream missing-loader-prop message for bare loader:"custom"', async () => { + const root = makeProjectRoot(); + const mod = (await importGenerated( + root, + "generated-custom.mjs", + generateImageLoaderModule({ loader: "custom" }), + )) as { default: (p: { src: string; width: number }) => string }; + + expect(() => mod.default({ src: "/photo.jpg", width: 640 })).toThrow( + 'Image with src "/photo.jpg" is missing "loader" prop.\n' + + "Read more: https://nextjs.org/docs/messages/next-image-missing-loader", + ); + }); +}); + +// ─── config resolution ───────────────────────────────────────────────────── + +describe("resolveNextConfig images.loaderFile", () => { + 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`); + }); + + it("leaves loaderFile undefined when unset", async () => { + const config = await resolveNextConfig({ images: { unoptimized: true } }); + expect(config.images?.loaderFile).toBeUndefined(); + }); + + it("throws when the loaderFile does not exist", async () => { + const root = makeProjectRoot(); + // The failure this guards: a typo'd path silently falling back to the + // built-in loader looks identical to loaderFile not being supported. + await expect(resolveNextConfig({ images: { loaderFile: "./nope.js" } }, root)).rejects.toThrow( + /images.loaderFile does not exist/, + ); + }); + + it("rejects a loaderFile paired with an unsupported named loader", async () => { + const root = makeProjectRoot(); + await expect( + resolveNextConfig( + // Named CDN loaders exist only in next/legacy/image upstream. + { images: { loader: "imgix" as "custom", loaderFile: "./loader.mjs" } }, + root, + ), + ).rejects.toThrow(/cannot be used with images.loaderFile/); + }); + + it('allows loader:"custom" with no loaderFile', async () => { + const config = await resolveNextConfig({ images: { loader: "custom" } }); + expect(config.images?.loader).toBe("custom"); + expect(config.images?.loaderFile).toBeUndefined(); + }); +}); diff --git a/tests/image-loader-plugin.test.ts b/tests/image-loader-plugin.test.ts new file mode 100644 index 0000000000..4db72cbd51 --- /dev/null +++ b/tests/image-loader-plugin.test.ts @@ -0,0 +1,81 @@ +/** + * End-to-end plugin wiring for `images.loaderFile`. + * + * The codegen and config-resolution units are covered in + * `image-loader-config.test.ts`. This file checks the part those cannot: that + * the vinext plugin actually resolves and loads `virtual:vinext-image-loader` + * from a real next.config, which is the step that was missing entirely and made + * the option look unsupported. + */ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { describe, it, expect } from "vite-plus/test"; +import { createServer } from "vite"; +import vinext from "../packages/vinext/src/index.js"; + +/** + * Build a throwaway project root with an app dir, a loader file, and the given + * next.config body. `node_modules` is symlinked so plugin-internal resolution + * behaves as it would in a real project. + */ +function createProject(nextConfigBody: string): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "vinext-loaderfile-plugin-")); + fs.mkdirSync(path.join(root, "app"), { recursive: true }); + fs.writeFileSync( + path.join(root, "app", "page.tsx"), + "export default function Page() { return null; }\n", + ); + fs.writeFileSync( + path.join(root, "my-loader.js"), + "export default ({ src, width, quality }) =>\n" + + " `https://images.example.com${src}?width=${width}&quality=${quality ?? 75}`;\n", + ); + fs.writeFileSync(path.join(root, "next.config.mjs"), nextConfigBody); + fs.symlinkSync( + path.resolve(import.meta.dirname, "..", "node_modules"), + path.join(root, "node_modules"), + "junction", + ); + return root; +} + +/** Resolve + load `virtual:vinext-image-loader` through the real plugin. */ +async function loadVirtualLoaderModule(root: string): Promise { + const server = await createServer({ + root, + configFile: false, + plugins: [vinext({ appDir: root })], + server: { port: 0 }, + logLevel: "silent", + }); + try { + const resolved = await server.pluginContainer.resolveId("virtual:vinext-image-loader"); + expect(resolved).toBeTruthy(); + const loaded = await server.pluginContainer.load(resolved!.id); + return typeof loaded === "string" ? loaded : ((loaded as { code?: string } | null)?.code ?? ""); + } finally { + await server.close(); + } +} + +describe("virtual:vinext-image-loader plugin wiring", () => { + it("loads the configured loaderFile from next.config", async () => { + const root = createProject( + "export default { images: { loader: 'custom', loaderFile: './my-loader.js' } };\n", + ); + const code = await loadVirtualLoaderModule(root); + + // The generator receives an already-absolute path from resolveNextConfig. + expect(code).toContain("my-loader.js"); + expect(code).toContain("export default __vinextImageLoader;"); + expect(code).not.toContain("export default undefined;"); + }); + + it("falls back to the built-in loader when no loaderFile is set", async () => { + const root = createProject("export default { images: { unoptimized: false } };\n"); + const code = await loadVirtualLoaderModule(root); + + expect(code).toContain("export default undefined;"); + }); +}); diff --git a/tests/shims.test.ts b/tests/shims.test.ts index 0fe8ae7b37..3ca80fb19d 100644 --- a/tests/shims.test.ts +++ b/tests/shims.test.ts @@ -21767,8 +21767,13 @@ describe("next/image enhancements", () => { height: 600, loader: ({ src, width, quality }) => `https://cdn.example.com${src}?w=${width}&q=${quality}`, }); - // Custom loader bypasses the /_next/image endpoint - expect(result.props.src).toBe("https://cdn.example.com/photo.jpg?w=800&q=75"); + // Custom loader bypasses the /_next/image endpoint. Per Next.js + // `getWidths`, candidates are [800, 1600] snapped up to [828, 1920], and + // `src` is the largest so non-srcSet browsers get the best image. + expect(result.props.src).toBe("https://cdn.example.com/photo.jpg?w=1920&q=75"); + expect(result.props.srcSet).toBe( + "https://cdn.example.com/photo.jpg?w=828&q=75 1x, https://cdn.example.com/photo.jpg?w=1920&q=75 2x", + ); expect(result.props.src).not.toContain("/_next/image"); }); @@ -21923,7 +21928,11 @@ describe("next/image component rendering", () => { `https://cdn.example.com${src}?w=${width}&q=${quality}`, }), ); - expect(html).toContain('src="https://cdn.example.com/photo.jpg?w=800&q=75"'); + // `src` is the largest candidate width (800 -> 828, 1600 -> 1920). + expect(html).toContain('src="https://cdn.example.com/photo.jpg?w=1920&q=75"'); + expect(html).toContain( + 'srcSet="https://cdn.example.com/photo.jpg?w=828&q=75 1x, https://cdn.example.com/photo.jpg?w=1920&q=75 2x"', + ); }); it("renders with custom sizes attribute", async () => { diff --git a/vite.config.ts b/vite.config.ts index 80c127e073..e0be72bc44 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -11,6 +11,14 @@ const MSW_SETUP = path.resolve(import.meta.dirname, "tests/_msw/setup.ts"); // @vinext/cloudflare dependency edge points at source (single module instance, // no prior build required). Shared by both test projects below. const WORKSPACE_SRC_ALIAS = { + // The next/image shim imports this virtual module, which only the vinext + // plugin can resolve. Tests load the shim source directly, so point it at the + // unconfigured stand-in — the same module the generator emits when + // `images.loaderFile` is unset. + "virtual:vinext-image-loader": path.resolve( + import.meta.dirname, + "packages/vinext/src/image/image-loader-unconfigured.ts", + ), "vinext/shims": SHIMS_SRC, "vinext/internal": VINEXT_SRC, "@vinext/cloudflare/internal": CLOUDFLARE_SRC, @@ -209,6 +217,7 @@ export default defineConfig({ "tests/entry-templates.test.ts", "tests/features.test.ts", "tests/favicon-short-circuit.test.ts", + "tests/image-loader-plugin.test.ts", "tests/image-optimization-parity.test.ts", "tests/node-modules-css.test.ts", "tests/optimize-imports-integration.test.ts", @@ -272,6 +281,7 @@ export default defineConfig({ "tests/entry-templates.test.ts", "tests/favicon-short-circuit.test.ts", "tests/features.test.ts", + "tests/image-loader-plugin.test.ts", "tests/image-optimization-parity.test.ts", "tests/kv-cache-handler.test.ts", "tests/node-modules-css.test.ts", From 3707766599569823fdf98d58607913deffc1955d Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Sat, 25 Jul 2026 09:20:23 +1000 Subject: [PATCH 2/8] fix(image): preserve custom loader URL ownership 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. --- packages/vinext/src/shims/image.tsx | 39 ++++++++------ tests/image-component.test.ts | 82 ++++++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 18 deletions(-) diff --git a/packages/vinext/src/shims/image.tsx b/packages/vinext/src/shims/image.tsx index f51dbc4143..ea63617f24 100644 --- a/packages/vinext/src/shims/image.tsx +++ b/packages/vinext/src/shims/image.tsx @@ -659,9 +659,10 @@ const Image = forwardRef(function Image( sizes, loader: effectiveLoader, }); + const renderedSrc = overrideSrc || attributes.src; preloadImageResource({ shouldPreload, - src: attributes.src, + src: renderedSrc, srcSet: attributes.srcSet, sizes: attributes.sizes, fetchPriority: priorityFetchPriority, @@ -669,7 +670,7 @@ const Image = forwardRef(function Image( return ( {alt} elements render differently. let blockedInProd = false; - if (isRemoteUrl(src)) { + if (!effectiveLoader && isRemoteUrl(src)) { const validation = validateRemoteUrl(src); if (!validation.allowed) { if (__isDev) { @@ -946,18 +951,16 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { // same srcSet `` would render — callers building elements // depend on the two agreeing. const imgQuality = typeof _quality === "string" ? Number(_quality) : (_quality ?? 75); - const effectiveLoader = loader ?? configuredImageLoader; - const loaderAttributes = - effectiveLoader && !blockedInProd - ? generateImgAttrs({ - src, - width: fill ? undefined : imgWidth, - quality: imgQuality, - sizes, - loader: effectiveLoader, - }) - : null; - const resolvedSrc = blockedInProd ? "" : (loaderAttributes?.src ?? src); + const loaderAttributes = effectiveLoader + ? generateImgAttrs({ + src, + width: fill ? undefined : imgWidth, + quality: imgQuality, + sizes, + loader: effectiveLoader, + }) + : null; + const resolvedSrc = loaderAttributes?.src ?? (blockedInProd ? "" : src); // For local images (no loader, not remote), route through optimization endpoint. // When `unoptimized` is true, bypass the endpoint entirely (Next.js compat). @@ -996,7 +999,9 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { : undefined; const imageProps: ImgProps = { - src: optimizedSrc, + // Match Next.js: overrideSrc changes the fallback `src` without changing + // the loader-generated candidates in `srcSet`. + src: overrideSrc || optimizedSrc, alt, width: fill ? undefined : imgWidth, height: fill ? undefined : imgHeight, diff --git a/tests/image-component.test.ts b/tests/image-component.test.ts index 450fd23b56..a801dfc679 100644 --- a/tests/image-component.test.ts +++ b/tests/image-component.test.ts @@ -11,7 +11,11 @@ import { describe, it, expect, vi, afterEach } from "vite-plus/test"; import React from "react"; import ReactDOMServer from "react-dom/server"; -import Image, { getImageProps, type StaticImageData } from "../packages/vinext/src/shims/image.js"; +import Image, { + getImageProps, + type ImageLoader, + type StaticImageData, +} from "../packages/vinext/src/shims/image.js"; /** Helper: expected optimization URL matching what the image shim produces. */ function optUrl(src: string, w: number, q = 75): string { @@ -913,6 +917,82 @@ describe("unoptimized remote images", () => { }); }); +describe("custom loader URL ownership", () => { + afterEach(() => { + vi.unstubAllEnvs(); + delete process.env.__VINEXT_IMAGE_REMOTE_PATTERNS; + vi.resetModules(); + }); + + it("bypasses remote pattern validation in production for Image and getImageProps", async () => { + vi.stubEnv("NODE_ENV", "production"); + process.env.__VINEXT_IMAGE_REMOTE_PATTERNS = JSON.stringify([ + { hostname: "allowed.example.com" }, + ]); + + vi.resetModules(); + const { default: CustomLoaderImage, getImageProps: getCustomLoaderImageProps } = + await import("../packages/vinext/src/shims/image.js"); + const src = "https://unconfigured.example.com/photo.jpg"; + const loader = ({ src: loaderSrc, width: loaderWidth }: Parameters[0]) => + `https://cdn.example.com/image?src=${encodeURIComponent(loaderSrc)}&w=${loaderWidth}`; + const expectedSrc = + "https://cdn.example.com/image?src=https%3A%2F%2Funconfigured.example.com%2Fphoto.jpg&w=640"; + + const html = ReactDOMServer.renderToString( + React.createElement(CustomLoaderImage, { + alt: "custom loader", + src, + width: 300, + height: 200, + loader, + }), + ); + const { props } = getCustomLoaderImageProps({ + alt: "custom loader", + src, + width: 300, + height: 200, + loader, + }); + + expect(html).toContain(`src="${expectedSrc.replace(/&/g, "&")}"`); + expect(props.src).toBe(expectedSrc); + expect(props.srcSet).toContain("https://cdn.example.com/image?"); + }); + + it("uses overrideSrc for the fallback src while preserving the loader srcSet", () => { + const src = "/photo.jpg"; + const overrideSrc = "https://cdn.example.com/original.jpg"; + const loader = ({ width: loaderWidth }: Parameters[0]) => + `https://cdn.example.com/optimized.jpg?w=${loaderWidth}`; + + const html = ReactDOMServer.renderToString( + React.createElement(Image, { + alt: "custom loader override", + src, + overrideSrc, + width: 300, + height: 200, + loader, + }), + ); + const { props } = getImageProps({ + alt: "custom loader override", + src, + overrideSrc, + width: 300, + height: 200, + loader, + }); + + expect(html).toContain(`src="${overrideSrc}"`); + expect(html).toContain("srcSet="); + expect(props.src).toBe(overrideSrc); + expect(props.srcSet).toContain("https://cdn.example.com/optimized.jpg"); + }); +}); + // ─── Reproduction: priority prop on remote URL paths ──────────────────── // Regression tests for: // "Received `true` for a non-boolean attribute `priority`." From 1976cf25a70f24228ec551ac16052d0ea59dc33c Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Sat, 25 Jul 2026 16:52:31 +1000 Subject: [PATCH 3/8] fix(image): validate custom loaders before optimization is decided MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `` 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 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. --- .changeset/image-custom-loader-support.md | 3 - packages/vinext/src/config/next-config.ts | 21 +- .../src/image/image-loader-unconfigured.ts | 4 +- .../vinext/src/image/image-loader-virtual.ts | 17 ++ packages/vinext/src/shims/image.tsx | 95 +++++---- .../src/virtual-vinext-image-loader.d.ts | 10 +- tests/image-component.test.ts | 76 ++++++++ tests/image-configured-loader.test.ts | 181 ++++++++++++++++++ tests/image-loader-config.test.ts | 36 ++++ tests/shims.test.ts | 5 +- 10 files changed, 395 insertions(+), 53 deletions(-) delete mode 100644 .changeset/image-custom-loader-support.md create mode 100644 tests/image-configured-loader.test.ts diff --git a/.changeset/image-custom-loader-support.md b/.changeset/image-custom-loader-support.md deleted file mode 100644 index ef6a76c9ed..0000000000 --- a/.changeset/image-custom-loader-support.md +++ /dev/null @@ -1,3 +0,0 @@ ---- -"vinext": minor ---- diff --git a/packages/vinext/src/config/next-config.ts b/packages/vinext/src/config/next-config.ts index 22e5b1507e..fc35ef52e0 100644 --- a/packages/vinext/src/config/next-config.ts +++ b/packages/vinext/src/config/next-config.ts @@ -1244,8 +1244,8 @@ export function createRscCompatibilityId( * @returns A filesystem path suitable for path operations */ /** - * Resolve `images.loaderFile` to an absolute path, mirroring Next.js's checks in - * `server/config.ts`. + * Validate `images.loader` and resolve `images.loaderFile` to an absolute path, + * mirroring Next.js's checks in `server/config.ts`. * * Failing at config time is deliberate: a mistyped path that silently fell back * to the built-in loader is indistinguishable from `loaderFile` not being @@ -1259,19 +1259,30 @@ function resolveImageLoaderFile( images: NonNullable, root: string, ): string | undefined { - const { loaderFile } = images; - if (typeof loaderFile !== "string" || loaderFile === "") return undefined; + const loaderFile = + typeof images.loaderFile === "string" && images.loaderFile !== "" + ? images.loaderFile + : undefined; // Widened deliberately: next.config is user-authored JavaScript, so the // declared union is a hint, not a guarantee. A `loader: "imgix"` left over // from a Next.js project must be reported rather than silently accepted. + // + // Checked before `loaderFile` is required, because a named loader on its own + // is the more dangerous case: nothing downstream would reject it, so every + // 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") { throw new Error( - `Specified images.loader property (${loader}) cannot be used with images.loaderFile property. Please set images.loader to "custom".`, + loaderFile + ? `Specified images.loader property (${loader}) cannot be used with images.loaderFile property. Please set images.loader to "custom".` + : `Specified images.loader property (${loader}) is not supported. The named CDN loaders exist only in next/legacy/image; set images.loader to "custom" and point images.loaderFile at a loader module instead.`, ); } + if (!loaderFile) return undefined; + const absolutePath = toSlash(path.resolve(root, loaderFile)); if (!fs.existsSync(absolutePath)) { throw new Error(`Specified images.loaderFile does not exist at "${absolutePath}".`); diff --git a/packages/vinext/src/image/image-loader-unconfigured.ts b/packages/vinext/src/image/image-loader-unconfigured.ts index 19ac9b817a..76887035d3 100644 --- a/packages/vinext/src/image/image-loader-unconfigured.ts +++ b/packages/vinext/src/image/image-loader-unconfigured.ts @@ -10,7 +10,9 @@ * `generateImageLoaderModule()` output rather than trying to swap this module. */ const imageLoader: - | ((props: { src: string; width: number; quality?: number }) => string) + | (((props: { src: string; width: number; quality?: number }) => string) & { + __vinext_img_missing_loader?: true; + }) | undefined = undefined; export default imageLoader; diff --git a/packages/vinext/src/image/image-loader-virtual.ts b/packages/vinext/src/image/image-loader-virtual.ts index 65df88af61..a741bb9618 100644 --- a/packages/vinext/src/image/image-loader-virtual.ts +++ b/packages/vinext/src/image/image-loader-virtual.ts @@ -19,6 +19,17 @@ /** Public virtual module id imported by the `next/image` shim. */ export const VIRTUAL_IMAGE_LOADER = "virtual:vinext-image-loader"; +/** + * Property stamped on the generated loader when `images.loader` is `"custom"` + * with no `images.loaderFile`, so the shim can distinguish "the user configured + * a loader" from "the user configured that every image must bring its own". + * + * Mirrors upstream's `__next_img_default` marker on its built-in loader. The + * name is duplicated in `shims/image.tsx` and `virtual-vinext-image-loader.d.ts` + * because the generated module is a string and cannot import this constant. + */ +export const MISSING_CUSTOM_LOADER_MARKER = "__vinext_img_missing_loader"; + /** * Next.js's error for a `loaderFile` whose module has no default export. * Kept verbatim so existing troubleshooting docs and searches still apply. @@ -53,6 +64,12 @@ export function generateImageLoaderModule(images?: { " );", "}", "", + "// Marks this export as a misconfiguration report rather than a working", + "// loader. Upstream raises the missing-prop error before it decides whether", + "// an image is optimized, so the shim needs to tell the two apart: an", + "// `unoptimized` image bypasses a real loader, but must not bypass this one.", + `customImageLoader.${MISSING_CUSTOM_LOADER_MARKER} = true;`, + "", ].join("\n"); } diff --git a/packages/vinext/src/shims/image.tsx b/packages/vinext/src/shims/image.tsx index ea63617f24..3ef42f2e20 100644 --- a/packages/vinext/src/shims/image.tsx +++ b/packages/vinext/src/shims/image.tsx @@ -239,6 +239,25 @@ function getFillStyle( }; } +/** + * The blur placeholder background shown until the real image loads. + * + * Upstream builds this style independently of which loader produced the URLs + * (`get-img-props.ts` merges `placeholderStyle` last, for every path), so the + * caller owns the "should it be showing" decision and this owns only the shape. + * + * @param sanitizedBlurDataURL Must already have passed `sanitizeBlurDataURL` — + * it is interpolated into a CSS `url()` and would otherwise allow injection. + */ +function blurBackgroundStyle(sanitizedBlurDataURL: string): React.CSSProperties { + return { + backgroundImage: `url(${sanitizedBlurDataURL})`, + backgroundSize: "cover", + backgroundRepeat: "no-repeat", + backgroundPosition: "center", + }; +} + /** * Resolve src, width, height, blurDataURL from Image props (string or StaticImageData). * Shared by the Image component and getImageProps to keep behavior in sync. @@ -394,7 +413,10 @@ function getWidths( function generateImgAttrs(input: { src: string; width: number | undefined; - quality: number; + // Deliberately allowed to be `undefined`: upstream applies the quality + // default inside the built-in optimizer loader, not at this seam, so a + // custom loader can apply its own default or let a CDN pick automatically. + quality: number | undefined; sizes: string | undefined; loader: ImageLoader; }): { src: string; srcSet: string; sizes: string | undefined } { @@ -425,7 +447,7 @@ const builtInImageLoader: ImageLoader = ({ src, width, quality }) => function generateImageAttributes( src: string, width: number, - quality: number = 75, + quality?: number, sizes?: string, ): { src: string; srcSet: string } { const { src: resolvedSrc, srcSet } = generateImgAttrs({ @@ -600,7 +622,15 @@ const Image = forwardRef(function Image( } : undefined; - if (_unoptimized === true || __globallyUnoptimized) { + // `images.loader: "custom"` with no `images.loaderFile` makes the generated + // loader a misconfiguration report rather than a working loader. Upstream + // raises that error before it decides whether an image is optimized, so + // `unoptimized` must not skip past it — fall through to the loader branch, + // where invoking the loader surfaces the error carrying this image's src. + const reportsMissingLoaderProp = + loader === undefined && configuredImageLoader?.__vinext_img_missing_loader === true; + + if ((_unoptimized === true || __globallyUnoptimized) && !reportsMissingLoaderProp) { // Unoptimized images are fetched directly by the browser, so intentionally // skip remote URL validation: there is no server-side optimizer fetch and // therefore no SSRF surface. This matches Next.js behavior. @@ -608,12 +638,7 @@ const Image = forwardRef(function Image( const sanitizedBlur = imgBlurDataURL ? sanitizeBlurDataURL(imgBlurDataURL) : undefined; const blurStyle = !blurComplete && placeholder === "blur" && sanitizedBlur - ? { - backgroundImage: `url(${sanitizedBlur})`, - backgroundSize: "cover", - backgroundRepeat: "no-repeat", - backgroundPosition: "center", - } + ? blurBackgroundStyle(sanitizedBlur) : undefined; preloadImageResource({ shouldPreload, @@ -651,7 +676,7 @@ const Image = forwardRef(function Image( // replaces the default loader for every image, local or remote. const effectiveLoader = loader ?? configuredImageLoader; if (effectiveLoader) { - const resolvedQuality = typeof quality === "string" ? Number(quality) : (quality ?? 75); + const resolvedQuality = typeof quality === "string" ? Number(quality) : quality; const attributes = generateImgAttrs({ src, width: fill ? undefined : imgWidth, @@ -660,6 +685,13 @@ const Image = forwardRef(function Image( loader: effectiveLoader, }); const renderedSrc = overrideSrc || attributes.src; + // Omitting the placeholder here is what put this branch out of step with + // `getImageProps`, which returns the blur style for loader-generated URLs. + const sanitizedBlur = imgBlurDataURL ? sanitizeBlurDataURL(imgBlurDataURL) : undefined; + const blurStyle = + !blurComplete && placeholder === "blur" && sanitizedBlur + ? blurBackgroundStyle(sanitizedBlur) + : undefined; preloadImageResource({ shouldPreload, src: renderedSrc, @@ -683,7 +715,7 @@ const Image = forwardRef(function Image( data-nimg={fill ? "fill" : "1"} onLoad={handleLoad} onError={handleError} - style={fill ? getFillStyle(style) : style} + style={fill ? getFillStyle(style, blurStyle) : { ...blurStyle, ...style }} {...rest} /> ); @@ -707,14 +739,7 @@ const Image = forwardRef(function Image( const sanitizedBlur = imgBlurDataURL ? sanitizeBlurDataURL(imgBlurDataURL) : undefined; const showBlur = !blurComplete && placeholder === "blur" && sanitizedBlur; - const blurStyle = showBlur - ? { - backgroundImage: `url(${sanitizedBlur})`, - backgroundSize: "cover", - backgroundRepeat: "no-repeat", - backgroundPosition: "center", - } - : undefined; + const blurStyle = showBlur ? blurBackgroundStyle(sanitizedBlur) : undefined; const bg = showBlur ? `url(${sanitizedBlur})` : undefined; if (fill) { @@ -820,12 +845,7 @@ const Image = forwardRef(function Image( const sanitizedLocalBlur = imgBlurDataURL ? sanitizeBlurDataURL(imgBlurDataURL) : undefined; const blurStyle = !blurComplete && placeholder === "blur" && sanitizedLocalBlur - ? { - backgroundImage: `url(${sanitizedLocalBlur})`, - backgroundSize: "cover", - backgroundRepeat: "no-repeat", - backgroundPosition: "center", - } + ? blurBackgroundStyle(sanitizedLocalBlur) : undefined; const imageSizes = sizes ?? (fill ? "100vw" : undefined); @@ -897,19 +917,19 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { } = resolveImageSource({ src: srcProp, width, height, blurDataURL: blurDataURLProp }); const shouldPreload = _preload === true || priority === true; - if (_unoptimized === true || __globallyUnoptimized) { + // Mirrors the component path: a bare `images.loader: "custom"` is reported + // before optimization is decided, so `unoptimized` cannot hide it. + const reportsMissingLoaderProp = + loader === undefined && configuredImageLoader?.__vinext_img_missing_loader === true; + + if ((_unoptimized === true || __globallyUnoptimized) && !reportsMissingLoaderProp) { // As in the component path, unoptimized images never reach the server-side // optimizer, so remote URL validation is intentionally unnecessary. const renderedSrc = overrideSrc || src; const sanitizedBlurURL = imgBlurDataURL ? sanitizeBlurDataURL(imgBlurDataURL) : undefined; const blurStyle = placeholder === "blur" && sanitizedBlurURL - ? { - backgroundImage: `url(${sanitizedBlurURL})`, - backgroundSize: "cover", - backgroundRepeat: "no-repeat" as const, - backgroundPosition: "center" as const, - } + ? blurBackgroundStyle(sanitizedBlurURL) : undefined; const imageProps: ImgProps = { src: renderedSrc, @@ -950,7 +970,7 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { // the loader runs once per candidate width so `getImageProps` hands back the // same srcSet `` would render — callers building elements // depend on the two agreeing. - const imgQuality = typeof _quality === "string" ? Number(_quality) : (_quality ?? 75); + const imgQuality = typeof _quality === "string" ? Number(_quality) : _quality; const loaderAttributes = effectiveLoader ? generateImgAttrs({ src, @@ -989,14 +1009,7 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { // Blur placeholder styles — sanitize to prevent CSS injection const sanitizedBlurURL = imgBlurDataURL ? sanitizeBlurDataURL(imgBlurDataURL) : undefined; const blurStyle = - placeholder === "blur" && sanitizedBlurURL - ? { - backgroundImage: `url(${sanitizedBlurURL})`, - backgroundSize: "cover", - backgroundRepeat: "no-repeat" as const, - backgroundPosition: "center" as const, - } - : undefined; + placeholder === "blur" && sanitizedBlurURL ? blurBackgroundStyle(sanitizedBlurURL) : undefined; const imageProps: ImgProps = { // Match Next.js: overrideSrc changes the fallback `src` without changing diff --git a/packages/vinext/src/virtual-vinext-image-loader.d.ts b/packages/vinext/src/virtual-vinext-image-loader.d.ts index 403bfe155c..909563ca88 100644 --- a/packages/vinext/src/virtual-vinext-image-loader.d.ts +++ b/packages/vinext/src/virtual-vinext-image-loader.d.ts @@ -13,7 +13,15 @@ */ declare module "virtual:vinext-image-loader" { const imageLoader: - | ((props: { src: string; width: number; quality?: number }) => string) + | (((props: { src: string; width: number; quality?: number }) => string) & { + /** + * Set when `images.loader` is `"custom"` with no `images.loaderFile`: + * the export reports that each `` must pass its own `loader` + * prop rather than generating URLs. See `MISSING_CUSTOM_LOADER_MARKER` + * in `image/image-loader-virtual.ts`. + */ + __vinext_img_missing_loader?: true; + }) | undefined; export default imageLoader; } diff --git a/tests/image-component.test.ts b/tests/image-component.test.ts index a801dfc679..4b6958bc1f 100644 --- a/tests/image-component.test.ts +++ b/tests/image-component.test.ts @@ -991,6 +991,82 @@ describe("custom loader URL ownership", () => { expect(props.src).toBe(overrideSrc); expect(props.srcSet).toContain("https://cdn.example.com/optimized.jpg"); }); + + it("leaves an omitted quality undefined instead of defaulting it to 75", () => { + // Catches the loader written as `quality ?? 80`, or one picking an + // automatic CDN quality: a forced 75 makes it silently generate a + // different URL under vinext than under Next.js. + const seen: (number | undefined)[] = []; + const loader = ({ src, width, quality }: Parameters[0]) => { + seen.push(quality); + return `https://cdn.example.com${src}?w=${width}`; + }; + + ReactDOMServer.renderToString( + React.createElement(Image, { + alt: "no quality", + src: "/photo.jpg", + width: 300, + height: 200, + loader, + }), + ); + getImageProps({ alt: "no quality", src: "/photo.jpg", width: 300, height: 200, loader }); + + expect(seen.length).toBeGreaterThan(0); + expect(seen.every((quality) => quality === undefined)).toBe(true); + }); + + it("forwards an explicit quality unchanged", () => { + const seen: (number | undefined)[] = []; + const loader = ({ src, width, quality }: Parameters[0]) => { + seen.push(quality); + return `https://cdn.example.com${src}?w=${width}`; + }; + + getImageProps({ alt: "q", src: "/photo.jpg", width: 300, height: 200, quality: 40, loader }); + + expect(seen.every((quality) => quality === 40)).toBe(true); + }); + + it("keeps the blur placeholder, matching getImageProps", () => { + const blurDataURL = "data:image/png;base64,abc123"; + const loader = ({ src, width }: Parameters[0]) => + `https://cdn.example.com${src}?w=${width}`; + const props = { + alt: "blurred", + src: "/photo.jpg", + width: 300, + height: 200, + placeholder: "blur" as const, + blurDataURL, + loader, + }; + + const html = ReactDOMServer.renderToString(React.createElement(Image, props)); + expect(html).toContain(`url(${blurDataURL})`); + expect(getImageProps(props).props.style).toMatchObject({ + backgroundImage: `url(${blurDataURL})`, + }); + }); + + it("keeps the blur placeholder for fill images without dropping fill styles", () => { + const blurDataURL = "data:image/png;base64,abc123"; + const html = ReactDOMServer.renderToString( + React.createElement(Image, { + alt: "blurred fill", + src: "/photo.jpg", + fill: true, + placeholder: "blur", + blurDataURL, + loader: ({ src, width }: Parameters[0]) => + `https://cdn.example.com${src}?w=${width}`, + }), + ); + + expect(html).toContain(`url(${blurDataURL})`); + expect(html).toContain("position:absolute"); + }); }); // ─── Reproduction: priority prop on remote URL paths ──────────────────── diff --git a/tests/image-configured-loader.test.ts b/tests/image-configured-loader.test.ts new file mode 100644 index 0000000000..1958704b1a --- /dev/null +++ b/tests/image-configured-loader.test.ts @@ -0,0 +1,181 @@ +/** + * `next/image` behaviour when `virtual:vinext-image-loader` resolves to a + * *configured* loader. + * + * The shim imports that module statically, and the test alias points it at the + * unconfigured stand-in (see `WORKSPACE_SRC_ALIAS` in vite.config.ts), so every + * other image test exercises the built-in path. These tests swap the module per + * case with `vi.doMock` + `vi.resetModules()`, which is the only way to reach + * the `images.loaderFile` / `images.loader` branches from a unit test. + * + * `tests/image-loader-config.test.ts` covers what the plugin *generates*; this + * file covers what the shim *does* with it. + */ +import { describe, it, expect, vi, afterEach } from "vite-plus/test"; +import React from "react"; +import ReactDOMServer from "react-dom/server"; +import { MISSING_CUSTOM_LOADER_MARKER } from "../packages/vinext/src/image/image-loader-virtual.js"; + +type Shim = typeof import("../packages/vinext/src/shims/image.js"); + +/** Load a fresh copy of the shim bound to `configuredLoader`. */ +async function loadShim(configuredLoader: unknown): Promise { + vi.resetModules(); + vi.doMock("virtual:vinext-image-loader", () => ({ default: configuredLoader })); + return import("../packages/vinext/src/shims/image.js"); +} + +const MISSING_LOADER_MESSAGE = + 'Image with src "/photo.jpg" is missing "loader" prop.\n' + + "Read more: https://nextjs.org/docs/messages/next-image-missing-loader"; + +/** + * Stands in for the module the plugin generates for `images.loader: "custom"` + * with no `images.loaderFile`. Shape is pinned by + * `tests/image-loader-config.test.ts`, which asserts the generator emits both + * the throw and the marker. + */ +function bareCustomLoader(): unknown { + const report = ({ src }: { src: string }): string => { + throw new Error( + `Image with src "${src}" is missing "loader" prop.\n` + + "Read more: https://nextjs.org/docs/messages/next-image-missing-loader", + ); + }; + return Object.assign(report, { [MISSING_CUSTOM_LOADER_MARKER]: true }); +} + +afterEach(() => { + vi.doUnmock("virtual:vinext-image-loader"); + vi.resetModules(); +}); + +describe('images.loader: "custom" with no loaderFile', () => { + it("reports the missing loader prop when rendering", async () => { + const { default: Image } = await loadShim(bareCustomLoader()); + + expect(() => + ReactDOMServer.renderToString( + React.createElement(Image, { + alt: "no loader", + src: "/photo.jpg", + width: 300, + height: 200, + }), + ), + ).toThrow(MISSING_LOADER_MESSAGE); + }); + + it("reports it for unoptimized images too", async () => { + // Upstream raises this before it decides whether an image is optimized + // (`getImgProps` throws above `generateImgAttrs`, which is what handles + // `unoptimized`). Skipping it here would let `unoptimized` silently render + // the original URL from a config that cannot produce URLs at all. + const { default: Image, getImageProps } = await loadShim(bareCustomLoader()); + const props = { + alt: "no loader", + src: "/photo.jpg", + width: 300, + height: 200, + unoptimized: true, + }; + + expect(() => ReactDOMServer.renderToString(React.createElement(Image, props))).toThrow( + MISSING_LOADER_MESSAGE, + ); + expect(() => getImageProps(props)).toThrow(MISSING_LOADER_MESSAGE); + }); + + it("is satisfied by a per-image loader prop, including when unoptimized", async () => { + // The whole point of bare "custom": each brings its own loader. + const { default: Image, getImageProps } = await loadShim(bareCustomLoader()); + const loader = ({ src, width }: { src: string; width: number }) => + `https://cdn.example.com${src}?w=${width}`; + + const html = ReactDOMServer.renderToString( + React.createElement(Image, { + alt: "own loader", + src: "/photo.jpg", + width: 300, + height: 200, + loader, + }), + ); + expect(html).toContain("https://cdn.example.com/photo.jpg?w="); + + // An unoptimized image bypasses even a valid loader, matching upstream. + const { props } = getImageProps({ + alt: "own loader", + src: "/photo.jpg", + width: 300, + height: 200, + unoptimized: true, + loader, + }); + expect(props.src).toBe("/photo.jpg"); + expect(props.srcSet).toBeUndefined(); + }); +}); + +describe("images.loaderFile", () => { + it("replaces the built-in loader for local and remote sources alike", async () => { + const { default: Image, getImageProps } = await loadShim( + ({ src, width }: { src: string; width: number }) => + `https://cdn.example.com/i?u=${encodeURIComponent(src)}&w=${width}`, + ); + + for (const src of ["/photo.jpg", "https://origin.example.com/photo.jpg"]) { + const { props } = getImageProps({ alt: "configured", src, width: 300, height: 200 }); + expect(props.src).toContain("https://cdn.example.com/i?u="); + expect(props.srcSet).toContain("https://cdn.example.com/i?u="); + } + + const html = ReactDOMServer.renderToString( + React.createElement(Image, { + alt: "configured", + src: "/photo.jpg", + width: 300, + height: 200, + }), + ); + expect(html).not.toContain("/_next/image"); + }); + + it("keeps the blur placeholder", async () => { + // The loaderFile-specific half of the regression: an image that showed a + // placeholder under the built-in loader lost it purely by being configured. + const blurDataURL = "data:image/png;base64,abc123"; + const { default: Image } = await loadShim( + ({ src, width }: { src: string; width: number }) => + `https://cdn.example.com${src}?w=${width}`, + ); + + const html = ReactDOMServer.renderToString( + React.createElement(Image, { + alt: "configured blur", + src: "/photo.jpg", + width: 300, + height: 200, + placeholder: "blur", + blurDataURL, + }), + ); + + expect(html).toContain(`url(${blurDataURL})`); + }); + + it("is overridden by a per-image loader prop", async () => { + const { getImageProps } = await loadShim(() => "https://configured.example.com/x.jpg"); + + const { props } = getImageProps({ + alt: "prop wins", + src: "/photo.jpg", + width: 300, + height: 200, + loader: ({ width }: { src: string; width: number }) => + `https://prop.example.com/x.jpg?w=${width}`, + }); + + expect(props.src).toContain("https://prop.example.com/x.jpg"); + }); +}); diff --git a/tests/image-loader-config.test.ts b/tests/image-loader-config.test.ts index 3df1258bfe..ca4657ed1a 100644 --- a/tests/image-loader-config.test.ts +++ b/tests/image-loader-config.test.ts @@ -15,6 +15,7 @@ import { pathToFileURL } from "node:url"; import { describe, it, expect } from "vite-plus/test"; import { generateImageLoaderModule, + MISSING_CUSTOM_LOADER_MARKER, VIRTUAL_IMAGE_LOADER, } from "../packages/vinext/src/image/image-loader-virtual.js"; import { resolveNextConfig } from "../packages/vinext/src/config/next-config.js"; @@ -82,6 +83,18 @@ describe("generateImageLoaderModule", () => { expect(code).not.toContain("export default undefined;"); }); + it('marks the bare-"custom" export so the shim can tell it from a real loader', () => { + // The shim reads this to report the misconfiguration before it decides + // whether an image is optimized — an `unoptimized` image legitimately + // bypasses a real loader, but must not bypass this one. + const code = generateImageLoaderModule({ loader: "custom" }); + expect(code).toContain(`customImageLoader.${MISSING_CUSTOM_LOADER_MARKER} = true;`); + // Only the bare-"custom" stub carries it; a real loader must not. + expect(generateImageLoaderModule({ loaderFile: "/project/l.js" })).not.toContain( + MISSING_CUSTOM_LOADER_MARKER, + ); + }); + it('prefers the loaderFile when both it and loader:"custom" are set', () => { const code = generateImageLoaderModule({ loader: "custom", loaderFile: "/project/l.js" }); expect(code).toContain('from "/project/l.js"'); @@ -135,6 +148,20 @@ describe("generated virtual:vinext-image-loader module", () => { "Read more: https://nextjs.org/docs/messages/next-image-missing-loader", ); }); + + it('carries the marker on the executed bare-"custom" export', async () => { + // Asserted on the imported module rather than the source string: the + // property is assigned after `export default function`, and only running it + // proves the assignment lands on the exported binding. + const root = makeProjectRoot(); + const mod = (await importGenerated( + root, + "generated-custom-marker.mjs", + generateImageLoaderModule({ loader: "custom" }), + )) as { default: Record }; + + expect(mod.default[MISSING_CUSTOM_LOADER_MARKER]).toBe(true); + }); }); // ─── config resolution ───────────────────────────────────────────────────── @@ -171,6 +198,15 @@ describe("resolveNextConfig images.loaderFile", () => { ).rejects.toThrow(/cannot be used with images.loaderFile/); }); + it("rejects an unsupported named loader even without a loaderFile", async () => { + // The dangerous permutation: nothing downstream rejects a named loader on + // its own, so every image would quietly be served from `/_next/image` + // instead of the CDN the config names. + await expect( + resolveNextConfig({ images: { loader: "cloudinary" as "custom" } }), + ).rejects.toThrow(/images.loader property \(cloudinary\) is not supported/); + }); + it('allows loader:"custom" with no loaderFile', async () => { const config = await resolveNextConfig({ images: { loader: "custom" } }); expect(config.images?.loader).toBe("custom"); diff --git a/tests/shims.test.ts b/tests/shims.test.ts index 3ca80fb19d..83d982f3d0 100644 --- a/tests/shims.test.ts +++ b/tests/shims.test.ts @@ -21765,7 +21765,8 @@ describe("next/image enhancements", () => { alt: "Custom", width: 800, height: 600, - loader: ({ src, width, quality }) => `https://cdn.example.com${src}?w=${width}&q=${quality}`, + loader: ({ src, width, quality }) => + `https://cdn.example.com${src}?w=${width}&q=${quality ?? 75}`, }); // Custom loader bypasses the /_next/image endpoint. Per Next.js // `getWidths`, candidates are [800, 1600] snapped up to [828, 1920], and @@ -21925,7 +21926,7 @@ describe("next/image component rendering", () => { width: 800, height: 600, loader: ({ src, width, quality }: { src: string; width: number; quality?: number }) => - `https://cdn.example.com${src}?w=${width}&q=${quality}`, + `https://cdn.example.com${src}?w=${width}&q=${quality ?? 75}`, }), ); // `src` is the largest candidate width (800 -> 828, 1600 -> 1920). From c98d303b5fad412bcb3d7657da17ed4bfe134ce9 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Sat, 25 Jul 2026 17:12:11 +1000 Subject: [PATCH 4/8] test(image): drive configured-loader cases through the real plugin 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. --- tests/image-configured-loader.test.ts | 181 --------------------- tests/image-loader-plugin.test.ts | 224 +++++++++++++++++++++++++- 2 files changed, 217 insertions(+), 188 deletions(-) delete mode 100644 tests/image-configured-loader.test.ts diff --git a/tests/image-configured-loader.test.ts b/tests/image-configured-loader.test.ts deleted file mode 100644 index 1958704b1a..0000000000 --- a/tests/image-configured-loader.test.ts +++ /dev/null @@ -1,181 +0,0 @@ -/** - * `next/image` behaviour when `virtual:vinext-image-loader` resolves to a - * *configured* loader. - * - * The shim imports that module statically, and the test alias points it at the - * unconfigured stand-in (see `WORKSPACE_SRC_ALIAS` in vite.config.ts), so every - * other image test exercises the built-in path. These tests swap the module per - * case with `vi.doMock` + `vi.resetModules()`, which is the only way to reach - * the `images.loaderFile` / `images.loader` branches from a unit test. - * - * `tests/image-loader-config.test.ts` covers what the plugin *generates*; this - * file covers what the shim *does* with it. - */ -import { describe, it, expect, vi, afterEach } from "vite-plus/test"; -import React from "react"; -import ReactDOMServer from "react-dom/server"; -import { MISSING_CUSTOM_LOADER_MARKER } from "../packages/vinext/src/image/image-loader-virtual.js"; - -type Shim = typeof import("../packages/vinext/src/shims/image.js"); - -/** Load a fresh copy of the shim bound to `configuredLoader`. */ -async function loadShim(configuredLoader: unknown): Promise { - vi.resetModules(); - vi.doMock("virtual:vinext-image-loader", () => ({ default: configuredLoader })); - return import("../packages/vinext/src/shims/image.js"); -} - -const MISSING_LOADER_MESSAGE = - 'Image with src "/photo.jpg" is missing "loader" prop.\n' + - "Read more: https://nextjs.org/docs/messages/next-image-missing-loader"; - -/** - * Stands in for the module the plugin generates for `images.loader: "custom"` - * with no `images.loaderFile`. Shape is pinned by - * `tests/image-loader-config.test.ts`, which asserts the generator emits both - * the throw and the marker. - */ -function bareCustomLoader(): unknown { - const report = ({ src }: { src: string }): string => { - throw new Error( - `Image with src "${src}" is missing "loader" prop.\n` + - "Read more: https://nextjs.org/docs/messages/next-image-missing-loader", - ); - }; - return Object.assign(report, { [MISSING_CUSTOM_LOADER_MARKER]: true }); -} - -afterEach(() => { - vi.doUnmock("virtual:vinext-image-loader"); - vi.resetModules(); -}); - -describe('images.loader: "custom" with no loaderFile', () => { - it("reports the missing loader prop when rendering", async () => { - const { default: Image } = await loadShim(bareCustomLoader()); - - expect(() => - ReactDOMServer.renderToString( - React.createElement(Image, { - alt: "no loader", - src: "/photo.jpg", - width: 300, - height: 200, - }), - ), - ).toThrow(MISSING_LOADER_MESSAGE); - }); - - it("reports it for unoptimized images too", async () => { - // Upstream raises this before it decides whether an image is optimized - // (`getImgProps` throws above `generateImgAttrs`, which is what handles - // `unoptimized`). Skipping it here would let `unoptimized` silently render - // the original URL from a config that cannot produce URLs at all. - const { default: Image, getImageProps } = await loadShim(bareCustomLoader()); - const props = { - alt: "no loader", - src: "/photo.jpg", - width: 300, - height: 200, - unoptimized: true, - }; - - expect(() => ReactDOMServer.renderToString(React.createElement(Image, props))).toThrow( - MISSING_LOADER_MESSAGE, - ); - expect(() => getImageProps(props)).toThrow(MISSING_LOADER_MESSAGE); - }); - - it("is satisfied by a per-image loader prop, including when unoptimized", async () => { - // The whole point of bare "custom": each brings its own loader. - const { default: Image, getImageProps } = await loadShim(bareCustomLoader()); - const loader = ({ src, width }: { src: string; width: number }) => - `https://cdn.example.com${src}?w=${width}`; - - const html = ReactDOMServer.renderToString( - React.createElement(Image, { - alt: "own loader", - src: "/photo.jpg", - width: 300, - height: 200, - loader, - }), - ); - expect(html).toContain("https://cdn.example.com/photo.jpg?w="); - - // An unoptimized image bypasses even a valid loader, matching upstream. - const { props } = getImageProps({ - alt: "own loader", - src: "/photo.jpg", - width: 300, - height: 200, - unoptimized: true, - loader, - }); - expect(props.src).toBe("/photo.jpg"); - expect(props.srcSet).toBeUndefined(); - }); -}); - -describe("images.loaderFile", () => { - it("replaces the built-in loader for local and remote sources alike", async () => { - const { default: Image, getImageProps } = await loadShim( - ({ src, width }: { src: string; width: number }) => - `https://cdn.example.com/i?u=${encodeURIComponent(src)}&w=${width}`, - ); - - for (const src of ["/photo.jpg", "https://origin.example.com/photo.jpg"]) { - const { props } = getImageProps({ alt: "configured", src, width: 300, height: 200 }); - expect(props.src).toContain("https://cdn.example.com/i?u="); - expect(props.srcSet).toContain("https://cdn.example.com/i?u="); - } - - const html = ReactDOMServer.renderToString( - React.createElement(Image, { - alt: "configured", - src: "/photo.jpg", - width: 300, - height: 200, - }), - ); - expect(html).not.toContain("/_next/image"); - }); - - it("keeps the blur placeholder", async () => { - // The loaderFile-specific half of the regression: an image that showed a - // placeholder under the built-in loader lost it purely by being configured. - const blurDataURL = "data:image/png;base64,abc123"; - const { default: Image } = await loadShim( - ({ src, width }: { src: string; width: number }) => - `https://cdn.example.com${src}?w=${width}`, - ); - - const html = ReactDOMServer.renderToString( - React.createElement(Image, { - alt: "configured blur", - src: "/photo.jpg", - width: 300, - height: 200, - placeholder: "blur", - blurDataURL, - }), - ); - - expect(html).toContain(`url(${blurDataURL})`); - }); - - it("is overridden by a per-image loader prop", async () => { - const { getImageProps } = await loadShim(() => "https://configured.example.com/x.jpg"); - - const { props } = getImageProps({ - alt: "prop wins", - src: "/photo.jpg", - width: 300, - height: 200, - loader: ({ width }: { src: string; width: number }) => - `https://prop.example.com/x.jpg?w=${width}`, - }); - - expect(props.src).toContain("https://prop.example.com/x.jpg"); - }); -}); diff --git a/tests/image-loader-plugin.test.ts b/tests/image-loader-plugin.test.ts index 4db72cbd51..c71f50db85 100644 --- a/tests/image-loader-plugin.test.ts +++ b/tests/image-loader-plugin.test.ts @@ -2,36 +2,114 @@ * End-to-end plugin wiring for `images.loaderFile`. * * The codegen and config-resolution units are covered in - * `image-loader-config.test.ts`. This file checks the part those cannot: that - * the vinext plugin actually resolves and loads `virtual:vinext-image-loader` - * from a real next.config, which is the step that was missing entirely and made - * the option look unsupported. + * `image-loader-config.test.ts`. This file checks the parts those cannot: + * + * - that the vinext plugin actually resolves and loads + * `virtual:vinext-image-loader` from a real next.config, which is the step + * that was missing entirely and made the option look unsupported; + * - what the `next/image` shim then *does* with a configured loader. + * + * The second group has to run here rather than as a unit test. The shim imports + * the virtual module statically and unit tests alias it to the unconfigured + * stand-in (`image/image-loader-unconfigured.ts`), so the configured branches + * are only reachable by letting the plugin generate the module for real. */ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { describe, it, expect } from "vite-plus/test"; +import { describe, it, expect, beforeAll, afterAll } from "vite-plus/test"; import { createServer } from "vite"; +import type { ViteDevServer } from "vite-plus"; import vinext from "../packages/vinext/src/index.js"; +import { startFixtureServer, fetchHtml } from "./helpers.js"; + +/** Root layout — the app router refuses to render a page without one. */ +const ROOT_LAYOUT = `export default function RootLayout({ children }) { + return ( + + {children} + + ); +} +`; + +/** + * Renders one `
` per `getImageProps` case, so each outcome — including a
+ * thrown misconfiguration — arrives as text in the SSR HTML.
+ *
+ * It is a *client* page for two reasons. `next/image` is a `"use client"`
+ * module, so its exports are client references in the RSC environment and
+ * calling `getImageProps` from a page or route handler fails outright. And the
+ * `loader` prop is a function, which cannot cross the server/client boundary —
+ * declared here, it never has to.
+ */
+const PROBE_PAGE = `"use client";
+
+import { getImageProps } from "next/image";
+
+const loaderProp = ({ src, width }) => \`https://prop.example.com\${src}?w=\${width}\`;
+
+function probe(id, props) {
+  let text;
+  try {
+    const { props: imgProps } = getImageProps(props);
+    text = \`OK \${imgProps.src} | \${imgProps.srcSet ?? "none"}\`;
+  } catch (error) {
+    text = \`ERROR \${error.message}\`;
+  }
+  return 
{text}
; +} + +export default function Page() { + const base = { alt: "probe", src: "/photo.jpg", width: 300, height: 200 }; + return ( +
+ {probe("plain", base)} + {probe("unoptimized", { ...base, unoptimized: true })} + {probe("loader-prop", { ...base, loader: loaderProp })} + {probe("loader-prop-unoptimized", { ...base, loader: loaderProp, unoptimized: true })} +
+ ); +} +`; + +/** Read one probe's rendered text back out of the SSR HTML. */ +function probeText(html: string, id: string): string { + const match = new RegExp(`
]*>([\\s\\S]*?)
`).exec(html); + if (!match) throw new Error(`probe "${id}" not found in rendered HTML`); + return match[1] + .replace(//g, "") + .replace(/"/g, '"') + .replace(/'/g, "'") + .replace(/&/g, "&"); +} /** * Build a throwaway project root with an app dir, a loader file, and the given * next.config body. `node_modules` is symlinked so plugin-internal resolution * behaves as it would in a real project. */ -function createProject(nextConfigBody: string): string { +function createProject(nextConfigBody: string, extraFiles: Record = {}): string { const root = fs.mkdtempSync(path.join(os.tmpdir(), "vinext-loaderfile-plugin-")); fs.mkdirSync(path.join(root, "app"), { recursive: true }); fs.writeFileSync( path.join(root, "app", "page.tsx"), "export default function Page() { return null; }\n", ); + // `quality ?? "auto"` rather than `?? 75`: it makes an omitted quality + // visible in the URL, so a forced default at the loader seam cannot hide + // behind producing the same 75 the built-in optimizer would have used. fs.writeFileSync( path.join(root, "my-loader.js"), "export default ({ src, width, quality }) =>\n" + - " `https://images.example.com${src}?width=${width}&quality=${quality ?? 75}`;\n", + ' `https://images.example.com${src}?width=${width}&quality=${quality ?? "auto"}`;\n', ); fs.writeFileSync(path.join(root, "next.config.mjs"), nextConfigBody); + for (const [relativePath, contents] of Object.entries(extraFiles)) { + const target = path.join(root, relativePath); + fs.mkdirSync(path.dirname(target), { recursive: true }); + fs.writeFileSync(target, contents); + } fs.symlinkSync( path.resolve(import.meta.dirname, "..", "node_modules"), path.join(root, "node_modules"), @@ -79,3 +157,135 @@ describe("virtual:vinext-image-loader plugin wiring", () => { expect(code).toContain("export default undefined;"); }); }); + +// ─── the shim, driven by a real generated loader ─────────────────────────── + +describe("next/image with images.loaderFile configured", () => { + let server: ViteDevServer; + let baseUrl: string; + + beforeAll(async () => { + const root = createProject( + "export default { images: { loader: 'custom', loaderFile: './my-loader.js' } };\n", + { + "app/layout.tsx": ROOT_LAYOUT, + "app/probe/page.tsx": PROBE_PAGE, + // `blurDataURL` is required alongside placeholder="blur"; the value is + // opaque to the shim beyond its sanitization check. + "app/images/page.tsx": `import Image from "next/image"; + +export default function Page() { + return ( +
+ local + remote + blurred +
+ ); +} +`, + }, + ); + ({ server, baseUrl } = await startFixtureServer(root)); + await fetch(`${baseUrl}/images`).catch(() => {}); + }, 60_000); + + afterAll(async () => { + await server?.close(); + }); + + it("replaces the built-in loader for local and remote sources alike", async () => { + const { html } = await fetchHtml(baseUrl, "/images"); + + expect(html).toContain("https://images.example.com/photo.jpg?width="); + // The configured loader owns remote sources too, matching upstream, where + // loaderFile replaces the default loader for every image. No separator + // before the remote src: my-loader.js concatenates, and a remote source has + // no leading slash — the loader owns the URL, warts and all. + expect(html).toContain("https://images.example.comhttps://origin.example.com/photo.jpg"); + expect(html).not.toContain("/_next/image"); + }); + + it("keeps the blur placeholder", async () => { + // The regression this guards: selecting a configured loader dropped into a + // branch that ignored placeholder="blur", so an image that showed a + // placeholder under the built-in loader silently lost it. + const { html } = await fetchHtml(baseUrl, "/images"); + + expect(html).toContain("data:image/png;base64,abc123"); + expect(html).toContain("background-image"); + }); + + it("leaves an omitted quality to the loader instead of forcing 75", async () => { + // A configured loader must be able to tell "no quality requested" from + // "quality 75", which is what lets it pick an automatic CDN quality. + // `&` because the query separator is HTML-escaped in the attribute. + const { html } = await fetchHtml(baseUrl, "/images"); + + expect(html).toContain("&quality=auto"); + expect(html).not.toContain("&quality=75"); + }); + + it("is overridden by a per-image loader prop", async () => { + const { html } = await fetchHtml(baseUrl, "/probe"); + + expect(probeText(html, "loader-prop")).toContain("OK https://prop.example.com/photo.jpg"); + }); +}); + +describe('next/image with images.loader "custom" and no loaderFile', () => { + let server: ViteDevServer; + let baseUrl: string; + + beforeAll(async () => { + const root = createProject("export default { images: { loader: 'custom' } };\n", { + "app/layout.tsx": ROOT_LAYOUT, + "app/probe/page.tsx": PROBE_PAGE, + }); + ({ server, baseUrl } = await startFixtureServer(root)); + await fetch(`${baseUrl}/probe`).catch(() => {}); + }, 60_000); + + afterAll(async () => { + await server?.close(); + }); + + it("reports the missing loader prop", async () => { + const { html } = await fetchHtml(baseUrl, "/probe"); + + expect(probeText(html, "plain")).toContain( + 'ERROR Image with src "/photo.jpg" is missing "loader" prop.', + ); + }); + + it("reports it for unoptimized images too", async () => { + // Upstream raises this before it decides whether an image is optimized + // (`getImgProps` throws above `generateImgAttrs`, which is what handles + // `unoptimized`). Skipping it would let `unoptimized` silently render the + // original URL from a config that cannot produce URLs at all. + const { html } = await fetchHtml(baseUrl, "/probe"); + + expect(probeText(html, "unoptimized")).toContain('is missing "loader" prop.'); + }); + + it("is satisfied by a per-image loader prop", async () => { + const { html } = await fetchHtml(baseUrl, "/probe"); + + expect(probeText(html, "loader-prop")).toContain("OK https://prop.example.com/photo.jpg"); + }); + + it("still bypasses a valid loader prop for unoptimized images", async () => { + // The unoptimized shortcut is withheld only from the misconfiguration + // report — a working loader is still skipped, as upstream does. + const { html } = await fetchHtml(baseUrl, "/probe"); + + expect(probeText(html, "loader-prop-unoptimized")).toBe("OK /photo.jpg | none"); + }); +}); From ff11d0b76cadde29f5f42e2a5c5eb1711c605f08 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Mon, 27 Jul 2026 16:23:07 +1000 Subject: [PATCH 5/8] refactor(image): expose loader state as a module export 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. --- .../src/image/image-loader-unconfigured.ts | 5 +- .../vinext/src/image/image-loader-virtual.ts | 27 ++++------ packages/vinext/src/shims/image.tsx | 8 ++- .../src/virtual-vinext-image-loader.d.ts | 17 +++---- tests/image-loader-config.test.ts | 49 ++++++++++--------- 5 files changed, 48 insertions(+), 58 deletions(-) diff --git a/packages/vinext/src/image/image-loader-unconfigured.ts b/packages/vinext/src/image/image-loader-unconfigured.ts index 76887035d3..89b49e2572 100644 --- a/packages/vinext/src/image/image-loader-unconfigured.ts +++ b/packages/vinext/src/image/image-loader-unconfigured.ts @@ -10,9 +10,8 @@ * `generateImageLoaderModule()` output rather than trying to swap this module. */ const imageLoader: - | (((props: { src: string; width: number; quality?: number }) => string) & { - __vinext_img_missing_loader?: true; - }) + | ((props: { src: string; width: number; quality?: number }) => string) | undefined = undefined; export default imageLoader; +export const requiresLoaderProp = false; diff --git a/packages/vinext/src/image/image-loader-virtual.ts b/packages/vinext/src/image/image-loader-virtual.ts index a741bb9618..cd824d2c6a 100644 --- a/packages/vinext/src/image/image-loader-virtual.ts +++ b/packages/vinext/src/image/image-loader-virtual.ts @@ -10,8 +10,10 @@ * shim imports this virtual module unconditionally and the plugin generates * either a re-export of the user's loader or an inert stub. * - * The generated module always has a default export, so the shim's unconditional - * import stays valid whether or not `loaderFile` is configured. + * Every branch emits the same two exports so the shim's unconditional import + * stays valid whether or not `loaderFile` is configured: `default` is the loader + * (or `undefined`), and `requiresLoaderProp` says whether the configuration + * demands that each `` bring its own `loader` prop. * * This mirrors the adapter pattern in `image/image-adapters-virtual.ts`. */ @@ -19,17 +21,6 @@ /** Public virtual module id imported by the `next/image` shim. */ export const VIRTUAL_IMAGE_LOADER = "virtual:vinext-image-loader"; -/** - * Property stamped on the generated loader when `images.loader` is `"custom"` - * with no `images.loaderFile`, so the shim can distinguish "the user configured - * a loader" from "the user configured that every image must bring its own". - * - * Mirrors upstream's `__next_img_default` marker on its built-in loader. The - * name is duplicated in `shims/image.tsx` and `virtual-vinext-image-loader.d.ts` - * because the generated module is a string and cannot import this constant. - */ -export const MISSING_CUSTOM_LOADER_MARKER = "__vinext_img_missing_loader"; - /** * Next.js's error for a `loaderFile` whose module has no default export. * Kept verbatim so existing troubleshooting docs and searches still apply. @@ -64,11 +55,11 @@ export function generateImageLoaderModule(images?: { " );", "}", "", - "// Marks this export as a misconfiguration report rather than a working", - "// loader. Upstream raises the missing-prop error before it decides whether", - "// an image is optimized, so the shim needs to tell the two apart: an", + "// The default export above reports a misconfiguration rather than", + "// generating URLs. Upstream raises the missing-prop error before it decides", + "// whether an image is optimized, so the shim needs to tell the two apart: an", "// `unoptimized` image bypasses a real loader, but must not bypass this one.", - `customImageLoader.${MISSING_CUSTOM_LOADER_MARKER} = true;`, + "export const requiresLoaderProp = true;", "", ].join("\n"); } @@ -79,6 +70,7 @@ export function generateImageLoaderModule(images?: { return [ "// vinext: no images.loaderFile configured — the built-in /_next/image loader is used.", "export default undefined;", + "export const requiresLoaderProp = false;", "", ].join("\n"); } @@ -98,6 +90,7 @@ export function generateImageLoaderModule(images?: { "}", "", "export default __vinextImageLoader;", + "export const requiresLoaderProp = false;", "", ].join("\n"); } diff --git a/packages/vinext/src/shims/image.tsx b/packages/vinext/src/shims/image.tsx index 3ef42f2e20..fe2aa94e2c 100644 --- a/packages/vinext/src/shims/image.tsx +++ b/packages/vinext/src/shims/image.tsx @@ -31,7 +31,7 @@ import { useMergedRef } from "./use-merged-ref.js"; // 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 from "virtual:vinext-image-loader"; +import configuredImageLoader, { requiresLoaderProp } from "virtual:vinext-image-loader"; export type { ImageLoader, StaticImageData, StaticRequire }; export type ImageLoaderProps = Parameters[0]; @@ -627,8 +627,7 @@ const Image = forwardRef(function Image( // raises that error before it decides whether an image is optimized, so // `unoptimized` must not skip past it — fall through to the loader branch, // where invoking the loader surfaces the error carrying this image's src. - const reportsMissingLoaderProp = - loader === undefined && configuredImageLoader?.__vinext_img_missing_loader === true; + const reportsMissingLoaderProp = loader === undefined && requiresLoaderProp; if ((_unoptimized === true || __globallyUnoptimized) && !reportsMissingLoaderProp) { // Unoptimized images are fetched directly by the browser, so intentionally @@ -919,8 +918,7 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { // Mirrors the component path: a bare `images.loader: "custom"` is reported // before optimization is decided, so `unoptimized` cannot hide it. - const reportsMissingLoaderProp = - loader === undefined && configuredImageLoader?.__vinext_img_missing_loader === true; + const reportsMissingLoaderProp = loader === undefined && requiresLoaderProp; if ((_unoptimized === true || __globallyUnoptimized) && !reportsMissingLoaderProp) { // As in the component path, unoptimized images never reach the server-side diff --git a/packages/vinext/src/virtual-vinext-image-loader.d.ts b/packages/vinext/src/virtual-vinext-image-loader.d.ts index 909563ca88..a1077897a7 100644 --- a/packages/vinext/src/virtual-vinext-image-loader.d.ts +++ b/packages/vinext/src/virtual-vinext-image-loader.d.ts @@ -13,15 +13,14 @@ */ declare module "virtual:vinext-image-loader" { const imageLoader: - | (((props: { src: string; width: number; quality?: number }) => string) & { - /** - * Set when `images.loader` is `"custom"` with no `images.loaderFile`: - * the export reports that each `` must pass its own `loader` - * prop rather than generating URLs. See `MISSING_CUSTOM_LOADER_MARKER` - * in `image/image-loader-virtual.ts`. - */ - __vinext_img_missing_loader?: true; - }) + | ((props: { src: string; width: number; quality?: number }) => string) | undefined; export default imageLoader; + + /** + * True when `images.loader` is `"custom"` with no `images.loaderFile`: the + * default export then reports that each `` must pass its own `loader` + * prop rather than generating URLs. + */ + export const requiresLoaderProp: boolean; } diff --git a/tests/image-loader-config.test.ts b/tests/image-loader-config.test.ts index ca4657ed1a..c0a6d20b84 100644 --- a/tests/image-loader-config.test.ts +++ b/tests/image-loader-config.test.ts @@ -15,7 +15,6 @@ import { pathToFileURL } from "node:url"; import { describe, it, expect } from "vite-plus/test"; import { generateImageLoaderModule, - MISSING_CUSTOM_LOADER_MARKER, VIRTUAL_IMAGE_LOADER, } from "../packages/vinext/src/image/image-loader-virtual.js"; import { resolveNextConfig } from "../packages/vinext/src/config/next-config.js"; @@ -83,18 +82,6 @@ describe("generateImageLoaderModule", () => { expect(code).not.toContain("export default undefined;"); }); - it('marks the bare-"custom" export so the shim can tell it from a real loader', () => { - // The shim reads this to report the misconfiguration before it decides - // whether an image is optimized — an `unoptimized` image legitimately - // bypasses a real loader, but must not bypass this one. - const code = generateImageLoaderModule({ loader: "custom" }); - expect(code).toContain(`customImageLoader.${MISSING_CUSTOM_LOADER_MARKER} = true;`); - // Only the bare-"custom" stub carries it; a real loader must not. - expect(generateImageLoaderModule({ loaderFile: "/project/l.js" })).not.toContain( - MISSING_CUSTOM_LOADER_MARKER, - ); - }); - it('prefers the loaderFile when both it and loader:"custom" are set', () => { const code = generateImageLoaderModule({ loader: "custom", loaderFile: "/project/l.js" }); expect(code).toContain('from "/project/l.js"'); @@ -149,18 +136,32 @@ describe("generated virtual:vinext-image-loader module", () => { ); }); - it('carries the marker on the executed bare-"custom" export', async () => { - // Asserted on the imported module rather than the source string: the - // property is assigned after `export default function`, and only running it - // proves the assignment lands on the exported binding. + it('exports requiresLoaderProp from every branch, set only for bare-"custom"', async () => { + // The shim imports this name unconditionally, so a branch that omitted it + // would fail to link rather than degrade — importing each variant is what + // proves all three satisfy the contract. Only the bare-"custom" stub sets + // it: the shim reports that misconfiguration before it decides whether an + // image is optimized, since `unoptimized` legitimately bypasses a real + // loader but must not bypass the error. const root = makeProjectRoot(); - const mod = (await importGenerated( - root, - "generated-custom-marker.mjs", - generateImageLoaderModule({ loader: "custom" }), - )) as { default: Record }; - - expect(mod.default[MISSING_CUSTOM_LOADER_MARKER]).toBe(true); + const variants = [ + { name: "requires-none.mjs", images: undefined, expected: false }, + { + name: "requires-file.mjs", + images: { loaderFile: path.join(root, "loader.mjs") }, + expected: false, + }, + { name: "requires-custom.mjs", images: { loader: "custom" as const }, expected: true }, + ]; + + for (const variant of variants) { + const mod = (await importGenerated( + root, + variant.name, + generateImageLoaderModule(variant.images), + )) as { requiresLoaderProp: boolean }; + expect(mod.requiresLoaderProp).toBe(variant.expected); + } }); }); From b4cd9582db387d247adfb2b4bb0f4e38eb846b11 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Mon, 27 Jul 2026 16:48:23 +1000 Subject: [PATCH 6/8] fix(image): bypass loaders for data:/blob: and order src after srcSet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/vinext/src/shims/image.tsx | 39 ++++++++++++++++++++++++----- tests/image-loader-plugin.test.ts | 21 ++++++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/packages/vinext/src/shims/image.tsx b/packages/vinext/src/shims/image.tsx index fe2aa94e2c..50625d7ebe 100644 --- a/packages/vinext/src/shims/image.tsx +++ b/packages/vinext/src/shims/image.tsx @@ -216,6 +216,21 @@ function isRemoteUrl(src: string): boolean { return src.startsWith("http://") || src.startsWith("https://") || src.startsWith("//"); } +/** + * `data:` and `blob:` sources carry their own bytes, so no loader can do + * anything useful with them — a custom loader would rewrite an inline URI into a + * CDN request for a path that does not exist. + * + * Upstream forces these to `unoptimized` (`get-img-props.ts:270`), which makes + * `generateImgAttrs` hand back the original `src` with no `srcSet`. Callers must + * apply this alongside the other `unoptimized` triggers, and after the + * missing-loader-prop check: upstream raises that error earlier still (`:184`), + * so a bare `images.loader: "custom"` is reported even for an inline source. + */ +function isInlineSrc(src: string): boolean { + return src.startsWith("data:") || src.startsWith("blob:"); +} + function isSvgUrl(src: string): boolean { try { return new URL(src, "http://vinext.local").pathname.toLowerCase().endsWith(".svg"); @@ -629,7 +644,10 @@ const Image = forwardRef(function Image( // where invoking the loader surfaces the error carrying this image's src. const reportsMissingLoaderProp = loader === undefined && requiresLoaderProp; - if ((_unoptimized === true || __globallyUnoptimized) && !reportsMissingLoaderProp) { + if ( + (_unoptimized === true || __globallyUnoptimized || isInlineSrc(src)) && + !reportsMissingLoaderProp + ) { // Unoptimized images are fetched directly by the browser, so intentionally // skip remote URL validation: there is no server-side optimizer fetch and // therefore no SSRF surface. This matches Next.js behavior. @@ -701,7 +719,6 @@ const Image = forwardRef(function Image( return ( {alt}(function Image( decoding="async" srcSet={attributes.srcSet} sizes={attributes.sizes} + // Set after `srcSet`: React writes attributes in prop order, and a + // `src` applied first makes the browser start fetching the fallback + // candidate before it can see the responsive set. This is the ordering + // `generateImgAttrs` returns and upstream spreads. + src={renderedSrc} className={className} data-nimg={fill ? "fill" : "1"} onLoad={handleLoad} @@ -920,7 +942,10 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { // before optimization is decided, so `unoptimized` cannot hide it. const reportsMissingLoaderProp = loader === undefined && requiresLoaderProp; - if ((_unoptimized === true || __globallyUnoptimized) && !reportsMissingLoaderProp) { + if ( + (_unoptimized === true || __globallyUnoptimized || isInlineSrc(src)) && + !reportsMissingLoaderProp + ) { // As in the component path, unoptimized images never reach the server-side // optimizer, so remote URL validation is intentionally unnecessary. const renderedSrc = overrideSrc || src; @@ -1010,9 +1035,6 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { placeholder === "blur" && sanitizedBlurURL ? blurBackgroundStyle(sanitizedBlurURL) : undefined; const imageProps: ImgProps = { - // Match Next.js: overrideSrc changes the fallback `src` without changing - // the loader-generated candidates in `srcSet`. - src: overrideSrc || optimizedSrc, alt, width: fill ? undefined : imgWidth, height: fill ? undefined : imgHeight, @@ -1021,6 +1043,11 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { decoding: "async" as const, srcSet, sizes: loaderAttributes?.sizes ?? sizes ?? (fill ? "100vw" : undefined), + // Keyed after `srcSet` so a caller spreading these onto an writes the + // attributes in the order the browser wants — see the component path. + // Match Next.js: overrideSrc changes the fallback `src` without changing + // the loader-generated candidates in `srcSet`. + src: overrideSrc || optimizedSrc, className, style: fill ? getFillStyle(style, blurStyle) : { ...blurStyle, ...style }, ...rest, diff --git a/tests/image-loader-plugin.test.ts b/tests/image-loader-plugin.test.ts index c71f50db85..1e446b69cc 100644 --- a/tests/image-loader-plugin.test.ts +++ b/tests/image-loader-plugin.test.ts @@ -49,6 +49,10 @@ import { getImageProps } from "next/image"; const loaderProp = ({ src, width }) => \`https://prop.example.com\${src}?w=\${width}\`; +// A 1x1 GIF. Inline bytes: no loader can turn this into a fetchable CDN URL. +const DATA_URI = + "data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7"; + function probe(id, props) { let text; try { @@ -68,6 +72,8 @@ export default function Page() { {probe("unoptimized", { ...base, unoptimized: true })} {probe("loader-prop", { ...base, loader: loaderProp })} {probe("loader-prop-unoptimized", { ...base, loader: loaderProp, unoptimized: true })} + {probe("data-uri", { ...base, src: DATA_URI })} + {probe("blob-uri", { ...base, src: "blob:https://example.com/abc-123" })} ); } @@ -238,6 +244,21 @@ export default function Page() { expect(probeText(html, "loader-prop")).toContain("OK https://prop.example.com/photo.jpg"); }); + + it("leaves data: and blob: sources untouched", async () => { + // Upstream forces these to `unoptimized` before any loader runs + // (`get-img-props.ts:270`). Without that guard a configured loaderFile + // rewrites the inline bytes into `https://images.example.com/data:image/...` + // — a CDN request for a path that cannot exist — and emits a srcSet of + // them. Asserting the exact string, not just the absence of the CDN host, + // so a future change that merely mangles the URI differently still fails. + const { html } = await fetchHtml(baseUrl, "/probe"); + + expect(probeText(html, "data-uri")).toBe( + "OK data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7 | none", + ); + expect(probeText(html, "blob-uri")).toBe("OK blob:https://example.com/abc-123 | none"); + }); }); describe('next/image with images.loader "custom" and no loaderFile', () => { From 741fb3ec30d0507182449a51088b0add2a8d6fe2 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:05:15 +1000 Subject: [PATCH 7/8] fix(image): merge the blur placeholder after the caller's style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `` 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. --- packages/vinext/src/shims/image.tsx | 18 ++++++++++++------ tests/image-component.test.ts | 24 ++++++++++++++++++++++++ tests/image-loader-config.test.ts | 7 ++++++- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/packages/vinext/src/shims/image.tsx b/packages/vinext/src/shims/image.tsx index 50625d7ebe..fb79a55142 100644 --- a/packages/vinext/src/shims/image.tsx +++ b/packages/vinext/src/shims/image.tsx @@ -249,8 +249,8 @@ function getFillStyle( width: "100%", height: "100%", objectFit: "cover", - ...backgroundStyle, ...style, + ...backgroundStyle, }; } @@ -261,6 +261,12 @@ function getFillStyle( * (`get-img-props.ts` merges `placeholderStyle` last, for every path), so the * caller owns the "should it be showing" decision and this owns only the shape. * + * Callers must spread it *after* the user's `style`. It wins on purpose: a + * caller that sets `backgroundImage` for its own reasons would otherwise + * silently suppress the placeholder for the few hundred ms it exists, and the + * user style takes over again once `blurComplete` drops the placeholder + * entirely. Upstream merges in the same order (`get-img-props.ts:574-577`). + * * @param sanitizedBlurDataURL Must already have passed `sanitizeBlurDataURL` — * it is interpolated into a CSS `url()` and would otherwise allow injection. */ @@ -676,7 +682,7 @@ const Image = forwardRef(function Image( data-nimg={fill ? "fill" : "1"} onLoad={handleLoad} onError={handleError} - style={fill ? getFillStyle(style, blurStyle) : { ...blurStyle, ...style }} + style={fill ? getFillStyle(style, blurStyle) : { ...style, ...blurStyle }} {...rest} /> ); @@ -736,7 +742,7 @@ const Image = forwardRef(function Image( data-nimg={fill ? "fill" : "1"} onLoad={handleLoad} onError={handleError} - style={fill ? getFillStyle(style, blurStyle) : { ...blurStyle, ...style }} + style={fill ? getFillStyle(style, blurStyle) : { ...style, ...blurStyle }} {...rest} /> ); @@ -896,7 +902,7 @@ const Image = forwardRef(function Image( data-nimg={fill ? "fill" : "1"} onLoad={handleLoad} onError={handleError} - style={fill ? getFillStyle(style, blurStyle) : { ...blurStyle, ...style }} + style={fill ? getFillStyle(style, blurStyle) : { ...style, ...blurStyle }} {...rest} /> ); @@ -963,7 +969,7 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { fetchPriority: priority ? ("high" as const) : undefined, decoding: "async" as const, className, - style: fill ? getFillStyle(style, blurStyle) : { ...blurStyle, ...style }, + style: fill ? getFillStyle(style, blurStyle) : { ...style, ...blurStyle }, ...rest, sizes: sizes ?? (fill ? "100vw" : undefined), srcSet: undefined, @@ -1049,7 +1055,7 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { // the loader-generated candidates in `srcSet`. src: overrideSrc || optimizedSrc, className, - style: fill ? getFillStyle(style, blurStyle) : { ...blurStyle, ...style }, + style: fill ? getFillStyle(style, blurStyle) : { ...style, ...blurStyle }, ...rest, }; return { props: Object.assign(imageProps, { "data-nimg": fill ? "fill" : "1" }) }; diff --git a/tests/image-component.test.ts b/tests/image-component.test.ts index 4b6958bc1f..d2aa3351be 100644 --- a/tests/image-component.test.ts +++ b/tests/image-component.test.ts @@ -203,6 +203,30 @@ describe("Image SSR rendering", () => { expect(html).toContain("background-size:cover"); }); + it("keeps the blur placeholder above a caller background style", () => { + // Upstream merges `placeholderStyle` after `imgStyle` — which already + // contains the caller's `style` — so the placeholder wins while it is + // showing (`get-img-props.ts:574-577`). Merged the other way the + // placeholder is present in the style object but invisible, which is the + // failure this pins: assert the blur URL wins, not merely that it appears. + const blurDataURL = "data:image/png;base64,abc123"; + for (const fill of [false, true]) { + const html = ReactDOMServer.renderToString( + React.createElement(Image, { + alt: "blurry", + src: "/photo.jpg", + ...(fill ? { fill: true } : { width: 400, height: 300 }), + placeholder: "blur", + blurDataURL, + style: { backgroundImage: "url(/caller.png)", backgroundSize: "contain" }, + }), + ); + expect(html, `fill=${fill}`).toContain(`background-image:url(${blurDataURL})`); + expect(html, `fill=${fill}`).not.toContain("url(/caller.png)"); + expect(html, `fill=${fill}`).toContain("background-size:cover"); + } + }); + it("renders with custom loader", () => { const loader = ({ src, width, quality }: { src: string; width: number; quality?: number }) => `https://cdn.example.com${src}?w=${width}&q=${quality || 75}`; diff --git a/tests/image-loader-config.test.ts b/tests/image-loader-config.test.ts index c0a6d20b84..44af663ecf 100644 --- a/tests/image-loader-config.test.ts +++ b/tests/image-loader-config.test.ts @@ -12,6 +12,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { pathToFileURL } from "node:url"; +import { toSlash } from "pathslash"; import { describe, it, expect } from "vite-plus/test"; import { generateImageLoaderModule, @@ -171,7 +172,11 @@ describe("resolveNextConfig images.loaderFile", () => { 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`); + // `resolveImageLoaderFile` returns a `toSlash`-normalized path, so the + // expectation has to be normalized too — `root` comes from `mkdtempSync` + // and carries native separators, which on Windows would otherwise compare a + // half-backslash path against a forward-slash one. + expect(config.images?.loaderFile).toBe(toSlash(path.join(root, "loader.mjs"))); }); it("leaves loaderFile undefined when unset", async () => { From f6321d844d895997078c1f2d9ca9236daab83b1d Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:24:55 +1000 Subject: [PATCH 8/8] refactor(image): collapse duplicated shim logic and drop redundant loader calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/vinext/src/config/next-config.ts | 14 +- .../src/image/image-loader-unconfigured.ts | 6 +- .../vinext/src/image/image-loader-virtual.ts | 6 +- packages/vinext/src/shims/image.tsx | 184 +++++++++--------- tests/image-loader-plugin.test.ts | 10 +- 5 files changed, 106 insertions(+), 114 deletions(-) diff --git a/packages/vinext/src/config/next-config.ts b/packages/vinext/src/config/next-config.ts index fc35ef52e0..c4d08f927e 100644 --- a/packages/vinext/src/config/next-config.ts +++ b/packages/vinext/src/config/next-config.ts @@ -1236,13 +1236,6 @@ export function createRscCompatibilityId( return randomUUID(); } -/** - * Converts a cache handler path to a filesystem path. - * ESM's import.meta.resolve() returns file:// URLs which break when concatenated - * with path operations like path.join or path.relative. - * @param filePath - Absolute path, relative path, or file:// URL (e.g. from import.meta.resolve) - * @returns A filesystem path suitable for path operations - */ /** * Validate `images.loader` and resolve `images.loaderFile` to an absolute path, * mirroring Next.js's checks in `server/config.ts`. @@ -1290,6 +1283,13 @@ function resolveImageLoaderFile( return absolutePath; } +/** + * Converts a cache handler path to a filesystem path. + * ESM's import.meta.resolve() returns file:// URLs which break when concatenated + * with path operations like path.join or path.relative. + * @param filePath - Absolute path, relative path, or file:// URL (e.g. from import.meta.resolve) + * @returns A filesystem path suitable for path operations + */ function resolveCacheHandlerPathToFilesystem(filePath: string): string { // toSlash: fileURLToPath and user-supplied require.resolve() results are // backslash-separated on Windows; normalize into slash space. diff --git a/packages/vinext/src/image/image-loader-unconfigured.ts b/packages/vinext/src/image/image-loader-unconfigured.ts index 89b49e2572..03a054fb1a 100644 --- a/packages/vinext/src/image/image-loader-unconfigured.ts +++ b/packages/vinext/src/image/image-loader-unconfigured.ts @@ -9,9 +9,5 @@ * Tests that need a configured loader should assert on * `generateImageLoaderModule()` output rather than trying to swap this module. */ -const imageLoader: - | ((props: { src: string; width: number; quality?: number }) => string) - | undefined = undefined; - -export default imageLoader; +export default undefined; export const requiresLoaderProp = false; diff --git a/packages/vinext/src/image/image-loader-virtual.ts b/packages/vinext/src/image/image-loader-virtual.ts index cd824d2c6a..fe5a9c1add 100644 --- a/packages/vinext/src/image/image-loader-virtual.ts +++ b/packages/vinext/src/image/image-loader-virtual.ts @@ -55,10 +55,8 @@ export function generateImageLoaderModule(images?: { " );", "}", "", - "// The default export above reports a misconfiguration rather than", - "// generating URLs. Upstream raises the missing-prop error before it decides", - "// whether an image is optimized, so the shim needs to tell the two apart: an", - "// `unoptimized` image bypasses a real loader, but must not bypass this one.", + "// Tells the shim the default export reports a misconfiguration rather than", + "// generating URLs — see image/image-loader-virtual.ts.", "export const requiresLoaderProp = true;", "", ].join("\n"); diff --git a/packages/vinext/src/shims/image.tsx b/packages/vinext/src/shims/image.tsx index fb79a55142..bdedba3187 100644 --- a/packages/vinext/src/shims/image.tsx +++ b/packages/vinext/src/shims/image.tsx @@ -255,27 +255,41 @@ function getFillStyle( } /** - * The blur placeholder background shown until the real image loads. + * The blur placeholder background shown until the real image loads, or + * `undefined` when there is nothing to show — either the caller decided it is + * not showing, or the URL failed sanitization. * - * Upstream builds this style independently of which loader produced the URLs - * (`get-img-props.ts` merges `placeholderStyle` last, for every path), so the - * caller owns the "should it be showing" decision and this owns only the shape. + * Upstream builds this independently of which loader produced the URLs + * (`get-img-props.ts` merges `placeholderStyle` last, for every path), so + * `show` stays the caller's decision: the component gates on client + * `blurComplete` state, `getImageProps` has no state to gate on. * - * Callers must spread it *after* the user's `style`. It wins on purpose: a + * `style` must be spread *after* the user's `style`. It wins on purpose: a * caller that sets `backgroundImage` for its own reasons would otherwise * silently suppress the placeholder for the few hundred ms it exists, and the * user style takes over again once `blurComplete` drops the placeholder * entirely. Upstream merges in the same order (`get-img-props.ts:574-577`). * - * @param sanitizedBlurDataURL Must already have passed `sanitizeBlurDataURL` — - * it is interpolated into a CSS `url()` and would otherwise allow injection. + * `url` is the sanitized value for callers that need it as a bare CSS `url()` + * argument rather than a style object. */ -function blurBackgroundStyle(sanitizedBlurDataURL: string): React.CSSProperties { +function blurPlaceholder( + blurDataURL: string | undefined, + show: boolean, +): { style: React.CSSProperties; url: string } | undefined { + if (!show || !blurDataURL) return undefined; + // Sanitized before interpolation into `url()`, which would otherwise allow + // CSS injection. + const url = sanitizeBlurDataURL(blurDataURL); + if (!url) return undefined; return { - backgroundImage: `url(${sanitizedBlurDataURL})`, - backgroundSize: "cover", - backgroundRepeat: "no-repeat", - backgroundPosition: "center", + style: { + backgroundImage: `url(${url})`, + backgroundSize: "cover", + backgroundRepeat: "no-repeat", + backgroundPosition: "center", + }, + url, }; } @@ -388,13 +402,16 @@ function getImageWidths(width: number): number[] { * image, it is an image whose width is decided by the viewport, so it gets the * full device-size ladder with `w` descriptors rather than a single fixed URL. */ +// Module scope so it is not rebuilt per render. `matchAll` clones the regex +// internally, so the shared `lastIndex` is never mutated. +const VIEWPORT_WIDTH_PATTERN = /(^|\s)(1?\d?\d)vw/g; + function getWidths( width: number | undefined, sizes: string | undefined, ): { widths: number[]; kind: "w" | "x" } { if (sizes) { - const viewportWidthPattern = /(^|\s)(1?\d?\d)vw/g; - const viewportPercentages = Array.from(sizes.matchAll(viewportWidthPattern), (match) => + const viewportPercentages = Array.from(sizes.matchAll(VIEWPORT_WIDTH_PATTERN), (match) => Number.parseInt(match[2], 10), ); if (viewportPercentages.length > 0) { @@ -443,21 +460,21 @@ function generateImgAttrs(input: { }): { src: string; srcSet: string; sizes: string | undefined } { const { src, width, quality, sizes, loader } = input; const { widths, kind } = getWidths(width, sizes); + // Materialized once: the fallback `src` is the widest candidate, which the + // srcSet already generated. Loaders are arbitrary user code — a signed-URL + // loader hashes per call — so invoking one N+1 times to rebuild a string we + // hold would be a per-render tax on every image. + const urls = widths.map((candidateWidth) => loader({ src, width: candidateWidth, quality })); return { // Width-descriptor srcSets are meaningless to the browser without `sizes`, // so upstream supplies `100vw` when the caller omitted it. sizes: !sizes && kind === "w" ? "100vw" : sizes, - srcSet: widths - .map( - (candidateWidth, index) => - `${loader({ src, width: candidateWidth, quality })} ${ - kind === "w" ? candidateWidth : index + 1 - }${kind}`, - ) + srcSet: urls + .map((url, index) => `${url} ${kind === "w" ? widths[index] : index + 1}${kind}`) .join(", "), // Keep `src` last: React applies attributes in order, and a `src` set // before its `srcSet` makes the browser start a throwaway request. - src: loader({ src, width: widths[widths.length - 1], quality }), + src: urls[urls.length - 1], }; } @@ -465,20 +482,25 @@ function generateImgAttrs(input: { const builtInImageLoader: ImageLoader = ({ src, width, quality }) => imageOptimizationUrl(src, width, quality); -function generateImageAttributes( +/** + * Whether an image bypasses every loader and is handed to the browser as-is. + * + * The `!requiresLoaderProp` term is the ordering upstream enforces: a bare + * `images.loader: "custom"` is reported before optimization is decided + * (`get-img-props.ts:184`, ahead of the `unoptimized` handling), so an + * `unoptimized` image legitimately bypasses a real loader but must not bypass + * the misconfiguration error. Shared by both paths so that ordering has one + * home rather than one per path. + */ +function bypassesLoaders( src: string, - width: number, - quality?: number, - sizes?: string, -): { src: string; srcSet: string } { - const { src: resolvedSrc, srcSet } = generateImgAttrs({ - src, - width, - quality, - sizes, - loader: builtInImageLoader, - }); - return { src: resolvedSrc, srcSet }; + unoptimized: boolean | undefined, + loader: ImageLoader | undefined, +): boolean { + const reportsMissingLoaderProp = loader === undefined && requiresLoaderProp; + return ( + (unoptimized === true || __globallyUnoptimized || isInlineSrc(src)) && !reportsMissingLoaderProp + ); } const Image = forwardRef(function Image( @@ -560,6 +582,9 @@ const Image = forwardRef(function Image( const [completedBlurSrc, setCompletedBlurSrc] = useState(undefined); const blurComplete = completedBlurSrc === src; + // Every render branch below asks the same question, so it is answered once. + // `getImageProps` has no equivalent: it cannot observe load completion. + const showBlurPlaceholder = !blurComplete && placeholder === "blur"; const markBlurComplete = () => { if (placeholder !== "blur") return; @@ -643,26 +668,15 @@ const Image = forwardRef(function Image( } : undefined; - // `images.loader: "custom"` with no `images.loaderFile` makes the generated - // loader a misconfiguration report rather than a working loader. Upstream - // raises that error before it decides whether an image is optimized, so - // `unoptimized` must not skip past it — fall through to the loader branch, - // where invoking the loader surfaces the error carrying this image's src. - const reportsMissingLoaderProp = loader === undefined && requiresLoaderProp; - - if ( - (_unoptimized === true || __globallyUnoptimized || isInlineSrc(src)) && - !reportsMissingLoaderProp - ) { + // Falls through to the loader branch when a bare `images.loader: "custom"` + // must be reported: invoking the loader there surfaces the error carrying + // this image's src. + if (bypassesLoaders(src, _unoptimized, loader)) { // Unoptimized images are fetched directly by the browser, so intentionally // skip remote URL validation: there is no server-side optimizer fetch and // therefore no SSRF surface. This matches Next.js behavior. const renderedSrc = overrideSrc || src; - const sanitizedBlur = imgBlurDataURL ? sanitizeBlurDataURL(imgBlurDataURL) : undefined; - const blurStyle = - !blurComplete && placeholder === "blur" && sanitizedBlur - ? blurBackgroundStyle(sanitizedBlur) - : undefined; + const blurStyle = blurPlaceholder(imgBlurDataURL, showBlurPlaceholder)?.style; preloadImageResource({ shouldPreload, src: renderedSrc, @@ -710,11 +724,7 @@ const Image = forwardRef(function Image( const renderedSrc = overrideSrc || attributes.src; // Omitting the placeholder here is what put this branch out of step with // `getImageProps`, which returns the blur style for loader-generated URLs. - const sanitizedBlur = imgBlurDataURL ? sanitizeBlurDataURL(imgBlurDataURL) : undefined; - const blurStyle = - !blurComplete && placeholder === "blur" && sanitizedBlur - ? blurBackgroundStyle(sanitizedBlur) - : undefined; + const blurStyle = blurPlaceholder(imgBlurDataURL, showBlurPlaceholder)?.style; preloadImageResource({ shouldPreload, src: renderedSrc, @@ -764,10 +774,9 @@ const Image = forwardRef(function Image( } } - const sanitizedBlur = imgBlurDataURL ? sanitizeBlurDataURL(imgBlurDataURL) : undefined; - const showBlur = !blurComplete && placeholder === "blur" && sanitizedBlur; - const blurStyle = showBlur ? blurBackgroundStyle(sanitizedBlur) : undefined; - const bg = showBlur ? `url(${sanitizedBlur})` : undefined; + const blur = blurPlaceholder(imgBlurDataURL, showBlurPlaceholder); + const blurStyle = blur?.style; + const bg = blur && `url(${blur.url})`; if (fill) { const imageSizes = sizes ?? "100vw"; @@ -849,7 +858,13 @@ const Image = forwardRef(function Image( // Each entry points to /_next/image with the appropriate width. const optimizedAttributes = imgWidth && !fill && !skipOptimization - ? generateImageAttributes(src, imgWidth, imgQuality, sizes) + ? generateImgAttrs({ + src, + width: imgWidth, + quality: imgQuality, + sizes, + loader: builtInImageLoader, + }) : undefined; const srcSet = optimizedAttributes ? optimizedAttributes.srcSet @@ -868,12 +883,7 @@ const Image = forwardRef(function Image( : imageOptimizationUrl(src, RESPONSIVE_WIDTHS[0], imgQuality); // Blur placeholder: show a low-quality background while the image loads. - // Sanitize blurDataURL to prevent CSS injection via crafted data URLs. - const sanitizedLocalBlur = imgBlurDataURL ? sanitizeBlurDataURL(imgBlurDataURL) : undefined; - const blurStyle = - !blurComplete && placeholder === "blur" && sanitizedLocalBlur - ? blurBackgroundStyle(sanitizedLocalBlur) - : undefined; + const blurStyle = blurPlaceholder(imgBlurDataURL, showBlurPlaceholder)?.style; const imageSizes = sizes ?? (fill ? "100vw" : undefined); preloadImageResource({ @@ -944,22 +954,11 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { } = resolveImageSource({ src: srcProp, width, height, blurDataURL: blurDataURLProp }); const shouldPreload = _preload === true || priority === true; - // Mirrors the component path: a bare `images.loader: "custom"` is reported - // before optimization is decided, so `unoptimized` cannot hide it. - const reportsMissingLoaderProp = loader === undefined && requiresLoaderProp; - - if ( - (_unoptimized === true || __globallyUnoptimized || isInlineSrc(src)) && - !reportsMissingLoaderProp - ) { + if (bypassesLoaders(src, _unoptimized, loader)) { // As in the component path, unoptimized images never reach the server-side // optimizer, so remote URL validation is intentionally unnecessary. const renderedSrc = overrideSrc || src; - const sanitizedBlurURL = imgBlurDataURL ? sanitizeBlurDataURL(imgBlurDataURL) : undefined; - const blurStyle = - placeholder === "blur" && sanitizedBlurURL - ? blurBackgroundStyle(sanitizedBlurURL) - : undefined; + const blurStyle = blurPlaceholder(imgBlurDataURL, placeholder === "blur")?.style; const imageProps: ImgProps = { src: renderedSrc, alt, @@ -1014,15 +1013,25 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { // For local images (no loader, not remote), route through optimization endpoint. // When `unoptimized` is true, bypass the endpoint entirely (Next.js compat). // SVG sources auto-skip unless dangerouslyAllowSVG is enabled. - const isSvg = isSvgUrl(resolvedSrc); + // + // Ordered so the cheap decisive checks short-circuit `isSvgUrl`, which parses + // a URL: with a configured `loaderFile` the loader term is always true, and + // parsing every image's src to feed a term that cannot change the result is + // the single most expensive thing this function would do. const skipOpt = - (isSvg && !__dangerouslyAllowSVG) || - blockedInProd || !!effectiveLoader || - isRemoteUrl(resolvedSrc); + blockedInProd || + isRemoteUrl(resolvedSrc) || + (!__dangerouslyAllowSVG && isSvgUrl(resolvedSrc)); const optimizedAttributes = imgWidth && !fill && !skipOpt - ? generateImageAttributes(resolvedSrc, imgWidth, imgQuality, sizes) + ? generateImgAttrs({ + src: resolvedSrc, + width: imgWidth, + quality: imgQuality, + sizes, + loader: builtInImageLoader, + }) : null; const optimizedSrc = skipOpt ? resolvedSrc @@ -1035,10 +1044,7 @@ export function getImageProps(props: ImageProps): { props: ImgProps } { // comes from the loader instead. const srcSet = optimizedAttributes?.srcSet ?? loaderAttributes?.srcSet; - // Blur placeholder styles — sanitize to prevent CSS injection - const sanitizedBlurURL = imgBlurDataURL ? sanitizeBlurDataURL(imgBlurDataURL) : undefined; - const blurStyle = - placeholder === "blur" && sanitizedBlurURL ? blurBackgroundStyle(sanitizedBlurURL) : undefined; + const blurStyle = blurPlaceholder(imgBlurDataURL, placeholder === "blur")?.style; const imageProps: ImgProps = { alt, diff --git a/tests/image-loader-plugin.test.ts b/tests/image-loader-plugin.test.ts index 1e446b69cc..f0ac9e50c7 100644 --- a/tests/image-loader-plugin.test.ts +++ b/tests/image-loader-plugin.test.ts @@ -18,9 +18,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { describe, it, expect, beforeAll, afterAll } from "vite-plus/test"; -import { createServer } from "vite"; import type { ViteDevServer } from "vite-plus"; -import vinext from "../packages/vinext/src/index.js"; import { startFixtureServer, fetchHtml } from "./helpers.js"; /** Root layout — the app router refuses to render a page without one. */ @@ -126,13 +124,7 @@ function createProject(nextConfigBody: string, extraFiles: Record { - const server = await createServer({ - root, - configFile: false, - plugins: [vinext({ appDir: root })], - server: { port: 0 }, - logLevel: "silent", - }); + const { server } = await startFixtureServer(root, { listen: false }); try { const resolved = await server.pluginContainer.resolveId("virtual:vinext-image-loader"); expect(resolved).toBeTruthy();