feat: allow additional handlers in adapter-cloudflare-workers

[thomasfosterau]

closes #10117
closes #1712

Cloudflare Workers (and Pages, while it’s still a separate product) allows for handlers to be exported from a worker as methods on an object exported as a default export. Currently, adapter-cloudflare-workers exports a fetch handler method, but there is no way to add other handler methods, such as a scheduled or queue handler. This functionality was requested just over a year ago in #10496.

This PR adds a configuration option for @sveltejs/adapter-cloudflare-workers to provide a file whose exports get added to the default option exported by the module the adapter generates. The fetch handler the two adapter uses does not get overridden.

• What’s the best name for this option? export? handlers?
• Should the adapters expect the file to export names exports or a single default export? What about both (this might allow Durable Objects)?

Currently pnpm check is failing because the worktop dependency has type definitions for the exported handler type that are very out of date.

Edit: Cloudflare Pages does not, in fact, support additional handlers.

---

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.

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.

[changeset-bot]

🦋 Changeset detected

Latest commit: e71366e

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	Minor

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

[eltigerchino]

Does exporting handlers other than fetch work for the Cloudflare Pages function? I couldn't find any explicit mention that it doesn't from the official documentation.

• Community answer says no
• Pages docs doesn't explicitly say no

cc: @dario-piotrowicz @jamesopstad @petebacondarwin

What’s the best name for this option? export? handlers?

I think "handlers" works well since that's what they're called in the official docs https://developers.cloudflare.com/workers/runtime-apis/handlers/

We should add some documentation on adding these handlers to our Cloudflare adapter docs https://github.com/sveltejs/kit/blob/main/documentation/docs/25-build-and-deploy/70-adapter-cloudflare-workers.md

Should the adapters expect the file to export names exports or a single default export? What about both (this might allow Durable Objects)?

The _worker.js file needs to be the one that exports the durable object classes for it to work. See #1712 (comment)

[thomasfosterau]

Does exporting handlers other than fetch work for the Cloudflare Pages function?

I’ll admit I hadn’t actually checked because from the Cloudflare documentation I assumed _worker.js was just a normal worker (as opposed to normal Cloudflare Pages Functions, _worker.js is supposed to be in ‘advanced mode’ and act like a regular Cloudflare Worker)... and the answer seems to be no for a scheduled handler at least.

I agree the the Cloudflare documentation doesn’t explicitly say it’s not supported, at least because it does say you can copy-paste any worker into a _worker.js file -- but I just tried that and it refuses to deploy it 🤷🏼‍♂️ Cloudflare’s documentation around this could probably be a bit more explicit.

I can update the PR to remove the functionality from the @sveltejs/adapter-cloudflare adapter and leave it on the Cloudflare Workers one.

We should add some documentation on adding these handlers to our Cloudflare adapter docs

I will can write some and include it in this PR, might have to be post-Christmas though.

The _worker.js file needs to be the one that exports the durable object classes for it to work.

If this is desirable I can update the PR to do it as well. Given the handlers are just properties on the default export object they were fairly easy to do, the Durable Object classes would just need to be exported using export * from '';. I assume there aren’t any footguns with export * from '';, e.g. no way for someone to accidentally overwrite the default export?

[eltigerchino]

I can update the PR to remove the functionality from the @sveltejs/adapter-cloudflare adapter and leave it on the Cloudflare Workers one.

I will can write some and include it in this PR, might have to be post-Christmas though.

Thanks! Take your time.

The _worker.js file needs to be the one that exports the durable object classes for it to work.

If this is desirable I can update the PR to do it as well. Given the handlers are just properties on the default export object they were fairly easy to do, the Durable Object classes would just need to be exported using export * from '';. I assume there aren’t any footguns with export * from '';, e.g. no way for someone to accidentally overwrite the default export?

I think they might also have to be named exports but I haven't tested any of this myself.

[Rich-Harris]

preview: https://svelte-dev-git-preview-kit-13207-svelte.vercel.app/

this is an automated message

[thomasfosterau]

I’ve removed the handlers code from adapter-cloudflare and added named exports for Durable Objects, as writing some documentation. Whatever type incompatibility was causing the checks to fail was in adapter-cloudflare.

For future reference, the ‘compatibility matrix’ for Workers versus Pages is here: https://developers.cloudflare.com/workers/static-assets/compatibility-matrix/.

[kansson]

I’ve removed the handlers code from adapter-cloudflare and added named exports for Durable Objects, as writing some documentation. Whatever type incompatibility was causing the checks to fail was in adapter-cloudflare.

For future reference, the ‘compatibility matrix’ for Workers versus Pages is here: https://developers.cloudflare.com/workers/static-assets/compatibility-matrix/.

The pages adapter can also be used when deploying workers with static assets. The official create-cloudflare package uses this adapter by default instead of the workers adapter see here: https://github.com/cloudflare/workers-sdk/tree/main/packages/create-cloudflare/templates-experimental/svelte

It appears that you must use the pages adapter with static assets because the workers adapter requires the site.bucket option, which is incompatible with the assets.directory option.

[thomasfosterau]

@eltigerchino I definitely think it's doable, but the behaviour will need to be documented, yes.

FWIW, all the adapter needs to do is either do what this PR already does, or re-export the class extending WorkerEntrypoint with an overwritten fetch handler. We don't need to worry about class instances or RpcTarget.

[thomasfosterau]

It’s a little ugly but the changes now allow the fetch handler to be added to a WorkerEndpoint when it is detected. To be honest, the use cases for actually wanting to do this are slim (not sure you can access a WorkerEndpoint except via other Workers?) but it does work and match what Cloudflare allows.

[philippviereck]

I'm glad this is coming.
I tried to run this locally but it does not seem to just work - yet.
I run into this:

service core:user:__WRANGLER_EXTERNAL_DURABLE_OBJECTS_WORKER: Worker "core:user:__WRANGLER_EXTERNAL_DURABLE_OBJECTS_WORKER"'s binding "MyDurableObject" refers to a service "core:user:worker", but no such service is defined.

which I think should be solved by: cloudflare/workers-sdk#7292
Here's my reprod-repo: https://github.com/philippviereck/local-sveltekit-do

For the time being, I failed to get the "dual wrangler dev" setup to work. Even though I had it running it still didn't connect. But this might be my fault. But I never got around:

MyDurableObject undefined[wrangler] Couldn't find `wrangler dev` session for class "MyDurableObject" to proxy to

but that could be a workaround

And am I correct that handlers.js/ts needs to be explicitly specified? I think that's missing in the docs (of this PR)

[philippviereck]

Okey I updated the example repo.

Using this PR:

https://github.com/philippviereck/local-sveltekit-do
Issues:

• "dev" does not work (pnpm run dev)
• "build" (deploy) does not work (pnpm run build)

Both throw:

service core:user:__WRANGLER_EXTERNAL_DURABLE_OBJECTS_WORKER: Worker "core:user:__WRANGLER_EXTERNAL_DURABLE_OBJECTS_WORKER"'s binding "MyDurableObjectLocal" refers to a service "core:user:worker", but no such service is defined.

NOT Using this PR (workaround):

https://github.com/philippviereck/local-sveltekit-do/tree/workaround

Issues:

• Needs an extra wrangler process and all the "overhead" that comes with it. Like: two wrangler.toml and so on
• RPC does not work

Error: Cannot access `MyDurableObject#sayHello` as Durable Object RPC is not yet supported between multiple `wrangler dev` sessions.

---

EDIT: In both repos I only tested Durable Objects. I did not once try the other worker handlers! This is all only regarding DOs

[thomasfosterau]

@philippviereck when I get a chance I'll take a look at your repo and see what I can do (currently without reliable internet, which is a side-effect of moving house in Australia 🙃).

Do any of the Wrangler commands work, such as wrangler dev or wrangler deploy --dry-run? My initial thoughts are that this might be limitation of using vite to run preview/dev locally. I've run into similar issues before. If so, it's almost certainly out of the scope of this PR to fix (and might even be more of a vite issue?).

[thomasfosterau]

which I think should be solved by: cloudflare/workers-sdk#7292

Yes I think that PR needs to land to solve the Durable Objects issue. If that fixes the error you're encountering then everything's all good 😄

And am I correct that handlers.js/ts needs to be explicitly specified? I think that's missing in the docs (of this PR)

Yes, it needs to be explicitly specified in order to be used. Not sure if having a default is a good idea. What do you think needs to be improved in the docs for this PR?

[half2me]

This is some great work! I've run into this issue recently while trying to play around with DOs but being unable to find a way to deploy them unless I made a separate project, and even then nothing worked locally :(

At this point are we waiting on cloudflare?

[eltigerchino]

This is some great work! I've run into this issue recently while trying to play around with DOs but being unable to find a way to deploy them unless I made a separate project, and even then nothing worked locally :(

At this point are we waiting on cloudflare?

Can you share a minimal reproduction?

[thomasfosterau]

This is some great work! I've run into this issue recently while trying to play around with DOs but being unable to find a way to deploy them unless I made a separate project, and even then nothing worked locally :(
At this point are we waiting on cloudflare?

Can you share a minimal reproduction?

If I’m not mistaken, @half2me is referring to the same issue as @philippviereck, which is a Cloudflare issue: cloudflare/workers-sdk#7292. Not much we can do about it other than wait for Cloudflare to fix it, but given it really only affects local previews where Durable Objects are used, it might be tolerable?

[philippviereck]

'vite build' does error too, if I remember correctly. I will look at this today again

[svelte-docs-bot]

Preview: https://svelte-dev-git-preview-kit-13207-svelte.vercel.app/

[thomasfosterau]

@philippviereck I had a look at your repo - the issue is definitely that getPlatformProxy just doesn’t support Durable Objects when they’re defined in the same worker. Nothing any of us can do except wait for the Wrangler pull request fixing that to be finished and merged... but at the moment it’s had no commits for over two months and checks are currently failing.

@eltigerchino any objection to me removing the Durable Objects parts of this PR so the rest can be merged?

[philippviereck]

Apologies for the delayed response.

Documentation Improvements

The current documentation doesn't clearly state that handlers need to be explicitly specified. At minimum, we should add a note that the default value is undefined.

Proposed Enhancement: Standardized Naming Convention

Rather than just documenting the default behavior, I believe we should adopt a standardized naming convention consistent with other platform-specific integrations (like Vercel, Netlify, and Node - see PRs #13430 and #13477).

With this approach, a hooks.cloudflare.js/ts file would be automatically respected when using adapter-cloudflare-workers, similar to how hooks.server.js/ts works with the sveltekit server.

hooks.vercel.ts, hooks.netlify.ts, and hooks.node.ts would work respectively

...at least that's what I believe would create a clean API.

Short-term Solution

To avoid further delays, I suggest: Instead of implementing a consistent naming convention immediately, we could change the default value from undefined to a default handler/hooks file. This would be similar to how wrangler.jsonc is implicitly expected when using the CF Workers adapter.

Naming Considerations

Ideally, hooks.cloudflare.js/ts would work the same for both Workers and Pages, but currently they differ in too many features. I don't think it's the Svelte adapter's job to bridge or abstract away these differences, which raises the question of whether hooks.cloudflare.js/ts might be too confusing and should instead be hooks.cf-workers.js/ts or similar. Also, I use hooks to be consistent with the SvelteKit naming, but maybe handlers is better as that's what CF calls them.

---

@threepointone I'm tagging you as you've been influential in the Durable Objects space. This integration would be valuable for SvelteKit × DO users - I myself became a DO user specifically through using SvelteKit, as Cloudflare was the only adapter with a free commercial tier. I later upgraded to paid and started using the whole CF dev platform, but the integration discrepancies between SvelteKit and Cloudflare have always been a major pain point. There have been many improvements to the local dev and overall Cloudflare × SvelteKit experience, but this enhancement would finally make their harmony complete - so maybe you can bring this up internally 😇

[emily-shen]

Hey, I’m from the Cloudflare team - sorry for the delay on the durable object issue, we are actively working on it now, but I can’t really estimate a timeline and I don’t want to hold up this PR 😅

If you can leave the DO part for a followup PR maybe, I can comment there once I've got a PR up with a wrangler pre-release to test with? Or let me know how you else you’d like to go about it :)

Having a handlers.ts (or whatever it ends up being called) file that the user writes would work well though for what I think our solution will be - we’ll probably let you pass this into getPlatformProxy and that should give you access to Durable objects (and workflows or whatever) in local dev.

[thomasfosterau]

@philippviereck I’ve make some small changes to the documentation -- is there anything further specifically you think needs to be mentioned or that is unclear?

Rather than just documenting the default behavior, I believe we should adopt a standardized naming convention consistent with other platform-specific integrations (like Vercel, Netlify, and Node - see PRs #13430 and #13477).

I’m a bit skeptical that this is a good way to go about things, at least as far as this PR is concerned. What this PR does is to add the fetch handler created by the Cloudflare Workers adapter to a file specified by the user which is already an otherwise functional Cloudflare Worker. That’s quite a bit different to how hooks currently work and different to how the various middleware proposals would work -- I think if there is a Cloudflare Workers middleware PR that comes along, it probably shouldn’t use the handlers file this PR uses.

To avoid further delays, I suggest: Instead of implementing a consistent naming convention immediately, we could change the default value from undefined to a default handler/hooks file. This would be similar to how wrangler.jsonc is implicitly expected when using the CF Workers adapter.

I personally think the best default here is just to not include any handlers unless they’re explicitly specified. There isn’t really a pre-existing default handler file being used by anyone, unlike wrangler.jsonc which is defined externally by Wrangler. Maybe if a common filename emerges then a future PR could make it the default (that’s probably a breaking change anyway).

Ideally, hooks.cloudflare.js/ts would work the same for both Workers and Pages, but currently they differ in too many features. I don't think it's the Svelte adapter's job to bridge or abstract away these differences, which raises the question of whether hooks.cloudflare.js/ts might be too confusing and should instead be hooks.cf-workers.js/ts or similar. Also, I use hooks to be consistent with the SvelteKit naming, but maybe handlers is better as that's what CF calls them.

I don’t think the additional Cloudflare handlers are really analogous to SvelteKit’s hooks, so it’s probably best to avoid conflating the two. I’d be interested to see if anyone thinks worker would be a better name for the option than handlers, but that might also create confusion.

Happy to be corrected on this, but I also believe Cloudflare is (slowly) merging Pages and Workers into one product. I wouldn’t be surprised if the outcome of that eventually happening for SvelteKit is that there is eventually just a single @sveltejs/adapter-cloudflare adapter which would probably be mostly based on the current @sveltejs/adapter-cloudflare-workers package.

[thomasfosterau]

Hey, I’m from the Cloudflare team - sorry for the delay on the durable object issue, we are actively working on it now, but I can’t really estimate a timeline and I don’t want to hold up this PR 😅

Thanks for the update @emily-shen!

If you can leave the DO part for a followup PR maybe, I can comment there once I've got a PR up with a wrangler pre-release to test with? Or let me know how you else you’d like to go about it :)

I’ve updated this PR to remove anything to do with Durable Objects for now -- for me the scheduled and queue handlers are valuable enough to include without Durable Objects.

Having a handlers.ts (or whatever it ends up being called) file that the user writes would work well though for what I think our solution will be - we’ll probably let you pass this into getPlatformProxy and that should give you access to Durable objects (and workflows or whatever) in local dev.

Just to clarify, at the moment this PR works by merging the user’s handlers file into the Cloudflare Worker already created by this SvelteKit adapter (which just exports a fetch handler). If I’m not mistaken, SvelteKit won’t have to change how it uses getPlatformProxy to take advantage of local Durable Objects support when it lands?

[philippviereck]

#13207 (comment)

I agree with you on all points.

You make a good point about naming the option "worker," but since DOs aren't yet supported, "handlers" is more accurate.