implement custom cache handler logic #606

dario-piotrowicz · 2023-12-20T14:30:29Z

resolves #529
resolves #628

Example app: https://github.com/dario-piotrowicz/next-geolocation-pages
(application using a supabase custom cache handler)

Vercel deployment: https://next-geolocation-pages-glqcs9da0-dario-piotrowicz.vercel.app/
Pages deployment: https://8e0d4e38.next-geolocation-pages.pages.dev/

TODO:

Next and next-on-pages seem to be calling the handler inconsistently, they should get aligned (example)
(solved, see: dario-piotrowicz/next-geolocation-pages@a4eb865)
Add missing tsdocs and comments
Make sure that all different next.config.js files are correctly handled, this means function exports (as mentioned here and .mjs extensions)
Make sure that Cloudflare bindings are accessible from handlers (I can't see why not but it's still worth checking)
(tested and everything seems to be in order, see: dario-piotrowicz/next-geolocation-pages@ac3f05e)
Update docs

TODO as a follow up, not to scope-creep this PR:

create a new next-on-pages submodule for caching utilities, it should export the KV handler and make it configurable so that developers can use it and set any binding name they want
explore if bindings in a cache handler can be used in next-dev, I suspect not but it's still worth exploring (if it's not possible it also needs to be properly documented)

changeset-bot · 2023-12-20T14:30:33Z

⚠️ No Changeset found

Latest commit: dc6770c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

github-actions · 2023-12-20T14:31:14Z

🧪 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/7624406804/npm-package-next-on-pages-606

@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/7624406804/npm-package-eslint-plugin-next-on-pages-606

packages/next-on-pages/templates/_worker.js/utils/cache.ts

petebacondarwin

LGTM - I left a bunch of nits but nothing blocking.
(Caveat - I have not actually tried this out locally!!)

packages/next-on-pages/build-metadata.d.ts

packages/next-on-pages/src/buildApplication/buildCacheFiles.ts

packages/next-on-pages/src/buildApplication/nextConfig.ts

petebacondarwin · 2024-01-02T13:54:01Z

packages/next-on-pages/tests/src/buildApplication/nextConfig.test.ts

+
+	test('processing a next.config.js file exporting a function', async () => {
+		mocks.nextConfigJsFileExists = true;
+		mocks.nextConfigMjsFileExists = false;


What happens if both files exist??

If both files exist Next.js picks the first/default one (next.config.js) and ignored the mjs one
(source)

We also do the same:

next-on-pages/packages/next-on-pages/src/buildApplication/nextConfig.ts

Lines 28 to 29 in 72fb43d

const configFilePath =

(await getConfigFilePath('js')) || (await getConfigFilePath('mjs'));

packages/next-on-pages/docs/caching.md

this includes: - refactoring existing logic so that it uses the incrementalCache class concept that Next.js also uses (so that we alight the two implementations) - reading the user next.config.(m)js file and create a custom cache handler and collecting the cache handler referenced in the incrementalCacheHandlerPath config - building the custom cache handler instead of the built-in ones in case one was indeed provided by the user

james-elicx · 2024-01-22T21:03:52Z

Revalidating a tag:

This logging can be enabled with the environment variable NEXT_PRIVATE_DEBUG_CACHE=true

dario-piotrowicz · 2024-01-23T11:29:22Z

packages/next-on-pages/templates/cache/incrementalCache.ts

+		entry = {
+			isStale,
+			revalidateAfter,
+			value: null,
+		};
+
+		if (cacheData) {
+			entry.value = cacheData.value;
+			entry.age = age;
+		} else {
+			await this.set(cacheKey, entry.value, ctx);
+		}


Note: I think this is incorrect

if cacheData is falsy we end up setting the cacheKay value to null here, which generates the error mentioned in #606 (comment)

But the exact same happens in the Next.js code... 🤔

https://github.com/vercel/next.js/blob/fc7a28a0dd946437cbd5144a57f5296e2659899f/packages/next/src/server/lib/incremental-cache/index.ts#L524-L530

dario-piotrowicz commented Dec 27, 2023

View reviewed changes

packages/next-on-pages/templates/_worker.js/utils/cache.ts Outdated Show resolved Hide resolved

dario-piotrowicz force-pushed the custom-incremental-cache-handler branch from 3209dd8 to ad9cdbf Compare December 27, 2023 14:27

dario-piotrowicz marked this pull request as ready for review December 29, 2023 15:05

dario-piotrowicz requested review from petebacondarwin and jculvey January 2, 2024 10:31

dario-piotrowicz force-pushed the custom-incremental-cache-handler branch 3 times, most recently from 859f576 to 5b98363 Compare January 2, 2024 11:31

petebacondarwin reviewed Jan 2, 2024

View reviewed changes

petebacondarwin approved these changes Jan 2, 2024

View reviewed changes

dario-piotrowicz force-pushed the custom-incremental-cache-handler branch 2 times, most recently from 72fb43d to 95cf34b Compare January 22, 2024 10:32

dario-piotrowicz added 8 commits January 22, 2024 14:16

add unit tests for extractBuildMetadataConfig

a539601

update caching doc

4828126

update support (in docs and eslint) for incrementalCacheHandlerPath

cfdc265

fixup! update caching doc

ede3739

simplify config.experimental creation

5e629dd

move incrementalCacheHandlerPath check as suggested in PR review

689bad8

introduce NextConfigExperimental as suggested in PR review

de839e0

dario-piotrowicz force-pushed the custom-incremental-cache-handler branch from 2667560 to de839e0 Compare January 22, 2024 14:17

update experimental.incrementalCacheHandlerPath to cacheHandler

dc6770c

dario-piotrowicz commented Jan 23, 2024

View reviewed changes

dario-piotrowicz marked this pull request as draft January 31, 2024 10:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement custom cache handler logic #606

implement custom cache handler logic #606

dario-piotrowicz commented Dec 20, 2023 •

edited

changeset-bot bot commented Dec 20, 2023 •

edited

github-actions bot commented Dec 20, 2023 •

edited

@cloudflare/next-on-pages

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

petebacondarwin left a comment

petebacondarwin Jan 2, 2024

dario-piotrowicz Jan 16, 2024 •

edited

james-elicx commented Jan 22, 2024 •

edited

dario-piotrowicz Jan 23, 2024

dario-piotrowicz Jan 23, 2024

	const configFilePath =
	(await getConfigFilePath('js')) \|\| (await getConfigFilePath('mjs'));

implement custom cache handler logic #606

Are you sure you want to change the base?

implement custom cache handler logic #606

Conversation

dario-piotrowicz commented Dec 20, 2023 • edited

changeset-bot bot commented Dec 20, 2023 • edited

⚠️ No Changeset found

github-actions bot commented Dec 20, 2023 • edited

@cloudflare/next-on-pages

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

petebacondarwin left a comment

Choose a reason for hiding this comment

petebacondarwin Jan 2, 2024

Choose a reason for hiding this comment

dario-piotrowicz Jan 16, 2024 • edited

Choose a reason for hiding this comment

james-elicx commented Jan 22, 2024 • edited

dario-piotrowicz Jan 23, 2024

Choose a reason for hiding this comment

dario-piotrowicz Jan 23, 2024

Choose a reason for hiding this comment

dario-piotrowicz commented Dec 20, 2023 •

edited

changeset-bot bot commented Dec 20, 2023 •

edited

github-actions bot commented Dec 20, 2023 •

edited

dario-piotrowicz Jan 16, 2024 •

edited

james-elicx commented Jan 22, 2024 •

edited