cloudflare / templates

A collection of starter templates and examples for Cloudflare Workers and Pages
https://cloudflareworkers.com
MIT License
1k stars 638 forks source link

Modify request headers #3

Closed shagamemnon closed 6 years ago

shagamemnon commented 6 years ago

This example:

kentonv commented 6 years ago

Hi Frank,

Thanks for contributing!

It turns out there's an easier way -- I actually didn't even know this until just today. You can do:

let newReq = new Request(request)
newReq.headers.append("foo", "bar")
return fetch(newReq)

The first line copies the request including copying the headers object. In the newly-created request, the headers can be modified directly.

Also, if you want to override the URL (this only works as of last week):

let newReq = new Request(newUrl, request)

That is, it's no longer necessary to use Object.assign({}, request) to create an init object -- you can use the request itself.

shagamemnon commented 6 years ago

Hey Kenton,

Really appreciate the extended feedback! A few questions and comments:

1) Does the newURL argument behave like the event.newURL property? 2) On the current workers beta, this returns an error:

let newReq = new Request(request)
newReq.headers.append("foo", "bar")
return fetch(newReq)
// returns 'TypeError: Can't modify immutable headers.'

capto_capture 2018-02-06_02-37-48_pm

3) The main purpose of my example was the addHeaders() callback – which allows the author to set an unlimited number of new request headers instead of a single k-v pair. This can be reapplied for headers.set() and headers.get(). Is their some other technique that can be used for this?

kentonv commented 6 years ago

Does the newURL argument behave like the event.newURL property?

Sorry, what's event.newURL?

On the current workers beta, this returns an error:

Oh interesting, if I do new Request(request.url, request) then I get a request whose headers are modifiable, but if I do new Request(request) I don't. I think that's a bug; they should both be modifiable.

The main purpose of my example was the addHeaders() callback – which allows the author to set an unlimited number of new request headers instead of a single k-v pair.

Do you mean that representing the headers as an object literal looks nicer than writing a series of request.header.append() lines? Or is there another advantage?

shagamemnon commented 6 years ago

Sorry, what's event.newURL?

You noted above that you can use let newReq = new Request(newUrl, request) I was wondering what this newURL property referred to? event.newURL is a property from the hashbang era (https://developer.mozilla.org/en-US/docs/Web/Events/hashchange)

Or is there another advantage?

That is the only advantage. I do think it works well as an example because it explains how the append() method works for those unfamiliar with this API. That said, I acknowledge it also adds some complexity and perhaps should be omitted for now!

shagamemnon commented 6 years ago

One last note: request.header is a read-only property in the context of new Request(request) because passing a Request object into a new Request() constructor automatically initializes it (don't quote me on this)

I haven't looked further into this but I know there is a request.clone() method that might be used for this?

kentonv commented 6 years ago

You noted above that you can use let newReq = new Request(newUrl, request) I was wondering what this newURL property referred to?

Sorry, this was supposed to be an example of how to override the request URL. newUrl is an example variable name meant to represent whatever it is you want to change the URL to.

That is the only advantage. I do think it works well as an example because it explains how the append() method works for those unfamiliar with this API. That said, I acknowledge it also adds some complexity and perhaps should be omitted for now!

Hmm, I think it may be easier for users to follow if we just show some calls to .set() and .append(). The advanced users will already know how to write a helper function like yours, while the novice users will not get confused by the Javascript object manipulation that isn't really specific to Workers.

One last note: request.header is a read-only property in the context of new Request(request) because passing a Request object into a new Request() constructor automatically initializes it (don't quote me on this)

Nope, it's supposed to create a copy of the headers and the copy is supposed to be mutable. This works in Chrome's implementation, for example. So ours is definitely buggy.

I haven't looked further into this but I know there is a request.clone() method that might be used for this?

There's a subtle difference between new Request(request) and request.clone(): the latter "tees" the body, so that you can actually send both requests, whereas with the former, the new Request consumes the old Request's body, such that the old Request can no longer be sent (if it has a body, at least).

shagamemnon commented 6 years ago

One last note: request.header is a read-only property in the context of new Request(request) because passing a Request object into a new Request() constructor automatically initializes it (don't quote me on this)

Indeed – I just noticed that I had done that on a ServiceWorker in the past.

Needless to say, time to close this PR! Thanks for this thread – helpful for many reasons