-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
refactor: use Module Worker pattern instead of Service Worker, add error handling #4276
Merged
Rich-Harris
merged 6 commits into
sveltejs:master
from
dominikg:refactor/adapter-cloudflare-workers
Apr 15, 2022
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
75b3280
refactor: use Module Worker pattern instead of Service Worker, add er…
dominikg 06197a6
chore: add changeset
dominikg f102255
chore: improve changeset
dominikg 3e34bb0
fix: update adapter-cloudflare-workers to build module package see ht…
dominikg f0c820b
more validation
Rich-Harris 9b70922
feat: allow custom esbuild options for workers adapter
dominikg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
--- | ||
'@sveltejs/adapter-cloudflare-workers': patch | ||
--- | ||
|
||
[Breaking] refactor implementation from "Service Worker" pattern to "Module Worker" used in adapter-cloudflare | ||
|
||
#### add the following to your wrangler.toml | ||
```toml | ||
[build.upload] | ||
format = "modules" | ||
main = "./worker.mjs" | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,61 +1,101 @@ | ||
import { Server } from 'SERVER'; | ||
import { manifest, prerendered } from 'MANIFEST'; | ||
import { getAssetFromKV } 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); | ||
|
||
const prefix = `/${manifest.appDir}/`; | ||
|
||
addEventListener('fetch', (/** @type {FetchEvent} */ event) => { | ||
event.respondWith(handle(event)); | ||
}); | ||
export default { | ||
/** | ||
* @param {Request} req | ||
* @param {any} env | ||
* @param {any} context | ||
*/ | ||
async fetch(req, env, context) { | ||
const url = new URL(req.url); | ||
|
||
/** | ||
* @param {FetchEvent} event | ||
* @returns {Promise<Response>} | ||
*/ | ||
async function handle(event) { | ||
const { request } = event; | ||
|
||
const url = new URL(request.url); | ||
|
||
// generated assets | ||
if (url.pathname.startsWith(prefix)) { | ||
const res = await getAssetFromKV(event); | ||
return new Response(res.body, { | ||
headers: { | ||
'cache-control': 'public, immutable, max-age=31536000', | ||
'content-type': res.headers.get('content-type') | ||
// static assets | ||
if (url.pathname.startsWith(prefix)) { | ||
/** @type {Response} */ | ||
const res = await get_asset_from_kv(req, env, context); | ||
if (is_error(res.status)) { | ||
return res; | ||
} | ||
}); | ||
} | ||
return new Response(res.body, { | ||
headers: { | ||
// include original cache headers, minus cache-control which | ||
// is overridden, and etag which is no longer useful | ||
'cache-control': 'public, immutable, max-age=31536000', | ||
'content-type': res.headers.get('content-type'), | ||
'x-robots-tag': 'noindex' | ||
} | ||
}); | ||
} | ||
|
||
// prerendered pages and index.html files | ||
const pathname = url.pathname.replace(/\/$/, ''); | ||
let file = pathname.substring(1); | ||
// prerendered pages and index.html files | ||
const pathname = url.pathname.replace(/\/$/, ''); | ||
let file = pathname.substring(1); | ||
|
||
try { | ||
file = decodeURIComponent(file); | ||
} catch (err) { | ||
// ignore | ||
} | ||
try { | ||
file = decodeURIComponent(file); | ||
} catch (err) { | ||
// ignore | ||
} | ||
|
||
if ( | ||
manifest.assets.has(file) || | ||
manifest.assets.has(file + '/index.html') || | ||
prerendered.has(pathname || '/') | ||
) { | ||
return get_asset_from_kv(req, env, context); | ||
} | ||
|
||
if ( | ||
manifest.assets.has(file) || | ||
manifest.assets.has(file + '/index.html') || | ||
prerendered.has(pathname || '/') | ||
) { | ||
return await getAssetFromKV(event); | ||
// dynamically-generated pages | ||
try { | ||
return await server.respond(req, { | ||
platform: { env, context }, | ||
getClientAddress() { | ||
return req.headers.get('cf-connecting-ip'); | ||
} | ||
}); | ||
} catch (e) { | ||
return new Response('Error rendering route: ' + (e.message || e.toString()), { status: 500 }); | ||
} | ||
} | ||
}; | ||
|
||
// dynamically-generated pages | ||
/** | ||
* @param {Request} req | ||
* @param {any} env | ||
* @param {any} context | ||
*/ | ||
async function get_asset_from_kv(req, env, context) { | ||
try { | ||
return await server.respond(request, { | ||
getClientAddress() { | ||
return request.headers.get('cf-connecting-ip'); | ||
return await getAssetFromKV( | ||
{ | ||
request: req, | ||
waitUntil(promise) { | ||
return context.waitUntil(promise); | ||
} | ||
}, | ||
{ | ||
ASSET_NAMESPACE: env.__STATIC_CONTENT, | ||
ASSET_MANIFEST: static_asset_manifest | ||
} | ||
}); | ||
); | ||
} catch (e) { | ||
return new Response('Error rendering route:' + (e.message || e.toString()), { status: 500 }); | ||
const status = is_error(e.status) ? e.status : 500; | ||
return new Response(e.message || e.toString(), { status }); | ||
} | ||
} | ||
|
||
/** | ||
* @param {number} status | ||
* @returns {boolean} | ||
*/ | ||
function is_error(status) { | ||
return status > 399; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this needed or could/should we hardcode/use the filename for main instead?
The checks below feel a bit brittle, what if there is no glob but an excact matching rule instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be a bug with wrangler, but main seems to be treated according to the rules, regardless of the specified
format
. My build kept failing to upload until I changed the name to .mjs or added a rule to treat *.js as ESM.build.upload.main
is actually required, so we can probably get rid of all the?.
there.