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

Bind dispatcher to Request Object #2828

Closed
matthieusieben opened this issue Feb 23, 2024 · 2 comments · Fixed by #2831
Closed

Bind dispatcher to Request Object #2828

matthieusieben opened this issue Feb 23, 2024 · 2 comments · Fixed by #2831
Labels
enhancement New feature or request

Comments

@matthieusieben
Copy link
Contributor

matthieusieben commented Feb 23, 2024

This would solve...

From the whatwg spec we can read Let requestObject be the result of invoking the initial value of Request as constructor with input and init as arguments.

This makes it seem like the two following are exactly equivalent, no matter what input and init are:

fetch(input, init)
fetch(new Request(input, init))

This is mostly true, with the exception of init.dispatcher (I know, not defined by the spec):

dispatcher: init?.dispatcher ?? getGlobalDispatcher() // undici

I believe it would make more sense to be able to attach a dispatcher directly to the request object so that the behavior defined by the spec also applies to the custom(s) options added by undici.

The implementation should look like...

Similar to the body Request attribute, dispatcher would be set as an attribute of the request that can be overridden using init.

I have also considered...

Status quo implies that if we are only working with Request object as input, we still need to carry over the init for the sole purpose of being able to pass a custom dispatcher to undici.

Additional context

I created a bunch of utilities to work with fetch(). Being able to work with a single value makes a lot of things simpler. Consider for example the following wrapper:

const logWrap = fetch => async (input, init) => {
  const request = new Request(input, init) // Should be fine per Whatwg spec
  console.log(request.method, request.uri)
  const response = await fetch(request)
  console.log(response.status)
  return response
}

Being able to use new Request to be able to compute the merge of input and init is super useful. This causes however an issue when working with undici because of the additionnal dispatcher option.

Fixing this requires to do the following:

const logWrap = fetch => async (input, init) => {
  const request = new Request(input, init)
  console.log(request.method, request.uri)
  const response = await fetch(request, { dispatcher: init?.dispatcher }) // undici specific
  console.log(response.status)
  return response
}

This implementation is not agnostic of the actual fetch implementation used under the hood, which breaks separation of concerns.

@matthieusieben matthieusieben added the enhancement New feature or request label Feb 23, 2024
@mcollina
Copy link
Member

cc @KhafraDev

KhafraDev added a commit to KhafraDev/undici that referenced this issue Feb 23, 2024
@matthieusieben
Copy link
Contributor Author

Thanks guys ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants