-
Notifications
You must be signed in to change notification settings - Fork 520
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: extended cloudflare context #677
Conversation
src/runtime/entries/cloudflare.ts
Outdated
function toNodeListener (app: App, context: H3EventContext): NodeListener { | ||
const toNodeHandle: NodeListener = async function (req, res) { | ||
const event = createEvent(req, res) | ||
event.context = { ...event.context, ...context } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we might want to add CF context as event.context.cloudflare
for better type support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
event.context = { ...event.context, ...context } | |
event.context = { ...event.context, cloudflare: context } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops - didn't mean to edit. What about?
event.context = { ...event.context, ...context } | |
event.context.cloudflare = context |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for feedback, implemented π
Codecov Report
@@ Coverage Diff @@
## main #677 +/- ##
=======================================
Coverage 66.91% 66.91%
=======================================
Files 57 57
Lines 4232 4232
Branches 459 460 +1
=======================================
Hits 2832 2832
Misses 1397 1397
Partials 3 3 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
function toNodeListener (app: App, context: H3EventContext): NodeListener { | ||
const toNodeHandle: NodeListener = async function (req, res) { | ||
const event = createEvent(req, res) | ||
event.context.cloudflare = context |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be:
event.context.cloudflare = context | |
event.context.cloudflare = context.cf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the answer to this depends on where you'll want the fetch event context to live when module syntax support lands in #681.
Thanks for working on this @barisbora β€οΈ Adding support with a slightly more generic approach in #927 |
π Linked issue
resolves #391
β Type of change
π Description
Extending context object with cloudflare CF Properties and headers
π Checklist