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: allow additional handlers and durable objects #13207

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/thirty-ghosts-cover.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@sveltejs/adapter-cloudflare-workers': minor
'@sveltejs/adapter-cloudflare': minor
---

feat: allow additional handlers to be included in generated worker
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ export default {

Path to your custom `wrangler.toml` or `wrangler.json` config file.

### handlers
eltigerchino marked this conversation as resolved.
Show resolved Hide resolved

Path to a file with additional [handlers](https://developers.cloudflare.com/workers/runtime-apis/handlers/) and (optionally) [Durable Objects](https://developers.cloudflare.com/durable-objects/) to be exported from the file the adapter generates. This allows you to, for example, include handlers for scheduled or queue triggers alongside the fetch handler your SvelteKit app.
eltigerchino marked this conversation as resolved.
Show resolved Hide resolved

eltigerchino marked this conversation as resolved.
Show resolved Hide resolved
> [!NOTE] The adapter expects the `handlers` file to have a default export. If you only want to export a Durable Object, add `export default {};` to the file.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell, the bundler should throw an error at build time if there isn’t a default export in the handlers file. I can’t see a nice way to handle there not being a default export, even an empty object is a sensible default/fallback.

Copy link
Member

@eltigerchino eltigerchino Dec 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we split the durable objects into a separate file/option to avoid this pitfall?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think so, although it will be a minor annoyance to anyone only using Durable Objects. It will be a documented requirement and there will be an error if the default export is missing.

Removing the requirement of a default export in the future wouldn’t be a breaking change if there was an easy way at build time to check whether there’s a default export and modify the generated worker accordingly.

Copy link
Member

@eltigerchino eltigerchino Dec 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may have to add a check ourselves since the bundling step will be removed and delegated to Cloudflare's wrangler (meaning the error would only surface when deploying).

Is it possible to do something like:

if (handler) {
  const module = await import(handler);

  if (!('default' in module)) {
    throw new Error('The adapter expects the `handlers` file to have a default export. If you only want to export a Durable Object, add `export default {};` to the file.');
  }
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would assume that would throw an error at build time if the handlers file is written in, e.g., TypeScript or if the handlers file references a module not available at build time (e.g., something available in Cloudflare Workers but not Node). Only trivial workers aren't going to have this problem.

I think this is just an unavoidable problem if bundling is delegated to Wrangler. A possible solution would be to parse the handlers file and look for a default export in the AST, but that seems overly complicated.


> [!NOTE] The adapter will overwrite any [fetch handler](https://developers.cloudflare.com/workers/runtime-apis/handlers/fetch/) exported from the `handlers` file in the generated worker. Most uses for a fetch handler are covered by endpoints or server hooks, so you should use those instead.
thomasfosterau marked this conversation as resolved.
Show resolved Hide resolved

### platformProxy

Preferences for the emulated `platform.env` local bindings. See the [getPlatformProxy](https://developers.cloudflare.com/workers/wrangler/api/#syntax) Wrangler API documentation for a full list of options.
Expand Down
5 changes: 5 additions & 0 deletions packages/adapter-cloudflare-workers/files/entry.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { Server } from 'SERVER';
import { manifest, prerendered, base_path } from 'MANIFEST';
import handlers from 'HANDLERS';
import { getAssetFromKV, mapRequestToAsset } from '@cloudflare/kv-asset-handler';
import static_asset_manifest_json from '__STATIC_CONTENT_MANIFEST';

export * from 'HANDLERS';
thomasfosterau marked this conversation as resolved.
Show resolved Hide resolved

const static_asset_manifest = JSON.parse(static_asset_manifest_json);

const server = new Server(manifest);
Expand All @@ -12,6 +16,7 @@ const immutable = `${app_path}/immutable/`;
const version_file = `${app_path}/version.json`;

export default {
...handlers,
/**
* @param {Request} req
* @param {any} env
Expand Down
4 changes: 4 additions & 0 deletions packages/adapter-cloudflare-workers/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ export default function plugin(options?: AdapterOptions): Adapter;

export interface AdapterOptions {
config?: string;
/**
* Path to a file with additional {@link https://developers.cloudflare.com/workers/runtime-apis/handlers/ | handlers} and (optionally) {@link https://developers.cloudflare.com/durable-objects/ | Durable Objects} to be exported from the file the adapter generates.
*/
handlers?: string;
/**
* Config object passed to {@link https://developers.cloudflare.com/workers/wrangler/api/#getplatformproxy | getPlatformProxy}
* during development and preview.
Expand Down
22 changes: 19 additions & 3 deletions packages/adapter-cloudflare-workers/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { existsSync, readFileSync, writeFileSync } from 'node:fs';
import { posix, dirname } from 'node:path';
import { posix, dirname, resolve } from 'node:path';
import { execSync } from 'node:child_process';
import { cwd } from 'node:process';
import esbuild from 'esbuild';
import toml from '@iarna/toml';
import { fileURLToPath } from 'node:url';
Expand Down Expand Up @@ -32,7 +33,7 @@ const compatible_node_modules = [
];

/** @type {import('./index.js').default} */
export default function ({ config = 'wrangler.toml', platformProxy = {} } = {}) {
export default function ({ config = 'wrangler.toml', platformProxy = {}, handlers } = {}) {
return {
name: '@sveltejs/adapter-cloudflare-workers',

Expand All @@ -58,7 +59,8 @@ export default function ({ config = 'wrangler.toml', platformProxy = {} } = {})
builder.copy(`${files}/entry.js`, `${tmp}/entry.js`, {
replace: {
SERVER: `${relativePath}/index.js`,
MANIFEST: './manifest.js'
MANIFEST: './manifest.js',
HANDLERS: './_handlers.js'
}
});

Expand All @@ -78,6 +80,20 @@ export default function ({ config = 'wrangler.toml', platformProxy = {} } = {})
`export const base_path = ${JSON.stringify(builder.config.kit.paths.base)};\n`
);

if (handlers) {
// TODO: find a more robust way to resolve files relative to svelte.config.js
const handlers_file = resolve(cwd(), handlers);
thomasfosterau marked this conversation as resolved.
Show resolved Hide resolved
writeFileSync(
`${tmp}/_handlers.js`,
`import handlers from "${handlers_file}";\n\n` +
`export * from "${handlers_file}";\n\n` +
'export default handlers;'
);
} else {
// The handlers file must export a plain object as its default export.
writeFileSync(`${tmp}/_handlers.js`, 'export default {};');
}

const external = ['__STATIC_CONTENT_MANIFEST', 'cloudflare:*'];
if (compatibility_flags && compatibility_flags.includes('nodejs_compat')) {
external.push(...compatible_node_modules.map((id) => `node:${id}`));
Expand Down
8 changes: 8 additions & 0 deletions packages/adapter-cloudflare-workers/placeholders.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,11 @@ declare module '__STATIC_CONTENT_MANIFEST' {
const json: string;
export default json;
}

declare module 'HANDLERS' {
import { ExportedHandler } from '@cloudflare/workers-types';

const handlers: Omit<ExportedHandler, 'fetch'>;

export default handlers;
}
Loading