nitrojs / nitro

Next Generation Server Toolkit. Create web servers with everything you need and deploy them wherever you prefer.
https://nitro.build
MIT License
6.21k stars 512 forks source link

Disable swr behavior by default for cached functions #1950

Open pi0 opened 11 months ago

pi0 commented 11 months ago

Having SWR implicitly by default can have unexpected results especially since it will be also enabled for definedCachedFunction.

One example of when it can go unexpected is that when both a route (via routeRules) and the internal utils it is depending on are using cache, it doubles the cache invalidation time (it happened for ungh few days ago too for example although was explicit flag bug it is an easy trap!)

On the other hand, having SWR for page level caching i belive is something essential for best rendering performances (non API routes). But i guess for this, we have swr: true route rule shortcut so it should not be something to worry.

It is a breaking behavior change still, therefore I am thinking of considering either via a minor version with good enough merits/docs or consider for Nitro v3. Ideas welcome 👍🏼

MickL commented 11 months ago

I would highly suggest swr default set to false as swr creates unexpected behavior: The cache is invalid but still returned. This might be valid for some edge cases, but most of the times devs/users probably dont want to receive an invalid cache response.

Use case example:

pi0 commented 11 months ago

Thanks for the feedback dear @MickL indeed most of the unwanted scenarios are for API/Function calls for action which probably makes no sense with the SWR caching strategy to be enabled by default (but probably also not any caching strategy. it mainly needs to avoid running multiple times in parallel using a semaphore lock/variable).

passionate-bram commented 11 months ago

As a user of cachedEventHandler, I'd expect the function to cache the result and provide appropriate headers. This mimics the behavior of how a cachedFunction would behave; it returns the cached value immediately or re-runs the function. The stale-while-revalidate pattern is more advanced in this case, imagine if cachedFunction kicks off a promise to update the cache.

The twin-like similarity of cachedEventHandler and cachedFunction is probably the main reason for SWR to be opt-in, it defies the default understanding of how a cache would be implemented.

platform-kit commented 10 months ago

@pi0 is there any way to disable the default SWR behavior? I've just adopted Nuxt3 and am baffled as to why my API calls are being cached, and have no clear path to getting around this issue.

passionate-bram commented 10 months ago

@pi0 is there any way to disable the default SWR behavior? I've just adopted Nuxt3 and am baffled as to why my API calls are being cached, and have no clear path to getting around this issue.

You can find what you need in the options for cachedEventHandler. Basically, set swr field of the options object to false.