-
Notifications
You must be signed in to change notification settings - Fork 3
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
refactor(vite-node-miniflare): replace hattip with h3 for internal middleware #135
Conversation
const app = h3.createApp().use([ | ||
h3.eventHandler((event) => | ||
viteNodeServerRpc.requestHandler({ | ||
request: h3.toWebRequest(event), |
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.
This might be due to calling toWebRequest
twice in custom handler?
EDIT: temporary workaround? fa72b7c
const request = h3.toWebRequest(event) as any as MiniflareRequest; | ||
return miniflare.dispatchFetch(request.url, { |
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.
Another blocker... Miniflare probably uses its own Request/Response
polyfills and it breaks h3's isWebResponse
check https://github.com/unjs/h3/blob/53703dc860f1ff6fe7ce71d543deff1cfa810b11/src/utils/response.ts#L209-L211
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.
Fixed by 65258b0
Closing as it's obsolete |
It's a little ergonomically nicer to just install
h3
(instead of multiple@hattip/....
packages), but it seem proxyingrequest.body
is not working with miniflare?error log
Also, response headers aren't propagated properly, which breaks data request convention using
x-loader-response
header?curl examples