Skip to content

Commit f9b8411

Browse files
kdaviduikkdaviduik
committed
Align createRequestHandler with PR #3309
Updates both @Shopify/hydrogen and @shopify/remix-oxygen createRequestHandler to match the implementation in PR #3309: - Add collectTrackingInformation option (default: true) - Add warning when storefront instance is missing from context - Use appendServerTimingHeader utility instead of manual header - Move poweredByHeader append to after tracking header processing - Add TODO comments for future major version changes - Improve JSDoc documentation for all options
1 parent 456ad3c commit f9b8411

File tree

2 files changed

+108
-33
lines changed

2 files changed

+108
-33
lines changed

‎packages/hydrogen/src/createRequestHandler.ts‎

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,32 @@ import {
44
type ServerBuild,
55
} from '@remix-run/server-runtime';
66
import {HYDROGEN_SFAPI_PROXY_KEY} from './constants';
7+
import {appendServerTimingHeader} from './utils/server-timing';
8+
import {warnOnce} from './utils/warning';
79

810
type CreateRequestHandlerOptions<Context = unknown> = {
11+
/** Remix's server build */
912
build: ServerBuild;
13+
/** Remix's mode */
1014
mode?: string;
15+
/**
16+
* Function to provide the load context for each request.
17+
* It must contain Hydrogen's storefront client instance
18+
* for other Hydrogen utilities to work properly.
19+
*/
20+
getLoadContext?: (request: Request) => Promise<Context> | Context;
1121
/**
1222
* Whether to include the `powered-by` header in responses
1323
* @default true
1424
*/
1525
poweredByHeader?: boolean;
1626
/**
17-
* Function to provide the load context for each request.
18-
* It must contain Hydrogen's storefront client instance
19-
* for other Hydrogen utilities to work properly.
27+
* Collect tracking information from subrequests such as cookies
28+
* and forward them to the browser. Disable this if you are not
29+
* using Hydrogen's built-in analytics.
30+
* @default true
2031
*/
21-
getLoadContext?: (request: Request) => Promise<Context> | Context;
32+
collectTrackingInformation?: boolean;
2233
/**
2334
* Whether to proxy standard routes such as `/api/.../graphql.json` (Storefront API).
2435
* You can disable this if you are handling these routes yourself. Ensure that
@@ -36,6 +47,7 @@ export function createRequestHandler<Context = unknown>({
3647
mode,
3748
poweredByHeader = true,
3849
getLoadContext,
50+
collectTrackingInformation = true,
3951
proxyStorefrontApiRequests = true,
4052
}: CreateRequestHandlerOptions<Context>) {
4153
const handleRequest = createRemixRequestHandler(build, mode);
@@ -74,21 +86,30 @@ export function createRequestHandler<Context = unknown>({
7486
context as {storefront?: StorefrontForProxy} | undefined
7587
)?.storefront;
7688

77-
// Proxy Storefront API requests
78-
if (proxyStorefrontApiRequests && storefront?.isStorefrontApiUrl(request)) {
79-
const response = await storefront.forward(request);
80-
appendPoweredByHeader?.(response);
81-
return response;
89+
if (proxyStorefrontApiRequests) {
90+
if (!storefront) {
91+
// TODO: this should throw error in future major version
92+
warnOnce(
93+
'[h2:createRequestHandler] Storefront instance is required to proxy standard routes.',
94+
);
95+
}
96+
97+
// Proxy Storefront API requests
98+
if (storefront?.isStorefrontApiUrl(request)) {
99+
const response = await storefront.forward(request);
100+
appendPoweredByHeader?.(response);
101+
return response;
102+
}
82103
}
83104

84105
const response = await handleRequest(request, context);
85106

86-
appendPoweredByHeader?.(response);
87-
88-
// Collect tracking headers from storefront subrequests
89-
if (proxyStorefrontApiRequests && storefront) {
90-
storefront.setCollectedSubrequestHeaders(response);
107+
if (storefront && proxyStorefrontApiRequests) {
108+
if (collectTrackingInformation) {
109+
storefront.setCollectedSubrequestHeaders(response);
110+
}
91111

112+
// TODO: assume SFAPI proxy is available in future major version
92113
// Signal that SFAPI proxy is enabled for document requests.
93114
// Note: sec-fetch-dest is automatically added by modern browsers,
94115
// but we also check the Accept header for other clients.
@@ -97,13 +118,12 @@ export function createRequestHandler<Context = unknown>({
97118
(fetchDest && fetchDest === 'document') ||
98119
request.headers.get('accept')?.includes('text/html')
99120
) {
100-
response.headers.append(
101-
'Server-Timing',
102-
`${HYDROGEN_SFAPI_PROXY_KEY};desc=1`,
103-
);
121+
appendServerTimingHeader(response, {[HYDROGEN_SFAPI_PROXY_KEY]: '1'});
104122
}
105123
}
106124

125+
appendPoweredByHeader?.(response);
126+
107127
return response;
108128
};
109129
}

‎packages/remix-oxygen/src/server.ts‎

Lines changed: 70 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,57 @@ Error.prototype.toString = function () {
1414
/** Server-Timing header key to signal that the SFAPI proxy is enabled */
1515
const HYDROGEN_SFAPI_PROXY_KEY = '_sfapi_proxy';
1616

17+
let hasWarnedAboutStorefront = false;
18+
19+
function warnOnce(message: string) {
20+
if (!hasWarnedAboutStorefront) {
21+
hasWarnedAboutStorefront = true;
22+
console.warn(message);
23+
}
24+
}
25+
26+
function buildServerTimingHeader(values: Record<string, string | undefined>) {
27+
return Object.entries(values)
28+
.map(([key, value]) => (value ? `${key};desc=${value}` : undefined))
29+
.filter(Boolean)
30+
.join(', ');
31+
}
32+
33+
function appendServerTimingHeader(
34+
response: {headers: Headers},
35+
values: string | Record<string, string | undefined>,
36+
) {
37+
const header =
38+
typeof values === 'string' ? values : buildServerTimingHeader(values);
39+
40+
if (header) {
41+
response.headers.append('Server-Timing', header);
42+
}
43+
}
44+
1745
type CreateRequestHandlerOptions<Context = unknown> = {
46+
/** Remix's server build */
1847
build: ServerBuild;
48+
/** Remix's mode */
1949
mode?: string;
20-
poweredByHeader?: boolean;
50+
/**
51+
* Function to provide the load context for each request.
52+
* It must contain Hydrogen's storefront client instance
53+
* for other Hydrogen utilities to work properly.
54+
*/
2155
getLoadContext?: (request: Request) => Promise<Context> | Context;
56+
/**
57+
* Whether to include the `powered-by` header in responses
58+
* @default true
59+
*/
60+
poweredByHeader?: boolean;
61+
/**
62+
* Collect tracking information from subrequests such as cookies
63+
* and forward them to the browser. Disable this if you are not
64+
* using Hydrogen's built-in analytics.
65+
* @default true
66+
*/
67+
collectTrackingInformation?: boolean;
2268
/**
2369
* Whether to proxy standard routes such as `/api/.../graphql.json` (Storefront API).
2470
* You can disable this if you are handling these routes yourself. Ensure that
@@ -33,6 +79,7 @@ export function createRequestHandler<Context = unknown>({
3379
mode,
3480
poweredByHeader = true,
3581
getLoadContext,
82+
collectTrackingInformation = true,
3683
proxyStorefrontApiRequests = true,
3784
}: CreateRequestHandlerOptions<Context>) {
3885
const handleRequest = createRemixRequestHandler(build, mode);
@@ -71,11 +118,20 @@ export function createRequestHandler<Context = unknown>({
71118
context as {storefront?: StorefrontForProxy} | undefined
72119
)?.storefront;
73120

74-
// Proxy Storefront API requests
75-
if (proxyStorefrontApiRequests && storefront?.isStorefrontApiUrl(request)) {
76-
const response = await storefront.forward(request);
77-
appendPoweredByHeader?.(response);
78-
return response;
121+
if (proxyStorefrontApiRequests) {
122+
if (!storefront) {
123+
// TODO: this should throw error in future major version
124+
warnOnce(
125+
'[h2:createRequestHandler] Storefront instance is required to proxy standard routes.',
126+
);
127+
}
128+
129+
// Proxy Storefront API requests
130+
if (storefront?.isStorefrontApiUrl(request)) {
131+
const response = await storefront.forward(request);
132+
appendPoweredByHeader?.(response);
133+
return response;
134+
}
79135
}
80136

81137
if (process.env.NODE_ENV === 'development' && context) {
@@ -89,12 +145,12 @@ export function createRequestHandler<Context = unknown>({
89145

90146
const response = await handleRequest(request, context);
91147

92-
appendPoweredByHeader?.(response);
93-
94-
// Collect tracking headers from storefront subrequests
95-
if (proxyStorefrontApiRequests && storefront) {
96-
storefront.setCollectedSubrequestHeaders(response);
148+
if (storefront && proxyStorefrontApiRequests) {
149+
if (collectTrackingInformation) {
150+
storefront.setCollectedSubrequestHeaders(response);
151+
}
97152

153+
// TODO: assume SFAPI proxy is available in future major version
98154
// Signal that SFAPI proxy is enabled for document requests.
99155
// Note: sec-fetch-dest is automatically added by modern browsers,
100156
// but we also check the Accept header for other clients.
@@ -103,13 +159,12 @@ export function createRequestHandler<Context = unknown>({
103159
(fetchDest && fetchDest === 'document') ||
104160
request.headers.get('accept')?.includes('text/html')
105161
) {
106-
response.headers.append(
107-
'Server-Timing',
108-
`${HYDROGEN_SFAPI_PROXY_KEY};desc=1`,
109-
);
162+
appendServerTimingHeader(response, {[HYDROGEN_SFAPI_PROXY_KEY]: '1'});
110163
}
111164
}
112165

166+
appendPoweredByHeader?.(response);
167+
113168
if (process.env.NODE_ENV === 'development') {
114169
globalThis.__H2O_LOG_EVENT?.({
115170
eventType: 'request',

0 commit comments

Comments
 (0)