feat: fetch (suspense) cache handling, and next/cache support

[james-elicx]

This PR does the following:

• Introduces support for the suspense cache by intercepting the requests to our patched fetch.
• Support for the Cache API.
• Patches the built function contents to stop Next.js adding a cache property to RequestInit as it is not supported in workerd.

---

fixes #292

[changeset-bot]

🦋 Changeset detected

Latest commit: 5edf607

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@cloudflare/next-on-pages	Minor
eslint-plugin-next-on-pages	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

🧪 Prereleases are available for testing 🧪

@cloudflare/next-on-pages

You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/next-on-pages/runs/5925180374/npm-package-next-on-pages-419

@cloudflare/eslint-plugin-next-on-pages

You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/next-on-pages/runs/5925180374/npm-package-eslint-plugin-next-on-pages-419

[bluebeel]

Hey,

Your code look a hell more clean and better than mine 🤣

What happens if a user make multiple binding?

export async function getSuspenseCacheInterface(): Promise<CacheInterface> {
	if (process.env.KV_SUSPENSE_CACHE) {
		return new KvCacheInterface(process.env.KV_SUSPENSE_CACHE);
	}

	const cacheApi = await caches.open('suspense-cache');
	return new CacheApiInterface(cacheApi);
}

Do we want to only let a user write in a single destination or multiple one?
If we enforce only one but multiple one are still set, which one should be consider as the "priority/best" one?

---

Is it possible to add R2 binding as a option too?

---

Is it possible to update your example to add the revalidate action so we can test it? 🙏🏾

[james-elicx]

What happens if a user make multiple binding?

export async function getSuspenseCacheInterface(): Promise<CacheInterface> {
	if (process.env.KV_SUSPENSE_CACHE) {
		return new KvCacheInterface(process.env.KV_SUSPENSE_CACHE);
	}

	const cacheApi = await caches.open('suspense-cache');
	return new CacheApiInterface(cacheApi);
}

Do we want to only let a user write in a single destination or multiple one? If we enforce only one but multiple one are still set, which one should be consider as the "priority/best" one?

A user should only have one binding set IMO, but yeah, it would be kind of a priority way I guess. I would probably say KV > D1 > DO > R2 > Cache API, but it shouldn't really matter though since people should not set multiple.

Is it possible to add R2 binding as a option too?

Yes, it just needs another instance of the cache interface class for it, like I did with KV and the Cache API.

Is it possible to update your example to add the revalidate action so we can test it? 🙏🏾

Updated the deployed demo with tag + path revalidation buttons 🙂

[bluebeel]

Thanks it works 💯

Would be interesting to see a benchmark of the different binding how they're behaving and when we access them from around the world.

I think personally I would go for the normal cache API or DO/R2 if I want a global unique store.
KV is the worst one for me because of the avg 60s update sync around the world... 😢

[james-elicx]

Thanks it works 💯

Would be interesting to see a benchmark of the different binding how they're behaving and when we access them from around the world.

I think personally I would go for the normal cache API or DO/R2 if I want a global unique store. KV is the worst one for me because of the avg 60s update sync around the world... 😢

Well, it's a trade-off between latency and availability. Personally, I would not advise people to use the Cache API, since that is different in each data centre.

While KV is eventually globally consistent, it's low latency and is designed for a high volume of reads, so it is more preferable and intended for this kind of use case - it's good for cases where the eventual consistency isn't that important. If one wanted relatively low latency and high consistency, D1 might be a good option, but that's still in alpha. I would assume that R2 probably has a higher latency, but that is also strongly consistent. And durable objects, well, that's the one thing I know nothing about.

[dario-piotrowicz]

Looks awesome! thanks a lot @james-elicx 😍 🚀

I added a bunch of minor comments/suggestions please have a look 🙂

I still need to run this locally and play a bit with it (but the code looks great 😄)

(Also I am still not sure if we should have built-in "adapters" for the caching 😓)

[dario-piotrowicz]

Looks great! 🚀 😄

I just left a few comments please have a look

I also still need to test this deployed, other that these it looks good for me 😄

[bluebeel]

Hello,
I see with the latest commit that you removed the env binding for the specific storage? The adapter will have to be added in user-land in the future?

[james-elicx]

Hello, I see with the latest commit that you removed the env binding for the specific storage? The adapter will have to be added in user-land in the future?

We decided to change the approach slightly where we are creating a generic adaptor that other classes can extend from and use to add additional storage mechanisms. By default, we will use the Cache API, however, if a user wants to use something else like a KV binding, they will need to specify in their next.config.js an import to the relevant code for that storage mechanism.

This is because part of our plan is to allow users to specify their own storage mechanisms in the future, so when we add support for other bindings, it will be done with the same approach that we will use for other custom user-provided solutions, and also will align with how Next.js expects a custom cache to be provided to itself.

We are adding back support for a KV binding in a follow-up PR, although that will be an export that the user will then have to use in their next.config.js, which is where we will then try to get it from, instead of doing it based on an environment variable existing.

[dario-piotrowicz]

This is awesome! thanks a lot @james-elicx for all the effort put into landing this! ❤️ 🚀

[dario-piotrowicz]

And thanks @bluebeel for starting this effort and helping reviewing this PR! 🙂

Sorry for not keeping you in the loop regarding the latest changes, it's as James mentioned the bindings (not sure if all) should be added back in a follow up PR 🙂 (+ the possibility for developers to provide their own adapter so that we don't force them what solution to use for caching)