-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
[Pages] New tutorial - How to implement a cache for Next.js apps #14620
base: production
Are you sure you want to change the base?
Conversation
…n Cloudflare Pages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few comments from me but leaving the full review to Dario
Mostly just around always using default tooling in the docs - lots of people come here who are new, they won't have experience of things like TypeScript or have tooling like pnpm installed.
Create your Next.js project using your preferred package manager. If you have an existing project, update or install the necessary dependencies introduced in the recent versions of `next-on-pages` to [setup bindings](https://developers.cloudflare.com/pages/framework-guides/nextjs/deploy-a-nextjs-site/#set-up-bindings-for-local-development) for local development. | ||
|
||
```sh | ||
$ pnpm create cloudflare@latest my-next-app --framework=next |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we always provide default tooling here (npm), if we want to provide more options that can be done with tabs - e.g.
cloudflare-docs/content/pages/get-started/c3.md
Lines 66 to 87 in 7896102
{{<tabs labels="npm | yarn | pnpm | bun">}} | |
{{<tab label="npm" default="true">}} | |
```sh | |
$ npm create cloudflare@latest [--] [<DIRECTORY>] [OPTIONS] [-- <NESTED ARGS...>] | |
``` | |
{{</tab>}} | |
{{<tab label="yarn">}} | |
```sh | |
$ yarn create cloudflare [--] [<DIRECTORY>] [OPTIONS] [-- <NESTED ARGS...>] | |
``` | |
{{</tab>}} | |
{{<tab label="pnpm">}} | |
```sh | |
$ pnpm create cloudflare@latest [--] [<DIRECTORY>] [OPTIONS] [-- <NESTED ARGS...>] | |
``` | |
{{</tab>}} | |
{{<tab label="bun">}} | |
```sh | |
$ bun create cloudflare@latest [--] [<DIRECTORY>] [OPTIONS] [-- <NESTED ARGS...>] | |
``` | |
{{</tab>}} | |
{{</tabs>}} |
Install the `cf-bindings-proxy` package using your preferred package manager: | ||
|
||
```sh | ||
$ pnpm i cf-bindings-proxy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again here, we want to default to npm
|
||
```sh | ||
# Run the Next.js development server | ||
$ pnpm dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here + above
content/pages/tutorials/how-to-implement-a-cache-for-nextjs/index.md
Outdated
Show resolved
Hide resolved
|
||
```ts | ||
--- | ||
filename: src/lib/cache/kv.ts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a big fan of encouraging KV here personally, spoken to Dario about this quite a bit recently too -- cc: @dario-piotrowicz
But I'll leave the final call here to Dario
content/pages/tutorials/how-to-implement-a-cache-for-nextjs/index.md
Outdated
Show resolved
Hide resolved
2. The `buildCacheKey` method that is necessary because the `.put` method from Cache API requires a URL. | ||
3. There are also `deserialize` and `serialize` methods needed to store and retrieve the cache entry. | ||
|
||
```ts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just like commands, we default to JS and if we want provide an option for TS (highly encouraged).
Thanks a lot for the tutorial! I can see that you've put a lot of time and care into it! ❤️ 🙏 However I have a few concerns about the tutorial's content, but before we get to those I wanted to check with you my fundamental concern that underpins the rest. My main issue with this tutorial is that it seems pretty complex without clearly stating why it's needed and why people should implement this, especially given the fact that Next.js comes with its own data cache implementation out of the box. So I wanted to ask you:
Sorry for the negative spin on things but I think that it's worth carefully consider what content we accept since once we accept it we inherit it with all of its recommendations and maintenance costs, so if you could help me understand better the motivation and goal behind this tutorial that'd be really helpful 🙏 |
@dario-piotrowicz Sorry for the late response and thank you for your feedback and appreciation. Here’s a brief clarification of the tutorial's intention and value. The tutorial presents a caching function similar to unstable_cache, offering explicit cache options (like swr) and works with both Cloudflare's Cache API and Workers KV, addressing issues such as the inability to purge globally with Cache API alone. It shows how to use both simultaneously for better control and flexibility. Target Audience: The Need:
Solution:
Benefits:
Clarification:
I hope this clarifies the objectives and benefits of the tutorial. I am open to suggestions on how to further simplify or better communicate these points within the tutorial. |
No description provided.