fix: delay write cookie headers to first write
Overview
fixes https://github.com/BuilderIO/qwik/issues/5951
What is it?
- [ ] Feature / enhancement
- [x] Bug
- [ ] Docs / tests / types / typos
Description
set headers once but allow for dev to add cookie values
Deploy Preview for qwik-insights ready!
| Name | Link |
|---|---|
| Latest commit | ef2c3b24096d0a83ea7973fae84fc25ff3d3f361 |
| Latest deploy log | https://app.netlify.com/sites/qwik-insights/deploys/660a4e10b172b5000846e4d8 |
| Deploy Preview | https://deploy-preview-5986--qwik-insights.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
Deploying qwik-docs with
Cloudflare Pages
| Latest commit: |
ef2c3b2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0218750d.qwik-docs.pages.dev |
| Branch Preview URL: | https://fix-5951.qwik-docs.pages.dev |
@wmertens we need to delay writing to headers until we first "write" to the client. that way any app code can run and add cookie headers before we stream.
yes this applies to all headers. the e2e tests hang with this fix but when I run it locally it works correctly in preview-only. so there is something going on. @wmertens is there a better way to hook into this?
I don't think there's a better way because each adapter had their own way of sending cookies.
I think the e2e problem may be when there's no write and the response just closes without sending the cookies? Odd but maybe.
In any case you should handle once still being true on close.
@PatrickJS notice that the e2e tests fail on this
...but sadly the e2e fails. Try removing the once=false in close? Not sure that's needed or even desired
@wmertens I'm thinking e2e is waiting for headers which is why it fails during e2e but not when running locally. maybe there's another way
I think it's better having devs use plugin@<name>.ts and middleware for this