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

Cache problem with client-side navigation: the response is now __data.js instead of __data.json #6458

Closed
kristjanmar opened this issue Aug 31, 2022 · 4 comments · Fixed by #6904
Labels
bug Something isn't working
Milestone

Comments

@kristjanmar
Copy link

kristjanmar commented Aug 31, 2022

Describe the bug

I believe the recent change where client-side navigation responds with __data.js instead of __data.json can be problematic.

I have Cloudflare on my site and I just realized that the __data.js files that are returned when navigating between pages are being cached on the edge and possibly also in the browser cache. I don't have any custom page rules for .js files in Cloudflare, this is the default behavior.

In some cases, the "age" in the response headers is showing very high numbers, much higher than what I have in my s-max-age cache-control headers in hooks.js (which used to cache on Vercel edge only, not Cloudflare).

681e4b3be00bfa462433bc8450004e6b

This means that many users are potentially seeing very old and outdated info for pages that are supposed to be highly dynamic.

Reproduction

I don't think I can set up a reproduction since it would require a custom domain and adding it to Cloudflare.

Logs

No response

System Info

System:
    OS: Windows 10 10.0.22000
    CPU: (32) x64 AMD Ryzen 9 5950X 16-Core Processor
    Memory: 50.08 GB / 63.91 GB
  Binaries:
    Node: 16.16.0 - C:\Program Files\nodejs\node.EXE
    npm: 8.11.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.22000.120.0), Chromium (104.0.1293.70)
    Internet Explorer: 11.0.22000.120
  npmPackages:
    @sveltejs/adapter-auto: 1.0.0-next.69 => 1.0.0-next.70
    @sveltejs/kit: 1.0.0-next.454 => 1.0.0-next.456
    svelte: ^3.49.0 => 3.49.0
    vite: 3.1.0-beta.1 => 3.1.0-beta.1

Severity

Blocking an upgrade

Additional Information

Update: I rolled back to 446 for now since the 447 update seems to also have introduced a bunch of weird issues with both Sentry and Plausible Analytics.

@kristjanmar
Copy link
Author

I checked my Cloudflare cache logs since I updated SvelteKit and it looks like Cloudflare has served 3.6% of all my __data.js files from Cache. I assume this number could be much higher for others using Cloudflare that don't have as many unique URLs as my site (over 100K).

e3a244e86361987e1892e18116fb4004

@Rich-Harris Rich-Harris added the bug Something isn't working label Sep 6, 2022
@Rich-Harris Rich-Harris added this to the 1.0 milestone Sep 6, 2022
@lkj4
Copy link

lkj4 commented Sep 8, 2022

fwiw, I get from ...__data.js no JSON but something starting with window.__sveltekit_data = {function{a,b.... What is this?

@PatrickG
Copy link
Contributor

PatrickG commented Sep 8, 2022

fwiw, I get from ...__data.js no JSON but something starting with window.__sveltekit_data = {function{a,b.... What is this?

It's devalue. That's the reason the file-extension has been changed to .js.

@Tal500
Copy link
Contributor

Tal500 commented Sep 10, 2022

I knew that this caching problem will occur not only in the context of legacy support in #6265!!

I had solved this issue by appending the local JS date value to the query, to force the caching to not happen, via:

const time_arg = `__svelte_kit_legacy_time=${new Date().getTime()}`;
const mark_idx = url.indexOf('?');
if (mark_idx < 0) {
url += '?' + time_arg;
} else if (mark_idx === url.length - 1) {
url += time_arg;
} else {
url += '&' + time_arg;
}

I think that one possibility for disable caching, is by send some "no-cache" header on the HTTP respond by the kit server, and then Cloudflare or any other CDN will avoid caching this result.

dummdidumm added a commit that referenced this issue Sep 14, 2022
Add all headers except cache-control which is always no-store
Fixes #6458
Fixes #6477
chientrm added a commit to chientrm/kit that referenced this issue Sep 19, 2022
chientrm added a commit to chientrm/kit that referenced this issue Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
5 participants