Skip to content

Fix headers on oxygen #1201

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 4, 2022
Merged

Fix headers on oxygen #1201

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b19d44
Fix headers on oxygen
blittle May 4, 2022
5c791ee
Cleanup
blittle May 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 13 additions & 49 deletions packages/hydrogen/src/entry-server.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ async function render(
* TODO: Also add `Vary` headers for `accept-language` and any other keys
* we want to shard our full-page cache for all Hydrogen storefronts.
*/
headers['cache-control'] = componentResponse.cacheControlHeader;
headers.set('cache-control', componentResponse.cacheControlHeader);

if (componentResponse.customBody) {
// This can be used to return sitemap.xml or any other custom response.
Expand All @@ -263,7 +263,7 @@ async function render(
});
}

headers[CONTENT_TYPE] = HTML_CONTENT_TYPE;
headers.set(CONTENT_TYPE, HTML_CONTENT_TYPE);

html = applyHtmlHead(html, request.ctx.head, template);

Expand Down Expand Up @@ -396,8 +396,10 @@ async function stream(
* queries which might be caught behind Suspense. Clarify this or add
* additional checks downstream?
*/
responseOptions.headers['cache-control'] =
componentResponse.cacheControlHeader;
responseOptions.headers.set(
'cache-control',
componentResponse.cacheControlHeader
);

if (isRedirect(responseOptions)) {
return false;
Expand All @@ -409,7 +411,7 @@ async function stream(
return false;
}

responseOptions.headers[CONTENT_TYPE] = HTML_CONTENT_TYPE;
responseOptions.headers.set(CONTENT_TYPE, HTML_CONTENT_TYPE);
writable.write(encoder.encode(DOCTYPE));

if (didError) {
Expand Down Expand Up @@ -475,20 +477,14 @@ async function stream(
}

if (await isStreamingSupported()) {
return new Response(transform.readable, {
...responseOptions,
headers: getHeaders(responseOptions.headers),
});
return new Response(transform.readable, responseOptions);
}

const bufferedBody = await bufferReadableStream(
transform.readable.getReader()
);

return new Response(bufferedBody, {
...responseOptions,
headers: getHeaders(responseOptions.headers),
});
return new Response(bufferedBody, responseOptions);
} else if (response) {
const {pipe} = ssrRenderToPipeableStream(AppSSR, {
nonce,
Expand Down Expand Up @@ -814,7 +810,7 @@ function startWritingHtmlToServerResponse(
}

type ResponseOptions = {
headers: Record<string, string>;
headers: Headers;
status: number;
statusText?: string;
};
Expand All @@ -825,20 +821,7 @@ function getResponseOptions(
) {
const responseInit = {} as ResponseOptions;

// @ts-ignore
responseInit.headers = Object.fromEntries(headers.entries());

// @ts-ignore
const rawHeaders = headers.raw();
// Warning! Headers.raw is non-standard and might disappear in undici or newer versions of node-fetch
// See: https://github.com/whatwg/fetch/issues/973
const setCookieKey = Object.keys(rawHeaders).find(
(key) => key.toLowerCase() === 'set-cookie'
);

if (setCookieKey) {
responseInit.headers['set-cookie'] = rawHeaders[setCookieKey];
}
responseInit.headers = headers;

if (error) {
responseInit.status = 500;
Expand Down Expand Up @@ -872,8 +855,8 @@ function writeHeadToServerResponse(
response.statusMessage = statusText;
}

Object.entries(headers).forEach(([key, value]) =>
response.setHeader(key, value)
Object.entries((headers as any).raw()).forEach(([key, value]) =>
response.setHeader(key, value as string)
Copy link
Contributor Author

@blittle blittle May 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

writeHeadToServerResponse only executes in node, because response is a node response object. So using .raw() should be fine row now, though a later version of node-fetch or undici might need to use something else.

);
}

Expand Down Expand Up @@ -930,22 +913,3 @@ function postRequestTasks(
logQueryTimings(type, request);
request.savePreloadQueries();
}

function getHeaders(rawHeaders: Record<string, String | Array<String>> = {}) {
const headers = new Headers();

for (const [key, values] of Object.entries(rawHeaders)) {
// values doesn't have an array prototype, so instanceof doesn't work.
// Check for .splice instead
// @ts-ignore
if (values?.splice) {
for (const value of values) {
headers.append(key, value as string);
}
} else {
headers.append(key, values as string);
}
}

return headers;
}