next.js icon indicating copy to clipboard operation
next.js copied to clipboard

Unclear caching behavior in nextjs 14 and 15

Open mastoj opened this issue 1 year ago • 3 comments

Link to the code that reproduces this issue

https://github.com/mastoj/wrapped-fetch-debug

To Reproduce

I have the exact same app in nextjs 14: https://github.com/mastoj/wrapped-fetch-bug-14 and nextjs 15: https://github.com/mastoj/wrapped-fetch-debug

What we are doing is testing what is happening when you wrap fetch in a higher order function, since that is a natural way to componse functions together.

Both of the solution is not behaving as I expect them to. To reproduce the issue in any of the repos do:

  1. pnpm run build
  2. pnpm run start
  3. http://localhost:3000/debug
  4. http://localhost:3000/debug

Current vs. Expected behavior

Doing the first load will print the logs from the api/route:

==> Getting some data: /api/data/1 ==> Getting some data: /api/data/2 ==> Getting some data: /api/data/3

which is as expected. Doing the second load of the page is what makes things interesting. In our /debug page we are making three api-calls in a slightly different way, but I expect them all to be cached.

Nextjs 14 behavior

For some reason the fetch call that is wrapped using a higher order function won't cache at all and we get the following output:

==> Getting some data: /api/data/2

Nextjs 15 behavior

In 15 the behavior differed quite a lot from 14. To some degree it make sense since the defaults has changed. Since we are using explicit const dynamic and revalidate on fetch I would actually expect the behavior to be the same, but that is not the case. In 15 it seems like nothing is cached and we make api calls every time.

==> Getting some data: /api/data/1 ==> Getting some data: /api/data/2 ==> Getting some data: /api/data/3

Expected behavior

Even though we have export const dynamic = "force-dynamic" I would expect the revalidate on the fetch calls to be honored. That assumptions means I would expect no outputs from the log from the routes to be outputted since no calls should be made.

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.6.0: Mon Jul 29 21:13:04 PDT 2024; root:xnu-10063.141.2~1/RELEASE_ARM64_T6020
  Available memory (MB): 65536
  Available CPU cores: 12
Binaries:
  Node: 20.14.0
  npm: 10.7.0
  Yarn: 1.22.22
  pnpm: 9.9.0
Relevant Packages:
  next: 14.2.8 // An outdated version detected (latest is 15.0.1), upgrade is highly recommended!
  eslint-config-next: 14.2.8
  react: 18.3.1
  react-dom: 18.3.1
  typescript: 5.6.3
Next.js Config:
  output: N/A
 ⚠ An outdated version detected (latest is 15.0.1), upgrade is highly recommended!
   Please try the latest canary version (`npm install next@canary`) to confirm the issue still exists before creating a new issue.
   Read more - https://nextjs.org/docs/messages/opening-an-issue

Which area(s) are affected? (Select all that apply)

Not sure

Which stage(s) are affected? (Select all that apply)

next start (local), Vercel (Deployed)

Additional context

I have verified the behavior both locally and on vercel.

mastoj avatar Oct 25 '24 20:10 mastoj

Typo. “ For some reason the fetch call that is wrapped using a higher order function want cache at all and we get the following output:”.

won’t cache**

gruckion avatar Oct 25 '24 20:10 gruckion

Force dynamic is used to ensure that the request is generated at request time. It is not used for caching.

https://nextjs.org/docs/app/api-reference/file-conventions/route-segment-config#dynamic

As for using revalidate in the fetch request. You are not using the ‘cache’ attribute as well. Which in NextJS 15 defaults to no store.

https://nextjs.org/docs/app/api-reference/functions/fetch

So you will need to set that if you want the fetch request to be cached.

Note the docs say:

As a convenience, it is not necessary to set the cache option if revalidate is set to a number.

But I believe this is only true for NextJS 14. Please try setting ‘“cache”: “force-cache”,’ and try again if you are using NextJS 15.

You can read more about this change here.

https://nextjs.org/blog/next-15 https://nextjs.org/blog/next-15-rc

image

gruckion avatar Oct 25 '24 21:10 gruckion

(fixed typo)

Added cache: "force-cache" with no difference in behavior: https://github.com/mastoj/wrapped-fetch-debug/commit/dff943b2babfda5313874217f9c8cdbc9964cc2b

mastoj avatar Oct 25 '24 22:10 mastoj

Is the Next.js 15 behavior for next dev or next start? Are there any differences between next start and next dev?

eps1lon avatar Oct 28 '24 15:10 eps1lon

It was running next start and also on vercel.

mastoj avatar Nov 04 '24 20:11 mastoj

Hi @mastoj -- just to provide a bit of additional context in addition to the above PRs to resolve the issue:

The issue you're observing in 14 & 15 with your custom fetch implementation not caching will be addressed by https://github.com/vercel/next.js/pull/72363. What's happening is that your custom fetch implementation is retaining an un-patched version of fetch, before Next.js has a chance to override its semantics to provide custom fetching behavior.

The issue where none of them are caching in 15 with force-dynamic is indeed a bug that should be fixed with the above PRs. force-dynamic was being overzealous and ignoring both force-cache and revalidate -- the presence of either one should opt into caching behavior.

ztanner avatar Nov 06 '24 14:11 ztanner

As another update, it's proving to be difficult to ensure the version of fetch that you originally captured in module scope is patched before we require the module. However, I think you can refactor this slightly and still get the behavior you want, by getting the base fetch function at invocation time.

const debugFetch = () => {
  return async (url, init) => {
    return fetch(url, init).then((res) => {
      return handleFetchResponse(res, url, init)
    })
  }
}
const testFetch = debugFetch();

That way you grab fetch after Next.js has instrumented it with the expected caching behaviors. Let me know if there's a particular reason why this approach doesn't work.

ztanner avatar Nov 07 '24 15:11 ztanner

Thanks for the input @ztanner . I will try if that works, which it might.

I wouldn't say that there is particular reason why that wouldn't work. For me it is more about expectations, I expect fetch to work in a certain way and I also expect to be able to use standard javascript features, like passing functions, to work. It is a bit clunky if I, or other developers, need to know when and how to use it.

A common use case as I see it would be to "enrich" behavior of fetch. You might have a function like otelFetch that takes in a fetch like function, or any function for that matter, as argument and adds extra tracing, than you could have a debugFetchthat adds extra functionality like debug logging on client and server, lastly you might want to compose those two together.

So in simple pseudo code that would be something like:

const debugFetch = (fetcher) => {
  return async (url, init) => {
    return fetcher(url, init).then((res) => {
      return handleFetchResponse(res, url, init)
    })
  }
}
const otelFetch = (fetcher) => {
  return async (url, init) => {
    return fetcher(url, init).then((res) => {
      return handleFetchResponse(res, url, init)
    })
  }
}

const myFetch = debugFetch(otelFetch(fetch));
const data = await myFetch(url);

The above won't be possible if I can't compose the functions, which I can't if I explicitly call fetch in the functions... then I have to choose one or the other or create a new function.

So in short, this breaks function composability.

mastoj avatar Nov 07 '24 17:11 mastoj

@mastoj that makes sense, and I think that's a perfectly reasonable assumption! I'm going to land the above mentioned PR that attempts to address it. It seems to work in the reproduction case I've created, but let me know if not.

ztanner avatar Nov 07 '24 21:11 ztanner

I will keep an eye for when it is possible to test in canary.

Sent from Outlook for iOShttps://aka.ms/o0ukef


From: Zack Tanner @.> Sent: Thursday, November 7, 2024 10:06:07 PM To: vercel/next.js @.> Cc: Tomas Jansson @.>; Mention @.> Subject: Re: [vercel/next.js] Unclear caching behavior in nextjs 14 and 15 (Issue #71881)

@mastojhttps://github.com/mastoj that makes sense, and I think that's a perfectly reasonable assumption! I'm going to land the above mentioned PR that attempts to address it. It seems to work in the reproduction case I've created, but let me know if not.

— Reply to this email directly, view it on GitHubhttps://github.com/vercel/next.js/issues/71881#issuecomment-2463213857, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAD2NMGLEB47F3AIUKXLJQDZ7PI37AVCNFSM6AAAAABQUAS2Y6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRTGIYTGOBVG4. You are receiving this because you were mentioned.Message ID: @.***>

mastoj avatar Nov 07 '24 21:11 mastoj

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

github-actions[bot] avatar Nov 22 '24 00:11 github-actions[bot]