-
Notifications
You must be signed in to change notification settings - Fork 501
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
fix: split cookie headers #1452
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1452 +/- ##
==========================================
- Coverage 76.17% 76.09% -0.09%
==========================================
Files 73 73
Lines 7437 7428 -9
Branches 728 727 -1
==========================================
- Hits 5665 5652 -13
- Misses 1771 1774 +3
- Partials 1 2 +1 |
Thanks. I think we can add fix to a wrapper of localFetch util directly so that entries using it already have fix |
We can do it like this : function normalizedHeaders(fetch: typeof globalThis.fetch) {
return async (...args: Parameters<typeof fetch>) => {
const r = await fetch(...args);
return new Response(r.body, {
headers: normalizeOutgoingHeaders(r.headers),
status: r.status,
statusText: r.statusText,
});
};
}
// Create local fetch callers
const localCall = createCall(toNodeListener(h3App) as any);
const localFetch = normalizedHeaders(
createLocalFetch(localCall, globalThis.fetch)
); I'm worried about the possible impact of changing |
Yes, i still think wrapper method would be better idea. |
Is there any reason why cloudflare, lagon, denos, netlify-edge, service worker and vercel-edge presets are using localCall ? They are all returning a Response. |
They need to be upgraded to localFetch. free to include fix in same PR (basically i made that refactor back in time for other presets to prepare for this unified fix :D) |
@danielroe tagging you for the Azure changes |
π Linked issue
Closes #989
Fix #988
β Type of change
π Description
Continuing the work done by @oleghalin and applying it more generally to every presets that needs it.
Note that the culprit for this would be
unenv
, but fixing this in Nitro allows us to not add any dependency tounenv
.π Checklist