add caching worker for npm installer #3
Conversation
gabbifish
requested review from
ashleygwilliams,
ashleymichal and
EverlastingBugstopper
| event.respondWith(handleRequest(event.request)) | ||
| }) | ||
|
|
||
| async function getJSONFromGitHub(url) { |
There was a problem hiding this comment.
since this takes a url, i feel like this function should be called getJSON since it only gets it from github if the url passed is a github url
| let urlParts = /^https?:\/\/workers\.cloudflare\.com\/get\-npm\-wrangler\-binary\/([^\/]+)\/([^\/]+)(?:\/|$)/.exec(request.url) | ||
| if (!urlParts) | ||
| return new Response("Missing URL components", {status: 400}) |
There was a problem hiding this comment.
it's hard for me to grok what's happening here - is it possible to use URL to have cleaner conditionals, or maybe a more descriptive error message than "Missing URL components"
| let assets = await getJSONFromGitHub(release.assets_url) | ||
|
|
||
| const [compatibleAsset] = assets.filter(asset => | ||
| asset.name.endsWith(arch + ".tar.gz") |
There was a problem hiding this comment.
nit: would use template string
`${arch}.tar.gz`| "test": "echo \"Error: no test specified\" && exit 1", | ||
| "format": "prettier --write '**/*.{js,css,json,md}'" | ||
| }, | ||
| "author": "Gabbi Fisher <gabbi@cloudflare.com>", |
There was a problem hiding this comment.
this should likely be wrangler@cloudflare.com to be consistent with wrangler author
| name = "cache-npm-wrangler-binary" | ||
| type = "js" | ||
| route = "https://workers.cloudflare.com/get-npm-wrangler-binary*" | ||
| workers_dev = false |
There was a problem hiding this comment.
workers_dev = false is unnecessary now! :)
| @@ -0,0 +1,6 @@ | |||
| account_id = "8995c0f49cdcf57eb54d2c1e52b7d2f3" | |||
| name = "cache-npm-wrangler-binary" | |||
| type = "js" | |||
There was a problem hiding this comment.
any reason we aren't using webpack to minify?
Third caching worker, this time for the wrangler binary installed by the npm installer.