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

fix: deep merge fetch options #243

Merged
merged 12 commits into from
Jun 6, 2023
Merged

fix: deep merge fetch options #243

merged 12 commits into from
Jun 6, 2023

Conversation

DennisBetawerk
Copy link
Contributor

Problem
If a fetch instance is created with default header options, and that instance is then called with more headers the default headers are overwritten. The same goes for the param and query properties.

const createdFetch = $fetch.create({
  headers: {
    "Header-Default": "1",
  }
});

createdFetch("/some-url", {
  headers: {
    "Header-Request": "1",
  }
});

The outgoing request should have both the Header-Request and Header-Default headers, but will only contain the former.

Fix
Merging the headers, params and query properties of the FetchOptions object before fetching fixes this issue. A more generic deep merge approach was tried, but merging other properties such as the body makes little sense.

@pi0 pi0 changed the title Merge FetchOptions deeper fix: deep merge fetch options Jun 6, 2023
@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Merging #243 (d174e6e) into main (253707a) will increase coverage by 3.56%.
The diff coverage is 100.00%.

❗ Current head d174e6e differs from pull request most recent head f346fa9. Consider uploading reports for the commit f346fa9 to get more accurate results

@@            Coverage Diff             @@
##             main     #243      +/-   ##
==========================================
+ Coverage   86.76%   90.32%   +3.56%     
==========================================
  Files           5        5              
  Lines         423      455      +32     
  Branches       67       73       +6     
==========================================
+ Hits          367      411      +44     
+ Misses         56       44      -12     
Impacted Files Coverage Δ
src/fetch.ts 94.60% <100.00%> (+5.02%) ⬆️
src/utils.ts 95.19% <100.00%> (+2.04%) ⬆️

@pi0 pi0 merged commit 933e754 into unjs:main Jun 6, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants