Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/vinext/src/check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ const CONFIG_SUPPORT: Record<string, { status: Status; detail?: string }> = {
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: {
Expand Down
62 changes: 62 additions & 0 deletions packages/vinext/src/config/next-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;" */
Expand Down Expand Up @@ -1222,6 +1236,53 @@ export function createRscCompatibilityId(
return randomUUID();
}

/**
* 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
* 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<NextConfig["images"]>,
root: string,
): string | 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") {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Support the valid named image loaders

When an existing Next.js app configures images.loader as imgix, cloudinary, or akamai without loaderFile, this branch now throws during config resolution and prevents the app from starting. These are current valid loader values—the repository's vendored Next 16.2.7 declaration in packages/types/next/upstream/dist/shared/lib/image-config.d.ts includes all three in VALID_LOADERS—so the claim that they exist only for next/legacy/image is incorrect. Implement their URL generation or preserve a nonfatal fallback rather than rejecting valid Next.js configuration.

AGENTS.md reference: AGENTS.md:L293-L297

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disagreeing on this one — Next.js rejects this configuration too, so the current behavior is parity, not a regression.

VALID_LOADERS does list all three, but that list gates both next/image and next/legacy/image. What decides the outcome is the config validation. Next 16.2.7, dist/server/config.js:540-541:

if (images.loader !== 'default' && images.loader !== 'custom' && images.path === imageConfigDefault.path) {
    throw new Error(
      `Specified images.loader property (${images.loader}) also requires images.path property to be assigned to a URL prefix.\n` +
      `See more info here: https://nextjs.org/docs/api-reference/next/legacy/image#loader-configuration`
    )
}

So images.loader: "imgix" with no loaderFile and no custom images.path throws during config resolution and prevents the app from starting — in Next.js, today. That is precisely the case this branch rejects. Note also where upstream's own error message points: the next/legacy/image loader-configuration docs.

The implementations back that up. In 16.2.7 the named loaders exist only in dist/client/legacy/image.js:

45: function imgixLoader({ config, src, width, quality })
58: function akamaiLoader({ config, src, width })
61: function cloudinaryLoader({ config, src, width, quality })

dist/shared/lib/image-loader.js — what modern next/image actually uses — contains only defaultLoader (:13, marked __next_img_default at :110). There is no imgix/cloudinary/akamai code path in modern next/image to implement or fall back to.

The one real difference: upstream permits a named loader when images.path is set to a custom prefix, because legacy image consumes it. vinext implements neither next/legacy/image nor custom images.path, so there is no configuration under which a named loader could work — which is why this rejects unconditionally and says so in the message.

A non-fatal fallback is specifically what the earlier round of this review asked to remove, and for a good reason: silently serving /_next/image when the config names a CDN is the failure mode that is hardest to notice in production.

Reverting would restore that. Leaving as-is unless there's a concrete case where Next.js starts an app in this configuration — happy to look at one.

throw new Error(
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}".`);
}
return absolutePath;
}

/**
* Converts a cache handler path to a filesystem path.
* ESM's import.meta.resolve() returns file:// URLs which break when concatenated
Expand Down Expand Up @@ -1876,6 +1937,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
? {
Expand Down
13 changes: 13 additions & 0 deletions packages/vinext/src/image/image-loader-unconfigured.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/**
* 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.
*/
export default undefined;
export const requiresLoaderProp = false;
94 changes: 94 additions & 0 deletions packages/vinext/src/image/image-loader-virtual.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/**
* 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.
*
* 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 `<Image>` bring its own `loader` prop.
*
* 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',
"// <Image> 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',",
" );",
"}",
"",
"// 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");
}

// 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;",
"export const requiresLoaderProp = false;",
"",
].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;",
"export const requiresLoaderProp = false;",
"",
].join("\n");
}
12 changes: 12 additions & 0 deletions packages/vinext/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1112,6 +1113,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`;
Expand Down Expand Up @@ -3663,6 +3666,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);
}
Expand Down Expand Up @@ -3836,6 +3842,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);
}
Expand Down
Loading
Loading