kit icon indicating copy to clipboard operation
kit copied to clipboard

+page.server.js no longer uses/respects setHeaders

Open homerjam opened this issue 3 years ago • 6 comments

Describe the bug

Since this commit the +page.server.js endpoints return a js file rather than json. This js file response no longer uses the headers set with the setHeaders function. This means we can't use cache-control etc.

Reproduction

To reproduce, hover over the 'TODOS' menu item and observer the '__data.js' response headers. There should be a header as defined in routes/todos/+page.server.js;

https://stackblitz.com/edit/sveltejs-kit-template-default-8tt4be?file=src/routes/todos/+page.server.js

Screenshot 2022-08-31 at 17 29 32

Logs

No response

System Info

System:
    OS: macOS 12.5
    CPU: (8) arm64 Apple M2
    Memory: 155.45 MB / 24.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.16.0 - ~/.nvm/versions/node/v16.16.0/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.11.0 - ~/.nvm/versions/node/v16.16.0/bin/npm
  Browsers:
    Chrome: 104.0.5112.101
    Safari: 15.6
  npmPackages:
    @sveltejs/adapter-auto: 1.0.0-next.70 => 1.0.0-next.70 
    @sveltejs/kit: 1.0.0-next.456 => 1.0.0-next.456 
    svelte: ^3.49.0 => 3.49.0 
    vite: 3.1.0-beta.1 => 3.1.0-beta.1

Severity

annoyance

Additional Information

No response

homerjam avatar Aug 31 '22 16:08 homerjam

I noticed this in particular when trying to overwrite some headers with throw redirect(301, '/todos'). In particular want the redirect to look like it is external not internal; no luck.

example

setHeaders({
    'sec-fetch-mode': 'navigate',
    'sec-fetch-dest': 'document',
})
...

throw redirect(301,'/todos')

Doesn't work, actual headers

    'sec-fetch-mode' => 'cors',
    'sec-fetch-dest' => 'script'

TranscendentalRide avatar Sep 05 '22 23:09 TranscendentalRide

This is very frustrating as it means making an unnecessary request to the page endpoint when going 'back' a page. As a workaround is there a way to intercept this somehow and prevent it happening (using an in memory store somewhere instead)?

homerjam avatar Sep 14 '22 11:09 homerjam

Not sure how to solve this with regards to cache headers. Other headers seem easy, just add them back. Cache headers however are in conflict with other feature such as await parent() - how to ensure that what you request is really something for which a cache hit should occur? If the parent +layout.server.js changed, or your +page.server.js depends on url, setting cache headers would result in wrong cache hits. One could argue that's on the developer to ensure cache headers are set appropriately, but I'm not sure this is just a lazy answer.

dummdidumm avatar Sep 14 '22 14:09 dummdidumm

The issue right now is that we can't add headers at all (we also seemingly can't prevent calling the page endpoint unnecessarily even if the data is held in memory). I think it's sensible not to add cache headers by default and leave that to the developer. Caching is, in general, a nightmare of complexity but I just want a simple 15min max-age to make the once-a-day visitor to this portfolio site have a slightly better experience.

Side note, but also now that the request goes to __data.ja it doesn't appear in the fetch/xhr panel in dev tools (where I would expect it!).

homerjam avatar Sep 14 '22 14:09 homerjam

Cache headers won't work with __data.js, because SvelteKit needs to add a ?__id={x} query parameter otherwise it will always use stale data. Since it's using import rather than fetch, it can't access the headers, and so can't not add __id. Furthermore, if the response was cached then you might reload the page and start getting the wrong data entirely, just because __id matched a previous response for a different URL.

The solution here is for SvelteKit to read cache headers when generating the __data.js file and add something along these lines:

window.__sveltekit_ttl = 300;

The client can read that value and cache the data for 300 seconds, reusing it if the user navigates back to the same URL. (Though there is a footgun here — if the data was mutated, the mutation would persist.)

Rich-Harris avatar Sep 22 '22 15:09 Rich-Harris

Would this solution work per page?

Where would SvelteKit read the headers from exactly?

Could we add __ttl to the object returned by the load function in the *.server.js file to make things more explicit?

I'm basically looking to avoid requesting/importing the data if I've already visited the page. I hope I'm understanding this correctly - but it seems like we're on the same page (no pun intended : )

homerjam avatar Sep 22 '22 16:09 homerjam

Another thought I just had: What if the id was made up of two things? One unique per page session (full page reload -> new ID), one unique per request. The latter is stored in some url->id map, so if you go back to an already visited page that id is reused. That way the data request is unique per full-page-reload but reusable per client-side page visit:

const urlId = map.get(url) ?? id++;  // maybe we can even use event.routeId here and spare us the map?
//...
__data.js?__id={uniqueId}_{urlId}

This then enables us to store the cache header on the request.

dummdidumm avatar Sep 23 '22 07:09 dummdidumm

Where would SvelteKit read the headers from exactly?

If you call setHeaders({ 'cache-control': 'max-age: 300' }) in one of your load functions, SvelteKit would turn that into window.__sveltekit_ttl = 300. Very similar to this... https://github.com/sveltejs/kit/blob/2c6f53792f986b712098794d7c6c835ed13e20a8/packages/kit/src/runtime/server/page/serialize_data.js#L78-L84 ...which allows fetch to be aware of cache headers applied to responses that are inlined into the HTML.

This would only re-use data for back/forward navigations — it wouldn't help if you visited /a, navigated to /b, then in a new session visited /a then /b/b/__data.js would be generated in both cases. If we wanted to make that work, I think we'd have to rethink the decision to use devalue.

@dummdidumm that wouldn't work because the data would be stale when revisiting a URL — you wouldn't be loading data from the server, you'd be loading it from the module cache (except you wouldn't, because we delete window.__sveltekit_data to prevent memory leaks)

Rich-Harris avatar Sep 23 '22 19:09 Rich-Harris

If you call setHeaders({ 'cache-control': 'max-age: 300' }) in one of your load functions, SvelteKit would turn that into window.__sveltekit_ttl = 300

This would only re-use data for back/forward navigations

Great. If I'm understanding correctly this seems to solve my issue/goal precisely. In a real world scenario I wouldn't expect a visitor to refresh the page/start a new session and even if they I think it's fine to not use the browser cache.

But I'm still wondering if this works per page/route? For instance there may be some pages that I would like to be "refreshed" each time (although I guess it might be possible to manually trigger an update onmount for example each time the page is visited). Does the __sveltekit_ttl value get applied/removed each navigation?

homerjam avatar Sep 23 '22 20:09 homerjam

It would apply per-URL, and data would be reused until it was stale. Manual invalidation would override ttl

Rich-Harris avatar Sep 23 '22 20:09 Rich-Harris