Skip to content

Commit 683c12d

Browse files
kevinjiteemingc
kevinji
and
teemingc
authored
fix: support assets-only workers in wrangler validation (#14019)
* fix: support assets-only workers in wrangler validation Fixes #14018. * remove main key validation for asset only workers since it's already validated by wrangler * add tests --------- Co-authored-by: Chew Tee Ming <chew.tee.ming@nindatech.com>
1 parent fda0165 commit 683c12d

File tree

4 files changed

+124
-29
lines changed

4 files changed

+124
-29
lines changed

‎.changeset/warm-rats-say.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@sveltejs/adapter-cloudflare': patch
3+
---
4+
5+
fix: support assets-only workers in wrangler validation

‎packages/adapter-cloudflare/index.js‎

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { VERSION } from '@sveltejs/kit';
22
import { copyFileSync, existsSync, writeFileSync } from 'node:fs';
33
import path from 'node:path';
44
import { fileURLToPath } from 'node:url';
5-
import { is_building_for_cloudflare_pages } from './utils.js';
5+
import { is_building_for_cloudflare_pages, validate_worker_settings } from './utils.js';
66
import { getPlatformProxy, unstable_readConfig } from 'wrangler';
77

88
const name = '@sveltejs/adapter-cloudflare';
@@ -31,7 +31,7 @@ export default function (options = {}) {
3131
);
3232
}
3333

34-
const wrangler_config = validate_config(options.config);
34+
const wrangler_config = validate_wrangler_config(options.config);
3535
const building_for_cloudflare_pages = is_building_for_cloudflare_pages(wrangler_config);
3636

3737
let dest = builder.getBuildDirectory('cloudflare');
@@ -286,33 +286,12 @@ _redirects
286286
* @param {string} config_file
287287
* @returns {import('wrangler').Unstable_Config}
288288
*/
289-
function validate_config(config_file = undefined) {
289+
function validate_wrangler_config(config_file = undefined) {
290290
const wrangler_config = unstable_readConfig({ config: config_file });
291291

292-
// we don't support workers sites
293-
if (wrangler_config.site) {
294-
throw new Error(
295-
`You must remove all \`site\` keys in ${wrangler_config.configPath}. Consult https://svelte.dev/docs/kit/adapter-cloudflare#Migrating-from-Workers-Sites-to-Workers-Static-Assets`
296-
);
297-
}
298-
299-
if (is_building_for_cloudflare_pages(wrangler_config)) {
300-
return wrangler_config;
301-
}
302-
303-
// probably deploying to Cloudflare Workers
304-
if (wrangler_config.main || wrangler_config.assets) {
305-
if (!wrangler_config.assets?.directory) {
306-
throw new Error(
307-
`You must specify the \`assets.directory\` key in ${wrangler_config.configPath}. Consult https://developers.cloudflare.com/workers/static-assets/binding/#directory`
308-
);
309-
}
310-
311-
if (!wrangler_config.assets?.binding) {
312-
throw new Error(
313-
`You must specify the \`assets.binding\` key in ${wrangler_config.configPath}. Consult https://developers.cloudflare.com/workers/static-assets/binding/#binding`
314-
);
315-
}
292+
if (!is_building_for_cloudflare_pages(wrangler_config)) {
293+
// probably deploying to Cloudflare Workers
294+
validate_worker_settings(wrangler_config);
316295
}
317296

318297
return wrangler_config;

‎packages/adapter-cloudflare/utils.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,37 @@ export function is_building_for_cloudflare_pages(wrangler_config) {
1515

1616
return true;
1717
}
18+
19+
/**
20+
* @param {import('wrangler').Unstable_Config} wrangler_config
21+
*/
22+
export function validate_worker_settings(wrangler_config) {
23+
// we don't support workers sites
24+
if (wrangler_config.site) {
25+
throw new Error(
26+
`You must remove all \`site\` keys in ${wrangler_config.configPath}. Consult https://svelte.dev/docs/kit/adapter-cloudflare#Migrating-from-Workers-Sites-to-Workers-Static-Assets`
27+
);
28+
}
29+
30+
// we need the `assets.directory` key so that the static assets are deployed
31+
if ((wrangler_config.main || wrangler_config.assets) && !wrangler_config.assets?.directory) {
32+
throw new Error(
33+
`You must specify the \`assets.directory\` key in ${wrangler_config.configPath}. Consult https://developers.cloudflare.com/workers/static-assets/binding/#directory`
34+
);
35+
}
36+
37+
// we need the `assets.binding` key so that the Worker can access the static assets
38+
if (wrangler_config.main && !wrangler_config.assets?.binding) {
39+
throw new Error(
40+
`You must specify the \`assets.binding\` key in ${wrangler_config.configPath} before deploying your Worker. Consult https://developers.cloudflare.com/workers/static-assets/binding/#binding`
41+
);
42+
}
43+
44+
// the user might have forgot the `main` key or should remove the `assets.binding`
45+
// key to deploy static assets without a Worker
46+
if (!wrangler_config.main && wrangler_config.assets?.binding) {
47+
throw new Error(
48+
`You must set the \`main\` key in ${wrangler_config.configPath} if you want to deploy a Worker alongside your static assets or remove the \`assets.binding\` key if you only want to deploy static assets.`
49+
);
50+
}
51+
}

‎packages/adapter-cloudflare/utils.spec.js‎

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, test, vi, expect } from 'vitest';
2-
import { is_building_for_cloudflare_pages } from './utils.js';
2+
import { is_building_for_cloudflare_pages, validate_worker_settings } from './utils.js';
33

44
describe('detects Cloudflare Pages project', () => {
55
test('by default', () => {
@@ -56,12 +56,89 @@ describe('detects Cloudflare Workers project', () => {
5656
is_building_for_cloudflare_pages(
5757
/** @type {import('wrangler').Unstable_Config} */ ({
5858
configPath: 'wrangler.jsonc',
59+
assets: {
60+
directory: 'dist/assets'
61+
}
62+
})
63+
)
64+
).toBe(false);
65+
});
66+
});
67+
68+
describe('validates Wrangler config', () => {
69+
test('Worker and static assets', () => {
70+
expect(() =>
71+
validate_worker_settings(
72+
/** @type {import('wrangler').Unstable_Config} */ ({
73+
configPath: 'wrangler.jsonc',
74+
main: 'dist/index.js',
5975
assets: {
6076
directory: 'dist/assets',
6177
binding: 'ASSETS'
6278
}
6379
})
6480
)
65-
).toBe(false);
81+
).not.toThrow();
82+
});
83+
84+
test('static assets only', () => {
85+
expect(() =>
86+
validate_worker_settings(
87+
/** @type {import('wrangler').Unstable_Config} */ ({
88+
configPath: 'wrangler.jsonc',
89+
assets: {
90+
directory: 'dist/assets'
91+
}
92+
})
93+
)
94+
).not.toThrow();
95+
});
96+
97+
test('missing `assets.directory` key', () => {
98+
expect(() =>
99+
validate_worker_settings(
100+
/** @type {import('wrangler').Unstable_Config} */ ({
101+
configPath: 'wrangler.jsonc',
102+
main: 'dist/index.js',
103+
assets: {
104+
binding: 'ASSETS'
105+
}
106+
})
107+
)
108+
).toThrow(
109+
`You must specify the \`assets.directory\` key in wrangler.jsonc. Consult https://developers.cloudflare.com/workers/static-assets/binding/#directory`
110+
);
111+
});
112+
113+
test('missing `assets.binding` key', () => {
114+
expect(() =>
115+
validate_worker_settings(
116+
/** @type {import('wrangler').Unstable_Config} */ ({
117+
configPath: 'wrangler.jsonc',
118+
main: 'dist/index.js',
119+
assets: {
120+
directory: 'dist/assets'
121+
}
122+
})
123+
)
124+
).toThrow(
125+
`You must specify the \`assets.binding\` key in wrangler.jsonc before deploying your Worker. Consult https://developers.cloudflare.com/workers/static-assets/binding/#binding`
126+
);
127+
});
128+
129+
test('missing `main` key or should remove `assets.binding` key', () => {
130+
expect(() =>
131+
validate_worker_settings(
132+
/** @type {import('wrangler').Unstable_Config} */ ({
133+
configPath: 'wrangler.jsonc',
134+
assets: {
135+
directory: 'dist/assets',
136+
binding: 'ASSETS'
137+
}
138+
})
139+
)
140+
).toThrow(
141+
`You must set the \`main\` key in wrangler.jsonc if you want to deploy a Worker alongside your static assets or remove the \`assets.binding\` key if you only want to deploy static assets.`
142+
);
66143
});
67144
});

0 commit comments

Comments
 (0)