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 the new Workers Static Assets feature from Cloudflare #13072

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
53 changes: 53 additions & 0 deletions .changeset/smart-owls-trade.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
---
'@sveltejs/adapter-cloudflare-workers': major
---

feat: use the new Workers Static Assets feature from Cloudflare

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.
Comment on lines +7 to +8
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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.
This changes the adapter to stop using the old Workers Sites (kv-asset-handler) approach in favor 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.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Also this change removes the extra esbuild step that was being run inside the adapter, relying upon Wrangler to do the bundling.
Also, this change removes the extra esbuild step that was being run inside the adapter, instead 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.
Comment on lines +11 to +12
Copy link
Member

Choose a reason for hiding this comment

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

I think we can probably drop this to keep things simpler

Suggested change
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.


## Breaking changes and migration

- 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:
Copy link
Member

Choose a reason for hiding this comment

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

is there any migration guide on the cloudflare site we could point to instead of including the details here? most of our changelog entries are fairly minimal


```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"`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
compatibility_date = "2021-11-12"`
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.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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.
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 they 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 }
```
2 changes: 1 addition & 1 deletion packages/adapter-cloudflare-workers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

SvelteKit adapter that creates a Cloudflare Workers site using a function for dynamic server rendering.

**Requires [Wrangler v2](https://developers.cloudflare.com/workers/wrangler/get-started/).** Wrangler v1 is no longer supported.
**Requires [Wrangler v3 or later](https://developers.cloudflare.com/workers/wrangler/get-started/).**.

## Docs

Expand Down
9 changes: 0 additions & 9 deletions packages/adapter-cloudflare-workers/files/_package.json

This file was deleted.

45 changes: 6 additions & 39 deletions packages/adapter-cloudflare-workers/files/entry.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { Server } from 'SERVER';
import { manifest, prerendered, base_path } from 'MANIFEST';
import { getAssetFromKV, mapRequestToAsset } from '@cloudflare/kv-asset-handler';
import static_asset_manifest_json from '__STATIC_CONTENT_MANIFEST';
const static_asset_manifest = JSON.parse(static_asset_manifest_json);

const server = new Server(manifest);

Expand All @@ -25,7 +22,7 @@ export default {
// static assets
if (url.pathname.startsWith(app_path)) {
/** @type {Response} */
const res = await get_asset_from_kv(req, env, context);
const res = await env.ASSETS.fetch(req);
if (is_error(res.status)) return res;

const cache_control = url.pathname.startsWith(immutable)
Expand Down Expand Up @@ -65,20 +62,11 @@ export default {

let location = pathname.at(-1) === '/' ? stripped_pathname : pathname + '/';

if (
is_static_asset ||
prerendered.has(pathname) ||
pathname === version_file ||
pathname.startsWith(immutable)
) {
return get_asset_from_kv(req, env, context, (request, options) => {
if (prerendered.has(pathname)) {
url.pathname = '/' + prerendered.get(pathname).file;
return new Request(url.toString(), request);
}

return mapRequestToAsset(request, options);
});
if (prerendered.has(pathname)) {
url.pathname = '/' + prerendered.get(pathname).file;
return env.ASSETS.fetch(new Request(url.toString(), req));
} else if (is_static_asset || pathname === version_file || pathname.startsWith(immutable)) {
return env.ASSETS.fetch(req);
} else if (location && prerendered.has(location)) {
if (search) location += search;
return new Response('', {
Expand Down Expand Up @@ -106,27 +94,6 @@ export default {
}
};

/**
* @param {Request} req
* @param {any} env
* @param {any} context
*/
async function get_asset_from_kv(req, env, context, map = mapRequestToAsset) {
return await getAssetFromKV(
{
request: req,
waitUntil(promise) {
return context.waitUntil(promise);
}
},
{
ASSET_NAMESPACE: env.__STATIC_CONTENT,
ASSET_MANIFEST: static_asset_manifest,
mapRequestToAsset: map
}
);
}

/**
* @param {number} status
* @returns {boolean}
Expand Down
142 changes: 25 additions & 117 deletions packages/adapter-cloudflare-workers/index.js
Original file line number Diff line number Diff line change
@@ -1,152 +1,64 @@
import { existsSync, readFileSync, writeFileSync } from 'node:fs';
import { posix, dirname } from 'node:path';
import { execSync } from 'node:child_process';
import esbuild from 'esbuild';
import toml from '@iarna/toml';
import { fileURLToPath } from 'node:url';
import { getPlatformProxy } from 'wrangler';

/**
* @typedef {{
* main: string;
* site: {
* bucket: string;
* assets: {
* directory: string;
* binding: string;
* }
* compatibility_flags?: string[];
* }} WranglerConfig
*/

// list from https://developers.cloudflare.com/workers/runtime-apis/nodejs/
const compatible_node_modules = [
'assert',
'async_hooks',
'buffer',
'crypto',
'diagnostics_channel',
'events',
'path',
'process',
'stream',
'string_decoder',
'util'
];

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

async adapt(builder) {
const { main, site, compatibility_flags } = validate_config(builder, config);

adapt(builder) {
const { main, assets } = validate_config(builder, config);
const files = fileURLToPath(new URL('./files', import.meta.url).href);
const tmp = builder.getBuildDirectory('cloudflare-workers-tmp');

builder.rimraf(site.bucket);
builder.rimraf(dirname(main));

builder.log.info('Installing worker dependencies...');
builder.copy(`${files}/_package.json`, `${tmp}/package.json`);

// TODO would be cool if we could make this step unnecessary somehow
const stdout = execSync('npm install', { cwd: tmp });
builder.log.info(stdout.toString());
const outDir = dirname(main);
const relativePath = posix.relative(outDir, builder.getServerDirectory());

builder.log.minor('Generating worker...');
const relativePath = posix.relative(tmp, builder.getServerDirectory());

builder.copy(`${files}/entry.js`, `${tmp}/entry.js`, {
// Clear out old files
builder.rimraf(assets.directory);
builder.rimraf(outDir);

// Create the entry-point for the Worker
builder.copy(`${files}/entry.js`, main, {
replace: {
SERVER: `${relativePath}/index.js`,
MANIFEST: './manifest.js'
MANIFEST: './manifest.js',
ASSETS: assets.binding || 'ASSETS'
}
});

// Create the manifest for the Worker
let prerendered_entries = Array.from(builder.prerendered.pages.entries());

if (builder.config.kit.paths.base) {
prerendered_entries = prerendered_entries.map(([path, { file }]) => [
path,
{ file: `${builder.config.kit.paths.base}/${file}` }
]);
}

writeFileSync(
`${tmp}/manifest.js`,
`${outDir}/manifest.js`,
`export const manifest = ${builder.generateManifest({ relativePath })};\n\n` +
`export const prerendered = new Map(${JSON.stringify(prerendered_entries)});\n\n` +
`export const base_path = ${JSON.stringify(builder.config.kit.paths.base)};\n`
);

const external = ['__STATIC_CONTENT_MANIFEST', 'cloudflare:*'];
if (compatibility_flags && compatibility_flags.includes('nodejs_compat')) {
external.push(...compatible_node_modules.map((id) => `node:${id}`));
}

try {
const result = await esbuild.build({
platform: 'browser',
// https://github.com/cloudflare/workers-sdk/blob/a12b2786ce745f24475174bcec994ad691e65b0f/packages/wrangler/src/deployment-bundle/bundle.ts#L35-L36
conditions: ['workerd', 'worker', 'browser'],
sourcemap: 'linked',
target: 'es2022',
entryPoints: [`${tmp}/entry.js`],
outfile: main,
bundle: true,
external,
alias: Object.fromEntries(compatible_node_modules.map((id) => [id, `node:${id}`])),
format: 'esm',
loader: {
'.wasm': 'copy',
'.woff': 'copy',
'.woff2': 'copy',
'.ttf': 'copy',
'.eot': 'copy',
'.otf': 'copy'
},
logLevel: 'silent'
});

if (result.warnings.length > 0) {
const formatted = await esbuild.formatMessages(result.warnings, {
kind: 'warning',
color: true
});

console.error(formatted.join('\n'));
}
} catch (error) {
for (const e of error.errors) {
for (const node of e.notes) {
const match =
/The package "(.+)" wasn't found on the file system but is built into node/.exec(
node.text
);

if (match) {
node.text = `Cannot use "${match[1]}" when deploying to Cloudflare.`;
}
}
}

const formatted = await esbuild.formatMessages(error.errors, {
kind: 'error',
color: true
});

console.error(formatted.join('\n'));

throw new Error(
`Bundling with esbuild failed with ${error.errors.length} ${
error.errors.length === 1 ? 'error' : 'errors'
}`
);
}

builder.log.minor('Copying assets...');
const bucket_dir = `${site.bucket}${builder.config.kit.paths.base}`;
builder.writeClient(bucket_dir);
builder.writePrerendered(bucket_dir);
const assets_dir = `${assets.directory}${builder.config.kit.paths.base}`;
builder.writeClient(assets_dir);
builder.writePrerendered(assets_dir);
},

async emulate() {
Expand Down Expand Up @@ -198,9 +110,9 @@ function validate_config(builder, config_file) {
throw err;
}

if (!wrangler_config.site?.bucket) {
if (!wrangler_config.assets?.directory) {
throw new Error(
`You must specify site.bucket in ${config_file}. Consult https://developers.cloudflare.com/workers/platform/sites/configuration`
`You must specify assets.directory in ${config_file}. Consult https://developers.cloudflare.com/workers/platform/sites/configuration`
);
}

Expand All @@ -223,14 +135,10 @@ function validate_config(builder, config_file) {
name = "<your-site-name>"
account_id = "<your-account-id>"
main = "./.cloudflare/worker.js"
site.bucket = "./.cloudflare/public"
main = ".svelte-kit/cloudflare/server/index.js"
assets = { directory = ".svelte-kit/cloudflare/client" }
build.command = "npm run build"
compatibility_date = "2021-11-12"
workers_dev = true`
compatibility_date = "2021-11-12"`
.replace(/^\t+/gm, '')
.trim()
);
Expand Down
3 changes: 1 addition & 2 deletions packages/adapter-cloudflare-workers/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,12 @@
"esbuild": "^0.21.5"
},
"devDependencies": {
"@cloudflare/kv-asset-handler": "^0.3.0",
"@sveltejs/kit": "workspace:^",
"@types/node": "^18.19.48",
"typescript": "^5.3.3"
},
"peerDependencies": {
"@sveltejs/kit": "^2.0.0",
"wrangler": "^3.28.4"
"wrangler": "^3.87.0"
}
}
5 changes: 0 additions & 5 deletions packages/adapter-cloudflare-workers/placeholders.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,3 @@ declare module 'MANIFEST' {
export const prerendered: Map<string, { file: string }>;
export const base_path: string;
}

declare module '__STATIC_CONTENT_MANIFEST' {
const json: string;
export default json;
}
Loading
Loading