[feat] Add configurable cache to worker responses

[lukaswelinder]

New breaking change here

• Who does this affect: Anyone using the cloudflare adapter.
• How to migrate: Users that want to maintain the existing behavior will have to set all cache options to null. Otherwise defaults will apply. Any static assets without file extensions will break.
• Why make this breaking change: The existing implementation of the cloudflare worker adapter has slow page response and does not return cache-control headers for static assets like the default does, so browser cache is not utilized for static assets.
• Severity (number of people affected x effort): Fairly low impact. Very simple to revert to original behavior.

I have been using svelte-kit to rebuild my personal site/blog and noticed a few areas for improvement.

Slow page response

The problem here is having to wait for asset fetch to fail before moving on to try to render. The solution I put in place is more of a stop-gap. It is just checking for a file extension at the end of the request path.

By adding this gate, page request time dropped by 60-80%. However, static assets without file extensions will break. I'd like to improve upon my approach and would appreciate feedback here.

Static assets are not returned with cache-control header

The default svelte-kit adapter returns this header so that browser cache can be utilized.

Refer to the updated README documenting the properties and their purpose.

Testing

I did not see tests in any of the adapters, don't mind writing some, but would appreciate input here.

That aside. I'm currently using this adapter code on a production site without issues.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 2c2e083

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[lukaswelinder]

I'd like to point out a couple things regarding the current implementation of detecting static asset requests.

My implementation here just checks for GET requests with a file extension. This means a couple things:

• Files without extensions will be broken (as mentioned above). Ideally you shouldn't do this in the first place, but I digress.
• Request routes with .json extension will still experience latency. Not ideal, but we're still in a better place overall.

Per the TODO comment, I investigated using the manifest to identify requests for static assets, but found this to be problematic. Perhaps it is an issue with my individual build tooling or configuration, but the manifest only includes js/css assets.

I think a better approach would be to recursively scan the _app output directory. Maybe there is already a way to do this via the adapter API, but I'm just not familiar enough with it. I can explore this further when I have spare time later this week, but input is welcome.

[benmccann]

I think we should look for _app rather than file extension. Take a look at #1416 which does that for adapter-node

[lukaswelinder]

@benmccann Agreed. I can implement this over the weekend so that we can get this in.

I have a question about another feature enhancement though. I'd like to add support for a cloudflare worker hook. Should I implement that in this PR or would you prefer that I open another once this has been merged? Thanks.

[benmccann]

I'd like to add support for a cloudflare worker hook. Should I implement that in this PR or would you prefer that I open another once this has been merged?

It's probably easier to review as separate PRs

[Rich-Harris]

Thank you!

Pages probably shouldn't have their own option here. There's already a place to set headers globally, and that's handle. I don't think we should have another Cloudflare-specific way to do that.

The regex stuff is definitely a no-go :) Could you remove that so that this PR is purely about caching? The way to check if a request is for an asset would be to check the path against a list of them. We have such a thing here...

kit/packages/kit/src/core/build/index.js

Line 74 in 3e30a1b

static: options.manifest.assets.map((asset) => posixify(asset.file)),

...we'd just need to figure out the right way to expose it along with a) the list of generated _app files and b) the list of prerendered pages, if any. That could happen in a separate PR.

Speaking of a), since those generated files are hashed, they can be treated as immutable, and should probably automatically get a public, max-age=31536000, immutable cache-control header.

Speaking of b), we might need a way to differentiate between prerendered pages and other assets in the KV store. (We might want to distinguish between different assets altogether — e.g. a longer TTL for /videos/* than /other-stuff/*, but that's probably overkill.)

So in summary, perhaps an API like this makes sense?

import adapter from '@sveltejs/adapter-cloudflare-workers';

export default {
  kit: {
    adapter: adapter({
      browserTTL: {
        prerendered: 60,
        static: 24 * 60 * 60
      },
      edgeTTL: {
        prerendered: 60,
        static: 24 * 60 * 60
      }
    })
  }
};

[pheuter]

Any plans to get this functionality over the line? Been a few months since last activity.

[Defman]

I'm happy to take a stab at this.

I'm going to simply export the manifest const from the ../output/server/app.js file and add _app, which will contain the same content as client from

kit/packages/kit/src/core/build/index.js

Line 72 in 3e30a1b

client,

_app field should ofc be a set.

import { init, render, manifest } from '../output/server/app.js';

if (manifest._app[path])
    // Should cache immutable
else if (manifest.assets[path])
    // Should cache but not immutable
...

I'm not sure how I should handle prerendered pages.

[Rich-Harris]

Reflecting further on this, many adapters can and should set cache-control headers for assets, so there should be a universal way to configure it and an adapter API for using that configuration. I've opened #3194, so I'll close this — thanks