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

feat: use Workers Static Assets #13427

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

astralhpi
Copy link

@astralhpi astralhpi commented Feb 6, 2025

resolves #13360
resolves #13071
resolves #13005
resolves #12813

This PR builds upon the excellent work by @petebacondarwin in PR #13072, focusing on incorporating reviewer feedback and refining the documentation.

Since the original PR seemed to have stalled, I wanted to help move things forward by addressing the feedback. I didn’t make any code changes—just focused on improving the documentation.
Huge thanks to @petebacondarwin for the initial implementation and to the reviewers for their insights.

Looking forward to any additional feedback to get this merged!

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it. These adapters don't appear to have tests.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

petebacondarwin and others added 9 commits November 28, 2024 11:56
This changes the adapter to stop using the old Workers Sites (kv-asset-handler) approach.
Instead, making use of the new Workers Static Assets feature, which is embedded into Cloudflare natively.

Also this change removes the extra esbuild step that was being run inside the adapter, relying upon Wrangler to do the bundling.
The extra esbuild step required a hardcoded list of Node.js compatible modules.
This is no longer needed since Wrangler now manages all of that.

- This version of the adapter requires Wrangler version 3.87.0 or later.

  Run `npm add -D wrangler@latest` (or similar) in your project to update Wrangler.
- The user's Wrangler configuration (`wrangler.toml`) must be migrated from using Workers Sites to using Workers Assets.

  Previously a user's `wrangler.toml` might look like:

  ```toml
  name = "<your-site-name>"
  account_id = "<your-account-id>"
  compatibility_date = "2021-11-12"
  main = "./.cloudflare/worker.js"

  # Workers Sites configuration
  site.bucket = "./.cloudflare/public"
  ```

  Change it to to look like:

  ```toml
  name = "<your-site-name>"
  account_id = "<your-account-id>"
  compatibility_date = "2021-11-12"`
  main = ".svelte-kit/cloudflare/server/index.js"

  # Workers Assets configuration
  assets = { directory = ".svelte-kit/cloudflare/client" }
  ```

- Workers Assets defaults to serving assets directly for a matching request, rather than routing it through the Worker code.

  The previous adapter would add custom headers to assets responses (such as `cache-control`, `content-type`, and `x-robots-tag`. Such direct asset responses no longer contain these headers - but the will include eTag headers that have proven (in Pages) to be an effective caching strategy for assets.

  If you wish to always run the Worker before every request then add `serve_directly = false` to the assets configuration section. For example:

  ```toml
  assets = { directory = ".svelte-kit/cloudflare/client", serve_directly = false }
  ```
- Removed detailed examples to keep the entry concise.
- Provided documentation links for reference.
Copy link

changeset-bot bot commented Feb 6, 2025

🦋 Changeset detected

Latest commit: b278361

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/adapter-cloudflare-workers Major

Not sure what this means? Click here to learn what changesets are.

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

@svelte-docs-bot
Copy link

@eltigerchino eltigerchino added documentation Improvements or additions to documentation pkg:adapter-cloudflare-workers and removed documentation Improvements or additions to documentation labels Feb 7, 2025
@eltigerchino eltigerchino changed the title docs: Follow-up on PR #13072: Enhancements to @sveltejs/adapter-cloudflare-worker Based on Review Feedback feat: use Workers Static Assets Feb 7, 2025
@eltigerchino
Copy link
Member

I don't think the original PR stalled. I'm guessing it's just not ready to be merged or to be continued yet since the Workers Static Assets feature is still in beta. @petebacondarwin what are your thoughts on this? Should we try to get support out during the beta period?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants