kit icon indicating copy to clipboard operation
kit copied to clipboard

Fix browser caching of endpoints instead of page

Open oscarhermoso opened this issue 2 years ago • 5 comments

Describe the bug

Set up: Visit a page with client side navigation, where the destination +page.svelte file is in the same directory as a +server.js GET request handler, and a +page.js universal load function.

Then duplicate the tab.

Expected: An identical HTML page is rendered in the new browser tab. Actual: The JSON response from the +server.js is rendered.

This also happens when using back/forwards browser navigation

Reproduction

Minimal repo:

https://github.com/oscarhermoso/bug-sveltekit-duplicate-tab/tree/main

Recording:

bug-sveltekit-duplicate-tab

Logs

N/A

System Info

npx envinfo --system --binaries --browsers --npmPackages "{svelte,@sveltejs/*,vite}"

  System:
    OS: Linux 6.2 Pop!_OS 22.04 LTS
    CPU: (12) x64 AMD Ryzen 5 3600 6-Core Processor
    Memory: 4.62 GB / 15.56 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 16.14.2 - ~/.nvm/versions/node/v16.14.2/bin/node
    Yarn: 1.22.18 - /usr/local/bin/yarn
    npm: 8.5.0 - ~/.nvm/versions/node/v16.14.2/bin/npm
  Browsers:
    Chrome: 112.0.5615.165
    Chromium: 112.0.5615.49
    Firefox: 112.0.1
  npmPackages:
    @sveltejs/adapter-auto: ^2.0.0 => 2.0.1 
    @sveltejs/kit: ^1.5.0 => 1.15.9 
    svelte: ^3.54.0 => 3.58.0 
    vite: ^4.3.0 => 4.3.3

Severity

annoyance

Additional Information

This isn't related to any recent Sveltekit version - I am able to reproduce on Sveltekit 1.0.

It seems to be because of browser caching of the fetch request made on client load, because I can work-around the issue by adding Cache-Control headers to the GET request handler

setHeaders({
    'Cache-Control': 'No-Store',
  });

No response

oscarhermoso avatar Apr 27 '23 07:04 oscarhermoso

This sounds like the expected - if unfortunate in this case - behavior to me. Browsers seem to cache fairly aggressively when duplicating tabs. If the request does make it to SvelteKit, then it will render either the page or endpoint according to the Accept header - per https://kit.svelte.dev/docs/routing#server-content-negotiation. So if you want your endpoint's response to be cached not only by URL (which is the same as your page) but also by the Accept header currently being used to request it, you can specify the Vary: Accept header in your response if you don't want to forego caching altogether. I don't know that there's anything SvelteKit could reasonably do here.

Conduitry avatar Apr 27 '23 10:04 Conduitry

@Conduitry - Very nice, the Vary: Accept header resolves the issue and is definitely a better solution than 'Cache-Control': 'No-Store'. Perhaps it should be a default response header for routes with adjacent +page.svelte and +server.js files?

oscarhermoso avatar Apr 27 '23 10:04 oscarhermoso

It does seem useful, but I can't decide how I feel about quietly mutating the Response explicitly returned from an endpoint handler and returning a different response with an additional header. Maybe this behavior ought to be opt-in? I'll let other maintainers weigh in here.

Conduitry avatar Apr 27 '23 11:04 Conduitry

I think it makes perfect sense for the Vary: Accept header to be implicitly added where we serve different data on the same URL path.

For example, Django REST Framework does this for their REST API - which has a friendly HTML interface when viewed in the browser.

https://github.com/encode/django-rest-framework/blob/1ce0853ac51635526dc1a285e6b83c9848002f0e/rest_framework/views.py#L153-L160

If the header was included in Sveltekit responses, we might need to address this block too:

https://github.com/sveltejs/kit/blob/526a2ed5676905807d83ace90d3853027f17f265/packages/kit/src/runtime/server/page/serialize_data.js#LL91C1-L102C1

oscarhermoso avatar Apr 28 '23 00:04 oscarhermoso

It does seem useful, but I can't decide how I feel about quietly mutating the Response explicitly returned from an endpoint handler and returning a different response with an additional header. Maybe this behavior ought to be opt-in? I'll let other maintainers weigh in here.

It makes sense to add it in when content-negotiation is happening. It'll be good to document it as well so that it can be opt-out / overridden / removed ?

teemingc avatar Apr 29 '23 11:04 teemingc