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

feat: improve typed headers #625

Merged
merged 104 commits into from
Jun 17, 2024
Merged

feat: improve typed headers #625

merged 104 commits into from
Jun 17, 2024

Conversation

GalacticHypernova
Copy link
Contributor

@GalacticHypernova GalacticHypernova commented Jan 21, 2024

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ 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 is something I thought about for a while. Since we have header intellisense, we might as well have value intellisense. This will ensure no wrong header values are set and would save time debugging header issues (a common example of this is the misconception that image/jpg is a valid content type, when in reality it doesn't have a universally acceptable type). Granted, this won't be a complete intellisense for all possible values as the value combinations can be endless, however it would give a good foundation.
EDIT: As of today, I have decided to expand this further and include status codes (maybe something else as well)

πŸ“ Checklist

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

@GalacticHypernova GalacticHypernova marked this pull request as draft January 21, 2024 14:12
@GalacticHypernova
Copy link
Contributor Author

Marking this as draft as this is very early in development. I will gradually add everything I can, but it will take a good while.

Copy link

codecov bot commented Jan 21, 2024

Codecov Report

All modified and coverable lines are covered by tests βœ…

Project coverage is 79.47%. Comparing base (a58d7c9) to head (b57497a).
Report is 76 commits behind head on main.

❗ Current head b57497a differs from pull request most recent head e935c8c

Please upload reports for the commit e935c8c to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #625      +/-   ##
==========================================
+ Coverage   77.83%   79.47%   +1.64%     
==========================================
  Files          47       54       +7     
  Lines        4286     5346    +1060     
  Branches      611      675      +64     
==========================================
+ Hits         3336     4249     +913     
- Misses        933     1078     +145     
- Partials       17       19       +2     

β˜” View full report in Codecov by Sentry.
πŸ“’ Have feedback on the report? Share it here.

@GalacticHypernova GalacticHypernova changed the title feat(headers): support value intellisense for headers feat(headers): support partial value intellisense for headers Jan 21, 2024
@GalacticHypernova
Copy link
Contributor Author

GalacticHypernova commented Feb 26, 2024

I believe this has covered everything in the scope of the PR. If I missed anything in any other function where it can be improved, please let me know and I'll gladly add it.

There are a few things I would like to change, like the AnyType usage, but I do need a tiny bit of help in figuring out how to properly take care of multi-value parameter-included headers (like www-authenticate), especially when used inside strings as that could probably mitigate the use of AnyType entirely.

That being said, I understand it can't be full-proof, and therefore I think this is good as a baseline intellisense.

@GalacticHypernova GalacticHypernova marked this pull request as ready for review February 26, 2024 19:24
@pi0 pi0 changed the title feat(headers): intellisense overhaul feat: improve ide intellisense Jun 17, 2024
@pi0 pi0 changed the title feat: improve ide intellisense feat:improve typed headers Jun 17, 2024
@pi0 pi0 changed the title feat:improve typed headers feat: improve typed headers Jun 17, 2024
Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

(in summary made refactors to improve maintainability + relax types to only hint and still allow artibrary string/number as input)

@pi0 pi0 merged commit fb66924 into unjs:main Jun 17, 2024
3 checks passed
@GalacticHypernova
Copy link
Contributor Author

GalacticHypernova commented Jun 17, 2024

Glad I can help!
Though as an http framework, shouldn't it conform to http specs and enforce officially-recognized status codes? I can't seem to find much use in allowing arbitrary status codes..

@GalacticHypernova GalacticHypernova deleted the patch-3 branch June 17, 2024 17:55
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.

2 participants