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

[Pages] New tutorial - How to implement a cache for Next.js apps #14620

Open
wants to merge 2 commits into
base: production
Choose a base branch
from

Conversation

van14U
Copy link

@van14U van14U commented May 17, 2024

No description provided.

Copy link
Member

@WalshyDev WalshyDev left a 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
Copy link
Member

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.

{{<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
Copy link
Member

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
Copy link
Member

Choose a reason for hiding this comment

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

and here + above


```ts
---
filename: src/lib/cache/kv.ts
Copy link
Member

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

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
Copy link
Member

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).

@github-actions github-actions bot added size/l and removed size/m labels May 17, 2024
@dario-piotrowicz
Copy link
Member

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.
I worry that providing something like this might scare devs off and make them think that they need to jump through a bunch of hoops just to get some caching working, which is definitely not the case.

So I wanted to ask you:

  • who is the target audience of this tutorial? I don't assume novice people, right? (if I were just starting with Next.js I don't think I'd be diving into this manual caching 🤔) if it's advanced users... I am not sure our tutorials here are specifically targeting/suited for them 🤔 (c.c. @kodster28, @WalshyDev)
  • there is a lot of re-implementing from the ground-up the caching solution we have in next-on-pages, but I don't see a huge benefit into doing so, am I missing something? is there a substantial gain from implementing this? or is it just to exemplify how it can be done?

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 🙏

@van14U
Copy link
Author

van14U commented May 24, 2024

@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:
Advanced Next.js users who want more explicit control over how their cache works and who are transitioning from other platforms like Vercel. They may not be familiar with Cloudflare and could encounter caching challenges but want to still use cloudflare technology. This resource offers explicit control over caching (using Cache API and Workers KV) and enhanced caching strategies across regions.

The Need:

  • fetch and unstable_cache in Next.js have implicit SWR behavior, limiting control.
  • Cloudflare’s Cache API doesn’t support global purges, leading to inconsistencies.

Solution:

  • Demonstrates leveraging both Cache API and Workers KV for explicit, flexible caching.
  • Integrates both caching mechanisms, solving global purge issues, and maintaining control.

Benefits:

  • Explicit Cache Options: Overcomes implicit SWR behavior.
  • Simultaneous Use: Ensures data consistency, allowing flexible caching choices.
  • Compatibility: Works with Next.js, offering better control.
  • Tailored Solution: Customizes caching strategies for specific needs. (user codebase)

Clarification:

  • Using KV alone can solve global purge issues, and Cache API can be used separately as needed.
  • This tutorial’s function allows using both simultaneously with explicit control for advanced needs.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants