-
Notifications
You must be signed in to change notification settings - Fork 280
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
Support withCache
in debug-network for workerd
#1438
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
--- | ||
'@shopify/hydrogen': patch | ||
--- | ||
|
||
Calls to `withCache` can now be shown in the `/debug-network` tool when using the Worker runtime. For this to work, use the new `request` parameter in `createWithCache`: | ||
|
||
```diff | ||
export default { | ||
fetch(request, env, executionContext) { | ||
// ... | ||
const withCache = createWithCache({ | ||
cache, | ||
waitUntil, | ||
+ request, | ||
}); | ||
// ... | ||
}, | ||
} | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,39 @@ | ||
import {type CacheKey, runWithCache} from './cache/fetch'; | ||
import type {CachingStrategy} from './cache/strategies'; | ||
|
||
type CrossRuntimeRequest = { | ||
headers: { | ||
get?: (key: string) => string | null | undefined; | ||
[key: string]: any; | ||
}; | ||
}; | ||
|
||
type CreateWithCacheOptions = { | ||
/** An instance that implements the [Cache API](https://developer.mozilla.org/en-US/docs/Web/API/Cache) */ | ||
cache: Cache; | ||
/** The `waitUntil` function is used to keep the current request/response lifecycle alive even after a response has been sent. It should be provided by your platform. */ | ||
waitUntil: ExecutionContext['waitUntil']; | ||
/** The `request` object is used to access certain headers for debugging */ | ||
request?: CrossRuntimeRequest; | ||
}; | ||
|
||
function getHeader(key: string, request?: CrossRuntimeRequest) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In Oxygen yes but in Node environments they will be passing I assume Bun and Deno probably have similar APIs for requests, either Web-like or Node-like, but maybe with different type name. |
||
const value = request?.headers?.get?.(key) ?? request?.headers?.[key]; | ||
return typeof value === 'string' ? value : undefined; | ||
} | ||
|
||
/** | ||
* Creates a utility function that executes an asynchronous operation | ||
* like `fetch` and caches the result according to the strategy provided. | ||
* Use this to call any third-party APIs from loaders or actions. | ||
* By default, it uses the `CacheShort` strategy. | ||
* | ||
*/ | ||
export function createWithCache<T = unknown>( | ||
options: CreateWithCacheOptions, | ||
): CreateWithCacheReturn<T> { | ||
const {cache, waitUntil} = options; | ||
export function createWithCache<T = unknown>({ | ||
cache, | ||
waitUntil, | ||
request, | ||
}: CreateWithCacheOptions): CreateWithCacheReturn<T> { | ||
return function withCache<T = unknown>( | ||
cacheKey: CacheKey, | ||
strategy: CachingStrategy, | ||
|
@@ -28,7 +43,10 @@ export function createWithCache<T = unknown>( | |
strategy, | ||
cacheInstance: cache, | ||
waitUntil, | ||
debugInfo: {}, | ||
debugInfo: { | ||
requestId: getHeader('request-id', request), | ||
purpose: getHeader('purpose', request), | ||
}, | ||
}); | ||
}; | ||
} | ||
|
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.
Are we losing
this
context without thebind
?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.
Yes, we already use
.bind
in all our templates so I just updated it here as well.