Skip to content
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.

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

feat: use the cache api if there is no kv cache available #136

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
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
7 changes: 7 additions & 0 deletions .changeset/gold-onions-lick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@opennextjs/cloudflare": minor
---

feat: use the cache api if there is no kv cache available

Instead of requiring a KV cache is available in the environment for Next.js caching to work, the cache handle will default to using the Cache API.
65 changes: 65 additions & 0 deletions packages/cloudflare/src/cli/templates/cache-handler/cache-store.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import type { IncrementalCacheValue } from "next/dist/server/response-cache";

export type CacheEntry = {
lastModified: number;
value: IncrementalCacheValue | null;
};

export type CacheStore = {
get: (key: string) => Promise<CacheEntry | null>;
Copy link
Contributor

Choose a reason for hiding this comment

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

async?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They are - the return type is a promise

put: (key: string, entry: CacheEntry, ttl?: number) => Promise<void>;
};

export function getCacheStore() {
const kvName = process.env.__OPENNEXT_KV_BINDING_NAME;
if (process.env[kvName]) {
Copy link
Contributor

@vicb vicb Nov 18, 2024

Choose a reason for hiding this comment

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

I think we should remove the bindings from process.env and we should move to getCloudflareContext - could be done in a follow up PR, in which case it would be nice to add a // TODO: ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

getCloudflareContext is async at the moment because of local dev, so it won't work here unless we move the retrieving of the cache to be part of an interaction with the cache instead of when the cache handler is instantiated.

Copy link
Contributor

Choose a reason for hiding this comment

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

yep, you can create a TODO/issue for that. thanks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

return new KVStore(process.env[kvName] as unknown as KVNamespace);
}

return new CacheAPIStore();
}

const oneYearInMs = 31536000;

class KVStore implements CacheStore {
constructor(private store: KVNamespace) {}

get(key: string) {
return this.store.get<CacheEntry>(key, "json");
}

put(key: string, entry: CacheEntry, ttl = oneYearInMs) {
return this.store.put(key, JSON.stringify(entry), {
expirationTtl: ttl,
});
}
}

class CacheAPIStore implements CacheStore {
Copy link
Contributor

@vicb vicb Nov 18, 2024

Choose a reason for hiding this comment

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

@petebacondarwin @dario-piotrowicz I'm not familiar the cf cache API, do you have thoughts about this class?

With my limited understanding of the cache API, I'm not sure if we should use it for the fetch cache (as long as the cache headers are set correctly on the fetch call).

I'm also wondering about custom keys.

constructor(private name = "__opennext_cache") {}

async get(key: string) {
const cache = await caches.open(this.name);
const response = await cache.match(this.createCacheKey(key));

if (response) {
return response.json<CacheEntry>();
}

return null;
}

async put(key: string, entry: CacheEntry, ttl = oneYearInMs) {
const cache = await caches.open(this.name);

const response = new Response(JSON.stringify(entry), {
headers: { "cache-control": `max-age=${ttl}` },
});

return cache.put(this.createCacheKey(key), response);
}

private createCacheKey(key: string) {
return `https://${this.name}.local/entry/${key}`;
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { KVNamespace } from "@cloudflare/workers-types";
import type {
CacheHandler,
CacheHandlerContext,
Expand All @@ -14,20 +13,17 @@ import {
RSC_SUFFIX,
SEED_DATA_DIR,
} from "../../constants/incremental-cache";
import type { CacheEntry, CacheStore } from "./cache-store";
import { getCacheStore } from "./cache-store";
import { getSeedBodyFile, getSeedMetaFile, getSeedTextFile, parseCtx } from "./utils";

type CacheEntry = {
lastModified: number;
value: IncrementalCacheValue | null;
};

export class OpenNextCacheHandler implements CacheHandler {
protected kv: KVNamespace | undefined;
protected cache: CacheStore;

protected debug: boolean = !!process.env.NEXT_PRIVATE_DEBUG_CACHE;

constructor(protected ctx: CacheHandlerContext) {
this.kv = process.env[process.env.__OPENNEXT_KV_BINDING_NAME] as KVNamespace | undefined;
this.cache = getCacheStore();
}

async get(...args: Parameters<CacheHandler["get"]>): Promise<CacheHandlerValue | null> {
Expand All @@ -36,13 +32,11 @@ export class OpenNextCacheHandler implements CacheHandler {

if (this.debug) console.log(`cache - get: ${key}, ${ctx?.kind}`);

if (this.kv !== undefined) {
try {
const value = await this.kv.get<CacheEntry>(key, "json");
if (value) return value;
} catch (e) {
console.error(`Failed to get value for key = ${key}: ${e}`);
}
try {
const value = await this.cache.get(key);
if (value) return value;
} catch (e) {
console.error(`Failed to get value for key = ${key}: ${e}`);
}

// Check for seed data from the file-system.
Expand Down Expand Up @@ -118,10 +112,6 @@ export class OpenNextCacheHandler implements CacheHandler {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const [key, entry, _ctx] = args;

if (this.kv === undefined) {
return;
}

if (this.debug) console.log(`cache - set: ${key}`);

const data: CacheEntry = {
Expand All @@ -130,17 +120,14 @@ export class OpenNextCacheHandler implements CacheHandler {
};

try {
await this.kv.put(key, JSON.stringify(data));
await this.cache.put(key, data);
} catch (e) {
console.error(`Failed to set value for key = ${key}: ${e}`);
}
}

async revalidateTag(...args: Parameters<CacheHandler["revalidateTag"]>) {
const [tags] = args;
if (this.kv === undefined) {
return;
}

if (this.debug) console.log(`cache - revalidateTag: ${JSON.stringify([tags].flat())}`);
}
Expand Down