Skip to content
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

Ensure that a Request object can be used as RequestInit #1486

Closed
davidbarratt opened this issue Sep 8, 2022 · 7 comments
Closed

Ensure that a Request object can be used as RequestInit #1486

davidbarratt opened this issue Sep 8, 2022 · 7 comments

Comments

@davidbarratt
Copy link

davidbarratt commented Sep 8, 2022

When constructing a Request object, this works in every browser:

const base = new Request('https://example.com', { headers: { 'accept': 'text/html' }});
const request = new Request('https://new.example.com', base);
console.log(request.headers.get('accept')); // text/html
console.log(request.url); // https://new.example.com
Browser Works
Firefox
Chrome
Safari
Deno

I assumed this was the prefered way to make modifications to a URL of a request object, given that this does not work in any browser given that it drops the headers from the base completely.

const base = new Request('https://example.com', { headers: { 'accept': 'text/html' }});
const request = new Request('https://new.example.com', { ...base });
console.log(request.headers.get('accept')); // null
console.log(request.url); // https://new.example.com
Browser Works
Firefox
Chrome
Safari
Deno

This seems straightforward enough, if you need to change the URL of a request object, pass the existing request object as the second parameter.

Things get really weird though when there is a body on the request.

const base = new Request('https://example.com', { method: 'POST', body: 'hello!', headers: { 'accept': 'text/html' }});
const request = new Request('https://new.example.com', base);
console.log(request.headers.get('accept')); // text/html
console.log(request.url); // https://new.example.com
console.log(await request.text()); // hello!
Browser Works Result
Firefox body is dropped
Chrome Uncaught TypeError: Failed to construct 'Request': The `duplex` member must be specified for a request with a streaming body
Safari
Deno

Could the spec clarify that Request should work as a RequestInit?

@davidbarratt davidbarratt changed the title Enusre that a Request object can be used as RequestInit Ensure that a Request object can be used as RequestInit Sep 8, 2022
@annevk
Copy link
Member

annevk commented Sep 15, 2022

Can you remind me what ...base ends up doing? The last case looks like a result from the recent changes around upload streams.

cc @jakearchibald

@davidbarratt
Copy link
Author

davidbarratt commented Sep 17, 2022

Can you remind me what ...base ends up doing?

Surprisingly, it becomes an empty object.

The only way I could convert Request to a "plain" object was like this:

const base = new Request('https://example.com', { method: 'POST', body: 'hello!', headers: { 'accept': 'text/html' }});

const init = {};
for (const property in base) {
  init[property] = base[property];
}

const request = new Request('https://new.example.com', {
  ...init,
  duplex: 'half'
});
console.log(request.headers.get('accept')); // text/html
console.log(request.url); // https://new.example.com
console.log(await request.text());  // hello!

Here are the results:

Browser Works Result
Firefox body is dropped
Chrome threw an error unless I added the duplex option above
Safari
Deno

The last case looks like a result from the recent changes around upload streams.

Yes. That is exactly right. In Chrome <= 104, Chrome had the same behavior as Firefox (dropping the body). Chrome >= 105, the error starts being thrown. I opened a bug here:
https://bugs.chromium.org/p/chromium/issues/detail?id=1360943


It seems like there isn't a "clean" way to merge two request objects or even simply override the URL of an existing Request object.

@annevk
Copy link
Member

annevk commented Sep 21, 2022

Oh, the reason it becomes empty is presumably because ... ignores the prototype chain. So this seems largely okay, modulo the bug in Firefox.

And I guess one way to read this is as a request to expose duplex on Request.

@davidbarratt
Copy link
Author

And I guess one way to read this is as a request to expose duplex on Request.

Yes. Or more precisely to update this type:

constructor(RequestInfo input, optional RequestInit init = {});

to be:

constructor(RequestInfo input, optional (Request or RequestInit) init = {});

https://fetch.spec.whatwg.org/#request-class

In other words, User-Agents should expect a Request as either the first parameter, the second, or both.

@annevk
Copy link
Member

annevk commented Sep 21, 2022

I suppose that could be an alternative if we decide not to expose duplex directly, but that doesn't match precedent very well. Note that for dictionaries the prototype chain is consulted. They do a Get() for members on the passed object.

@davidbarratt
Copy link
Author

davidbarratt commented Sep 21, 2022

If it's helpful, I would expect user agents to set the duplex to half when it is not required (i.e. when I pass in a string as the body) and then expose duplex as a property. This would have prevented the error in Chrome.

@annevk
Copy link
Member

annevk commented Sep 26, 2022

This will be fixed by #1493.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants