Skip to content

chore(open-next): apply various improvements #8304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

chore(open-next): apply various improvements #8304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b5e6731
bump wrangler and open-next adapter
dario-piotrowicz Oct 31, 2025
451d065
for the open-next incremental cache use R2 with regional cache instea…
dario-piotrowicz Oct 31, 2025
77a9433
add open-next DO queue
dario-piotrowicz Oct 31, 2025
b0d8f90
enable open-next cache interception
dario-piotrowicz Oct 31, 2025
16290e9
add open-next custom image loader
dario-piotrowicz Oct 31, 2025
65b4b4b
add inline comment for regional cache
dario-piotrowicz Nov 4, 2025
a259564
define `OPEN_NEXT_CLOUDFLARE` variable in `next.consstants.mjs`
dario-piotrowicz Nov 4, 2025
302ba1b
avoid ternaries in `next.config.mjs`
dario-piotrowicz Nov 4, 2025
57d3a4a
update cloudflare-build-and-deployment
dario-piotrowicz Nov 4, 2025
277143b
move image-loader file inside cloudflare directory
dario-piotrowicz Nov 4, 2025
7e6aca3
add missing parenthesis
dario-piotrowicz Nov 4, 2025
95b437c
add env variables for R2 cache batch uploads
dario-piotrowicz Nov 4, 2025
ecc8a53
remove extra backtick
dario-piotrowicz Nov 4, 2025
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
5 changes: 5 additions & 0 deletions .github/workflows/tmp-cloudflare-open-next-deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,8 @@ jobs:
env:
CF_WORKERS_SCRIPTS_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }}
CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }}
CLOUDFLARE_ACCOUNT_ID: fb4a2d0f103c6ff38854ac69eb709272
# Note the R2_* env variables below are useful for speeding up the assets upload
# process, see: https://opennext.js.org/cloudflare/cli#populatecache-command
R2_ACCESS_KEY_ID: ${{ env.R2_ACCESS_KEY_ID }}
R2_SECRET_ACCESS_KEY: ${{ secrets.R2_SECRET_ACCESS_KEY }}
Comment on lines +70 to +71
Copy link
Member Author

@dario-piotrowicz dario-piotrowicz Nov 4, 2025

Choose a reason for hiding this comment

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

I've added these two to the repository, I've created a token specific for the account that only has access to the site's incremental cache r2 bucket, I hope this works for you, if it doesn't I'm happy to change things 🙂

22 changes: 22 additions & 0 deletions apps/site/cloudflare/image-loader.ts
View file
Open in desktop
Copy link
Member

@avivkeller avivkeller Oct 31, 2025

Choose a reason for hiding this comment

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

Can this go in a cloudflare/ subdirectory (cloudflare/images.ts or something)?

Copy link
Member

@ovflowd ovflowd Nov 2, 2025

Choose a reason for hiding this comment

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

Part of me really does not like this function. something about how it's designed 😅

I bert this can be simplified/cleaned up. + I'd rather not use the frigging process.env.NODE_ENV here and actually use the constants we have defined on next.constants. Please also document what this function is doing, why it works and Cloudflare external links. We need to know this exists and howit works.

Copy link
Member Author

@dario-piotrowicz dario-piotrowicz Nov 4, 2025

Choose a reason for hiding this comment

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

This function is taken straight from the open-next documentation: https://opennext.js.org/cloudflare/howtos/image#use-a-custom-loader

I think it might be beneficial (since there are no like custom requirements or anything like that) to keep it as close as possible as the version in the open-next docs? That way if anything on the open-next site changes an updated loader version can just be copy-pasted here instead or having a more bespoke version that we need to more directly maintain? 🤔

Copy link
Member Author

@dario-piotrowicz dario-piotrowicz Nov 4, 2025

Choose a reason for hiding this comment

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

Can this go in a cloudflare/ subdirectory (cloudflare/images.ts or something)?

Moved 👍

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import type { ImageLoaderProps } from 'next/image';

const normalizeSrc = (src: string) => {
return src.startsWith('/') ? src.slice(1) : src;
};

export default function cloudflareLoader({
src,
width,
quality,
}: ImageLoaderProps) {
if (process.env.NODE_ENV === 'development') {
// Serve the original image when using `next dev`
return src;
}
const params = [`width=${width}`];
if (quality) {
params.push(`quality=${quality}`);
}
const paramsString = params.join(',');
return `/cdn-cgi/image/${paramsString}/${normalizeSrc(src)}`;
}
Comment on lines +1 to +22
Copy link
Member

@avivkeller avivkeller Oct 31, 2025

Choose a reason for hiding this comment

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

A few notes with this implementation:

  1. is normalizeSrc needed? Won't the leading / be normalized by the browser? If not, shouldn't there not be a slash / always be a slash passed here? This should only load images, inconsistencies should be resolved at their root, not here?

  2. I think this can be simplified to:

Suggested change
import type { ImageLoaderProps } from 'next/image';
const normalizeSrc = (src: string) => {
return src.startsWith('/') ? src.slice(1) : src;
};
export default function cloudflareLoader({
src,
width,
quality,
}: ImageLoaderProps) {
if (process.env.NODE_ENV === 'development') {
// Serve the original image when using `next dev`
return src;
}
const params = [`width=${width}`];
if (quality) {
params.push(`quality=${quality}`);
}
const paramsString = params.join(',');
return `/cdn-cgi/image/${paramsString}/${normalizeSrc(src)}`;
}
import type { ImageLoaderProps } from 'next/image';
// This can also be a `.replace(...)`?
const normalizeSrc = (src: string) => src.startsWith('/') ? src : `/${src}`;
// For tree-shaking, do this outside
export default process.env.NODE_ENV === 'development' ?
({ src }) => src :
({
src,
width,
quality,
}: ImageLoaderProps) => `/cdn-cgi/image/width=${width}${quality ? `quality=${quality}` : ''}${normalizeSrc(src)}`;

Copy link
Member Author

@dario-piotrowicz dario-piotrowicz Nov 4, 2025

Choose a reason for hiding this comment

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

I'm happy to go with your version, however as I mentioned there might be benefits in keeping this in sync with the open-next documentation? 🤔

(PS: if you want you can also try to upstream these changes in the open-next docs (cc. @vicb))

Copy link
Member

@flakey5 flakey5 Nov 4, 2025

Choose a reason for hiding this comment

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

Imo, the initial implementation is a lot more readable than this (though normalizeSrc may be better as a just a .replace but I'm either or on it)

120 changes: 71 additions & 49 deletions apps/site/next.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
'use strict';
import createNextIntlPlugin from 'next-intl/plugin';

import { BASE_PATH, ENABLE_STATIC_EXPORT } from './next.constants.mjs';
import {
BASE_PATH,
ENABLE_STATIC_EXPORT,
OPEN_NEXT_CLOUDFLARE,
} from './next.constants.mjs';
import { redirects, rewrites } from './next.rewrites.mjs';

/** @type {import('next').NextConfig} */
Expand All @@ -13,44 +17,7 @@ const nextConfig = {
// is being built on a subdirectory (e.g. /nodejs-website)
basePath: BASE_PATH,
// Vercel/Next.js Image Optimization Settings
images: {
// We disable image optimisation during static export builds
unoptimized: ENABLE_STATIC_EXPORT,
// We add it to the remote pattern for the static images we use from multiple sources
// to be marked as safe sources (these come from Markdown files)
remotePatterns: [
{
protocol: 'https',
hostname: 'avatars.githubusercontent.com',
port: '',
pathname: '/**',
},
{
protocol: 'https',
hostname: 'bestpractices.coreinfrastructure.org',
port: '',
pathname: '/**',
},
{
protocol: 'https',
hostname: 'raw.githubusercontent.com',
port: '',
pathname: '/nodejs/**',
},
{
protocol: 'https',
hostname: 'user-images.githubusercontent.com',
port: '',
pathname: '/**',
},
{
protocol: 'https',
hostname: 'website-assets.oramasearch.com',
port: '',
pathname: '/**',
},
],
},
images: getImagesConfig(),
serverExternalPackages: ['twoslash'],
outputFileTracingIncludes: {
// Twoslash needs TypeScript declarations to function, and, by default, Next.js
Expand Down Expand Up @@ -106,17 +73,72 @@ const nextConfig = {
'shiki',
],
},
// If we're building for the Cloudflare deployment we want to set
// an appropriate deploymentId (needed for skew protection)
// TODO: The `OPEN_NEXT_CLOUDFLARE` environment variable is being
// defined in the worker building script, ideally the open-next
// adapter should set it itself when it invokes the Next.js build
// process, onces it does that remove the manual `OPEN_NEXT_CLOUDFLARE`
// definition in the package.json script.
deploymentId: process.env.OPEN_NEXT_CLOUDFLARE
? (await import('@opennextjs/cloudflare')).getDeploymentId()
: undefined,
deploymentId: await getDeploymentId(),
};

function getImagesConfig() {
if (OPEN_NEXT_CLOUDFLARE) {
// If we're building for the Cloudflare deployment we want to use the custom cloudflare image loader
//
// Important: The custom loader ignores `remotePatterns` as those are configured as allowed source origins
// (https://developers.cloudflare.com/images/transform-images/sources/)
// in the Cloudflare dashboard itself instead (to the exact same values present in `remotePatterns` below).
//
return {
loader: 'custom',
loaderFile: './cloudflare/image-loader.ts',
};
}

return {
// We disable image optimisation during static export builds
unoptimized: ENABLE_STATIC_EXPORT,
// We add it to the remote pattern for the static images we use from multiple sources
// to be marked as safe sources (these come from Markdown files)
remotePatterns: [
{
protocol: 'https',
hostname: 'avatars.githubusercontent.com',
port: '',
pathname: '/**',
},
{
protocol: 'https',
hostname: 'bestpractices.coreinfrastructure.org',
port: '',
pathname: '/**',
},
{
protocol: 'https',
hostname: 'raw.githubusercontent.com',
port: '',
pathname: '/nodejs/**',
},
{
protocol: 'https',
hostname: 'user-images.githubusercontent.com',
port: '',
pathname: '/**',
},
{
protocol: 'https',
hostname: 'website-assets.oramasearch.com',
port: '',
pathname: '/**',
},
],
};
}

async function getDeploymentId() {
if (OPEN_NEXT_CLOUDFLARE) {
// If we're building for the Cloudflare deployment we want to set
// an appropriate deploymentId (needed for skew protection)
return (await import('@opennextjs/cloudflare')).getDeploymentId();
}

return undefined;
}

const withNextIntl = createNextIntlPlugin('./i18n.tsx');
export default withNextIntl(nextConfig);
11 changes: 11 additions & 0 deletions apps/site/next.constants.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,3 +193,14 @@ export const EOL_VERSION_IDENTIFIER = 'End-of-life';
*/
export const VULNERABILITIES_URL =
'https://raw.githubusercontent.com/nodejs/security-wg/main/vuln/core/index.json';

/**
* Whether the build process is targeting the Cloudflare open-next build or not.
*
* TODO: The `OPEN_NEXT_CLOUDFLARE` environment variable is being
* defined in the worker building script, ideally the open-next
* adapter should set it itself when it invokes the Next.js build
* process, once it does that remove the manual `OPEN_NEXT_CLOUDFLARE`
* definition in the package.json script.
*/
export const OPEN_NEXT_CLOUDFLARE = process.env.OPEN_NEXT_CLOUDFLARE;
17 changes: 15 additions & 2 deletions apps/site/open-next.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,21 @@
import type { OpenNextConfig } from '@opennextjs/cloudflare';
import { defineCloudflareConfig } from '@opennextjs/cloudflare';
import incrementalCache from '@opennextjs/cloudflare/overrides/incremental-cache/kv-incremental-cache';
import r2IncrementalCache from '@opennextjs/cloudflare/overrides/incremental-cache/r2-incremental-cache';
import { withRegionalCache } from '@opennextjs/cloudflare/overrides/incremental-cache/regional-cache';
import doQueue from '@opennextjs/cloudflare/overrides/queue/do-queue';

const cloudflareConfig = defineCloudflareConfig({ incrementalCache });
const cloudflareConfig = defineCloudflareConfig({
/**
* The regional cache implementation with R2 (instead of a KV one) is is chosen here
* for both R2's strong consistency alongside the regional cache performance gains.
* @see https://opennext.js.org/cloudflare/caching
*/
incrementalCache: withRegionalCache(r2IncrementalCache, {
Copy link
Member

@ovflowd ovflowd Nov 2, 2025

Choose a reason for hiding this comment

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

Can you add inline comments reference @see to relevant docs? And also inline comment what these settings do? 🙇

Copy link
Contributor

@vicb vicb Nov 3, 2025

Choose a reason for hiding this comment

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

The PR description mentions

replaced the kv incremental cache with the more performance r2 + regional cache implementation

The actual issue with KV is that it is eventually consistent which might lead to unexpected behavior. This is why we recommend R2 instead - R2 is strongly consistent.

Copy link
Member

@ovflowd ovflowd Nov 3, 2025

Choose a reason for hiding this comment

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

I think you misunderstood, I'm asking for comments to be added on the code, so that it stays there for reference :)

Copy link
Contributor

@vicb vicb Nov 3, 2025

Choose a reason for hiding this comment

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

I got that, I added the comment mostly for Dario to update the PR description and add comments here

Copy link
Member Author

@dario-piotrowicz dario-piotrowicz Nov 4, 2025

Choose a reason for hiding this comment

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

@ovflowd I've added an inline comment 65b4b4b

please let me know if this works for you 🙏

mode: 'long-lived',
}),
queue: doQueue,
enableCacheInterception: true,
});

const openNextConfig: OpenNextConfig = {
...cloudflareConfig,
Expand Down
4 changes: 2 additions & 2 deletions apps/site/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
"@flarelabs-net/wrangler-build-time-fs-assets-polyfilling": "^0.0.1",
"@next/eslint-plugin-next": "15.5.4",
"@node-core/remark-lint": "workspace:*",
"@opennextjs/cloudflare": "^1.6.4",
"@opennextjs/cloudflare": "^1.11.0",
"@playwright/test": "^1.56.1",
"@testing-library/user-event": "~14.6.1",
"@types/mdast": "^4.0.4",
Expand All @@ -108,7 +108,7 @@
"typescript": "catalog:",
"typescript-eslint": "~8.45.0",
"user-agent-data-types": "0.4.2",
"wrangler": "^4.33.1"
"wrangler": "^4.45.3"
},
"imports": {
"#site/*": [
Expand Down
20 changes: 17 additions & 3 deletions apps/site/wrangler.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,24 @@
"fs": "./.wrangler/fs-assets-polyfilling/polyfills/node/fs.ts",
"fs/promises": "./.wrangler/fs-assets-polyfilling/polyfills/node/fs/promises.ts",
},
"kv_namespaces": [
"r2_buckets": [
{
"binding": "NEXT_INC_CACHE_KV",
"id": "69b7422d56dd4244bc0127b69ecdc36f",
"binding": "NEXT_INC_CACHE_R2_BUCKET",
"bucket_name": "next-cache-r2-for-open-next-website",
},
],
"durable_objects": {
"bindings": [
{
"name": "NEXT_CACHE_DO_QUEUE",
"class_name": "DOQueueHandler",
},
],
},
"migrations": [
{
"tag": "v1",
"new_sqlite_classes": ["DOQueueHandler"],
},
],
}
5 changes: 3 additions & 2 deletions docs/cloudflare-build-and-deployment.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ Key configurations include:
- This is currently set to `fb4a2d0f103c6ff38854ac69eb709272`, which is the ID of a Cloudflare account controlled by Node.js, and used for testing.
- `build`: Defines the build command to generate the Node.js filesystem polyfills required for the application to run on Cloudflare Workers. This uses the [`@flarelabs/wrangler-build-time-fs-assets-polyfilling`](https://github.com/flarelabs-net/wrangler-build-time-fs-assets-polyfilling) package.
- `alias`: Maps aliases for the Node.js filesystem polyfills generated during the build process.
- `kv_namespaces`: Contains a single KV binding definition for `NEXT_INC_CACHE_KV`. This is used to implement the Next.js incremental cache.
- This is currently set up to a KV in the aforementioned Cloudflare testing account.
- `r2_buckets`: Contains a single R2 binding definition for `NEXT_INC_CACHE_R2_BUCKET`. This is used to implement the Next.js incremental cache.
- This is currently set up to a R2 bucket in the aforementioned Cloudflare testing account.
- `durable_objects`: Contains a single DurableObject binding definition for `NEXT_CACHE_DO_QUEUE`. This is used to implement the Open-next cache queue.

### OpenNext Configuration

Expand Down
Loading
Loading