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(headers): intellisense overhaul #625

Open
wants to merge 88 commits into
base: main
Choose a base branch
from

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

Attention: Patch coverage is 98.50746% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 78.72%. Comparing base (a58d7c9) to head (c025996).
Report is 33 commits behind head on main.

Files Patch % Lines
src/types.mimes.ts 0.00% 1 Missing and 1 partial ⚠️
src/adapters/plain.ts 50.00% 1 Missing ⚠️
src/utils/cors/handler.ts 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #625      +/-   ##
==========================================
+ Coverage   77.83%   78.72%   +0.88%     
==========================================
  Files          47       56       +9     
  Lines        4286     5433    +1147     
  Branches      611      684      +73     
==========================================
+ Hits         3336     4277     +941     
- Misses        933     1135     +202     
- Partials       17       21       +4     

β˜” 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 GalacticHypernova marked this pull request as ready for review February 26, 2024 18:38
@GalacticHypernova GalacticHypernova marked this pull request as draft February 26, 2024 18:39
@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
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

1 participant