h3 icon indicating copy to clipboard operation
h3 copied to clipboard

feat(headers): support array values for multi-field headers

Open GalacticHypernova opened this issue 1 year ago • 3 comments

🔗 Linked issue

Related to #625

❓ Type of change

  • [ ] 📖 Documentation (updates to the documentation, readme, or JSdoc annotations)
  • [ ] 🐞 Bug fix (a non-breaking change that fixes an issue)
  • [x] 👌 Enhancement (improving an existing functionality like performance)
  • [x] ✨ New feature (a non-breaking change that adds functionality)
  • [ ] 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • [ ] ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

This PR adds a way to provide header values in arrays, which could be beneficial for headers that can accept multiple values (example: Accept). This PR will also make it much easier to implement #625 typing wise.

📝 Checklist

  • [x] I have linked an issue or discussion.
  • [ ] I have updated the documentation accordingly.

GalacticHypernova avatar Jan 21 '24 20:01 GalacticHypernova

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (8345c1f) 77.80% compared to head (7ebfee5) 77.85%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #627      +/-   ##
==========================================
+ Coverage   77.80%   77.85%   +0.04%     
==========================================
  Files          47       47              
  Lines        4281     4290       +9     
  Branches      610      610              
==========================================
+ Hits         3331     3340       +9     
  Misses        933      933              
  Partials       17       17              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 21 '24 20:01 codecov[bot]

On second thought, this should probably be reworked to include semicolon separated headers, and perhaps create a helper function to determine what to separate by. I'm just worried about the performance side of it. Would love to hear your implementation suggestions!

GalacticHypernova avatar Jan 21 '24 22:01 GalacticHypernova

Seeing as this is already supported by the underlying http lib I will keep this a draft until I find out it works fine. If it doesn't I will likely mark this as ready to review, is that fine?

GalacticHypernova avatar Jan 22 '24 16:01 GalacticHypernova

I agree that multi headers is a headache. Node.js already does process by joining with ; (src) -- and different runtimes do it slightly differently :(

I think best is to let the users join multi values before calling methods if they need specific behavior.

pi0 avatar Jun 19 '24 11:06 pi0