fix(http): less restrictive arguments for accepts*() functions

[kitsonk]

Currently the http negotiation methods are arbitrarily restrictive on what objects can be passed to the methods. It requires a Fetch API request object, when in fact it only requires a very specific type in the code. Setting overly restrictive types reduces compatibility and should be avoided.

Prior to 1.0 of std/http this was a case, but was changed for some reason causing downstream regressions in dependent code. Please consider expressing minimally restrictive types that actually reflect what the code is dependent upon.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.26%. Comparing base (2e209c0) to head (0bdffcc).
Report is 7 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5850   +/-   ##
=======================================
  Coverage   96.26%   96.26%           
=======================================
  Files         479      479           
  Lines       38751    38738   -13     
  Branches     5620     5621    +1     
=======================================
- Hits        37302    37291   -11     
+ Misses       1406     1404    -2     
  Partials       43       43           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[timreichen]

Currently the http negotiation methods are arbitrarily restrictive on what objects can be passed to the methods. It requires a Fetch API request object, when in fact it only requires a very specific type in the code. Setting overly restrictive types reduces compatibility and should be avoided.

Prior to 1.0 of std/http this was a case, but was changed for some reason causing downstream regressions in dependent code. Please consider expressing minimally restrictive types that actually reflect what the code is dependent upon.

Great point, but shouldn't it be Headers then that should be passed instead of wrapping them in an object?

[iuioiua]

Yeah, it'd make much more sense if these functions simply accepted Headers.

[kitsonk]

That is a breaking API change with little ergonomic upside.

Prior to 1.0 it accepted an object with a headers property, now it is restrictive. The least disruptive change would be to revert to the behaviour prior to 1.0.

[kt3k]

@kitsonk
A question about some details. Can this be { headers: Headers } in your use case?

[iuioiua]

I'd prefer this be Headers too.

[kt3k]

It feels a bit undesirable to expose this type as a public API. If it's ok to be { headers: Headers }, maybe we can use Pick<Request, "headers"> type instead of Headered for function parameter types

[iuioiua]

While I think these functions should've accepted headers: Headers from the start, it's true that we can't make such changes post-1.0, for the sake of stability. I've changed this to go with Yoshiya's suggestion of using Pick<Request, "headers">.

Either way, thank you, Kitson.

[timreichen]

I think we also could do

export function acceptsLanguages(headers: Request | Headers, ...langs: string[]): string | string[] | undefined {
  headers = headers instanceof Request ? headers.headers : headers
  ...
}

This avoids object wrapping but also doesn't break the code.

[iuioiua]

I think we also could do

export function acceptsLanguages(headers: Request | Headers, ...langs: string[]): string | string[] | undefined {
  headers = headers instanceof Request ? headers.headers : headers
  ...
}

to allow Headers but not break the code.

I think the path of least action is what this PR currently does. It's ok how it is now.