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: add event.method and event.headers #429

Merged
merged 7 commits into from
Jul 10, 2023
Merged

feat: add event.method and event.headers #429

merged 7 commits into from
Jul 10, 2023

Conversation

Hebilicious
Copy link
Member

@Hebilicious Hebilicious commented Jul 6, 2023

πŸ”— Linked issue

#423, #73

❓ 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 adds event.method and event.headers support.

πŸ“ Checklist

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

@Hebilicious Hebilicious mentioned this pull request Jul 6, 2023
8 tasks
@Hebilicious Hebilicious requested a review from pi0 July 6, 2023 15:27
@Hebilicious Hebilicious added events-api ready enhancement New feature or request labels Jul 6, 2023
@pi0 pi0 changed the title feat: add method and headers to event feat: add event.method and event.headers Jul 10, 2023
@codecov
Copy link

codecov bot commented Jul 10, 2023

Codecov Report

Merging #429 (2361c85) into main (fb9e3e3) will decrease coverage by 1.66%.
The diff coverage is 100.00%.

❗ Current head 2361c85 differs from pull request most recent head 80aed9e. Consider uploading reports for the commit 80aed9e to get more accurate results

@@            Coverage Diff             @@
##             main     #429      +/-   ##
==========================================
- Coverage   76.82%   75.16%   -1.66%     
==========================================
  Files          26       26              
  Lines        2554     2569      +15     
  Branches      367      369       +2     
==========================================
- Hits         1962     1931      -31     
- Misses        592      638      +46     
Impacted Files Coverage Ξ”
src/event/event.ts 57.75% <100.00%> (+6.27%) ⬆️

... and 3 files with indirect coverage changes

@pi0
Copy link
Member

pi0 commented Jul 10, 2023

Thanks for the PR!

I've made a couple of updates to use a getter with lazy evaluation and keep the public interface read-only. Similar to _handled we have an internal way to override them using event._method and event._headers -- Not to be advised --

Fix tests for Node 16 not having global Headers. In H3 we have a working polyfill but thinking to node depend on for best web compatible behavior.

@pi0 pi0 merged commit f74a9b5 into main Jul 10, 2023
@pi0 pi0 deleted the event-method-headers branch July 10, 2023 12:41
@pi0 pi0 mentioned this pull request Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants