fix(fetch): improve Headers and Request type-compatibility
- Reopened #1947 because GitHub UI is fantastic.
As mentioned in the previous review, a type test has to be added here. I will add it this/next week due to the work load.
type tests are failing (npm run test:typescript)
test/types/fetch.test-d.ts:141:0
✖ 141:0 Parameter type string is declared too wide for argument type ReferrerPolicy.
Suddenly after an update in my package i got the problem with the referrer property missing and i just found this library and this PR is related.
I manually edited fetch.d.ts to test this PR. I can get rid of the test error you mentioned if i revert
readonly referrerPolicy: ReferrerPolicy
to
readonly referrerPolicy: string
- Would that be acceptable ? I fear this is not strict enough or just a bad solution
- Should i make another PR because the person who submitted it seems to be busy?
@MikeZeDev, not sure if string isn't too broad for the referrer policy. It is annotated as RefererPolicy type in lib.dom, I think it'd be best if Undici introduced compatibility with that as well. Value-wise, you cannot set unknown values anyway. Can you tell me more about the use case that resulted in the error for you?
@KhafraDev, I've fixed the test by adjusting the expected type. It's no longer a wider string but a narrower ReferrerPolicy, which is compatible with how lib.dom annotates the referrerPolicy request property.
Type tests are passing now.
> [email protected] test:typescript
> tsd && tsc --skipLibCheck test/imports/undici-import.ts
@KhafraDev, may I please get the workflow approved on this pull request to see if there are any other pending things to resolve? Thanks.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 94.17%. Comparing base (
e009b0c) to head (af9433a). Report is 3 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #1964 +/- ##
=======================================
Coverage 94.17% 94.17%
=======================================
Files 90 90
Lines 24434 24434
=======================================
Hits 23011 23011
Misses 1423 1423
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@mcollina, note that this change can be technically breaking if consumers relied on referrerPolicy being a plain string. A narrower type shipped by this change will error on the type level in some cases.
My personal take on situations like this is if you rely on a bug, you should expect breaking changes. I'd release this as a patch fix since the type annotations were previously incorrect.