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
Adds simple caching example #75
Conversation
Would not mind feedback on this
It's best if the examples just run (ie: no external dependencies are secret requirements). I think you can do most of what you want without any extra modules, and using onehostname.com or fly.io as the upstream site. |
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.
Need to make this run with no dependencies or setting of secrets required.
apps/simple-caching/index.js
Outdated
fly.http.respondWith(async (req) => { | ||
/* This just takes apart and reconstructs the url. Not super useful in this example, but I mean to use the params object later to do some other fly.io magic. Also protects against stuff like HOST having or not having a backslash. */ | ||
const params = parse(req.url) | ||
const path = resolve(destination, params.path) |
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.
These two lines can be done mostly like this, no external dependencies required:
const url = new URL(req.url)
const path = url.pathname
const backend = new URL(url)
backend.scheme = "https:"
backend.port = 443
backend.hostname = "onehostname.com"
There's a relatively advanced proxy function here we'll probably build in sometime: https://github.com/superfly/onehostname/blob/master/src/backends.ts#L140-L187
apps/simple-caching/index.js
Outdated
/* Fetch the requested site. Remember, this just happens if nothing is cached. */ | ||
const request = await fetch(path, { | ||
headers: { | ||
Accept: "application/json" // this header makes sure we don't receive .js requests as html, but doesn't seem to bother the html requests |
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.
You can just make a new request from the existing one:
const request = new Request(req)
request.url = path // new url, otherwise use existing request
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.
While I think this did work when doing fetch(request)
where request
is the instance of Request
with .url = path
like you described, this is apparently a read-only property (according to Mozilla docs). I think there is a Request.clone()
but iirc when I tried to use it in fly it said it wasn't a function? Any, playing around with this and thanks for mentioning it.
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.
Think I've worked this out. If you do const url = new URL(req.url)
, must also do request.url = url.href
, I think.
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.
Ah, derp, I gave you a bad example.
The "right" way to make a new request with a different url is:
const url = new URL("https://fly.io")
const req2 = new Request(url.toString(), req) // uses new url and all other params from req
You are right that the spec says .url
is read only, we cheated on that one because it's so convenient to change URL.
req.clone()
should also work but usually you'd only use that if you wanted to read a POST body more than once, not to make a request to a different URL.
@mrkurt This will run without any secrets. But a secrets file is included and explained, because it's basics everyone should learn. But a fallback is hardcoded in |
Would not mind feedback on this