storyblok / storyblok-js-client

Universal JavaScript client for Storyblok's API
MIT License
125 stars 86 forks source link

Update Cache Version (`cv`) Properly After API Response #823

Closed juusopiikkila closed 6 days ago

juusopiikkila commented 4 months ago

The current implementation of caching in the Storyblok JavaScript client does not seem to update the cv (cache version) in cacheVersions after receiving a new cv from the API response when the version is set to "published". This might lead to issues with stale data being served from the cache.

Expected Behavior

After receiving an API response, if the cv from the response is different from what is stored in cacheVersions for the given token, the client should:

  1. Invalidate the old cache.
  2. Update cacheVersions with the new cv from the response.

Suggested Fix

Enhance the cacheResponse function to update the cv in cacheVersions after every API response if the value changes. Here is the proposed code snippet to handle this:

// Inside cacheResponse function, after receiving the response
if (
    response.data.cv 
    && params.token 
    && cacheVersions[params.token] != response.data.cv
) {
    await this.flushCache()
    cacheVersions[params.token] = response.data.cv
}
remcoder commented 2 months ago

Just ran into this.. Annoying. Really should be fixed.

A crude workaround is to clear the cacheversions before requesting data:

client.clearCacheVersion();
client.get('cdn/stories/mypage')
cemaxx1990 commented 2 months ago

We also had this issue and searched for quite a while. We are using the nuxt2 storyblok wrapper.

Our solution is to call 'cdn/spaces/me' and use the version we get there as the cv value.

const cdnSpace = await storyblokApi.get('cdn/spaces/me');

cv: cdnSpace.data.space.version
codeflorist commented 2 months ago

I believe the culprit lies with this if-statement: https://github.com/storyblok/storyblok-js-client/blob/3592e384637659112d710fe59fe216130dab0350/src/index.ts#L632

          if (
            params.version === 'draft' &&
            cacheVersions[params.token] != response.data.cv
          ) {
            await this.flushCache()
          }

I believe this means, a cache-flush is only performed, if draft versions are requested. Otherwise i guess the response will stay in memory-cache forever.

I don't know the purpose of limiting the flush-behaviour to draft requests. If it is unwanted ot a bug, imho it should be removed.

This has been causing some headaches for quite a while - especially in the Nuxt and nextjs crowd and reports pop up every few weeks in the Discord.

awacode21 commented 2 months ago

I have been annoyed with this problem for more than half a year. I think this should really be fixed. Currently you always have to perform a new deployment/server restart to make the published changes visible on the server side.

zipme commented 1 month ago

Just ran into this.. Annoying. Really should be fixed.

A crude workaround is to clear the cacheversions before requesting data:

client.clearCacheVersion();
client.get('cdn/stories/mypage')

Tried this but doesn't work either

codeflorist commented 1 month ago

Just ran into this.. Annoying. Really should be fixed. A crude workaround is to clear the cacheversions before requesting data:

client.clearCacheVersion();
client.get('cdn/stories/mypage')

Tried this but doesn't work either

Alternative workarounds could be:

Setup the client with cache type none:

let Storyblok = new StoryblokClient({
  accessToken: <YOUR_SPACE_ACCESS_TOKEN>,
  cache: {
    type: "none",
  },
});

Or state the current timestamp as the cache-version in each if your calls:

Storyblok.get(
  'cdn/stories/home',
  {
    version: 'published',
    cv: Date:now()
  }
)

But both solutions effectively disable CDN caching, presumably creating more traffic.

pauloamgomes commented 1 month ago

Just ran into this.. Annoying. Really should be fixed. A crude workaround is to clear the cacheversions before requesting data:

client.clearCacheVersion();
client.get('cdn/stories/mypage')

Tried this but doesn't work either

Alternative workarounds could be:

Setup the client with cache type none:

let Storyblok = new StoryblokClient({
  accessToken: <YOUR_SPACE_ACCESS_TOKEN>,
  cache: {
    type: "none",
  },
});

Or state the current timestamp as the cache-version in each if your calls:

Storyblok.get(
  'cdn/stories/home',
  {
    version: 'published',
    cv: Date:now()
  }
)

But both solutions effectively disable CDN caching, presumably creating more traffic.

I'm using that solution in the context of next.js (cache type none and passing cv parameter with Date.now()) it seems the only way to make the sdk work properly with next.js fetch caching

alvarosabu commented 1 month ago

Hi everyone thanks for the detailed explanation.

I just triaged this one, I'm going to take it to plan internally.

Keep you posted