diff --git a/.changeset/four-bags-judge.md b/.changeset/four-bags-judge.md new file mode 100644 index 0000000000..b9a3eec2aa --- /dev/null +++ b/.changeset/four-bags-judge.md @@ -0,0 +1,5 @@ +--- +'@shopify/hydrogen': minor +--- + +Fixes an issue where cached sub-requests were not revalidating properly. diff --git a/packages/hydrogen/src/entry-server.tsx b/packages/hydrogen/src/entry-server.tsx index 56d4064e6a..63720e50ba 100644 --- a/packages/hydrogen/src/entry-server.tsx +++ b/packages/hydrogen/src/entry-server.tsx @@ -616,10 +616,11 @@ async function hydrate( ); const streamer = rscWriter.renderToPipeableStream(AppRSC); + const stream = streamer.pipe(response) as Writable; + response.writeHead(200, 'ok', { 'cache-control': componentResponse.cacheControlHeader, }); - const stream = streamer.pipe(response) as Writable; stream.on('finish', function () { postRequestTasks('rsc', response.statusCode, request, componentResponse); diff --git a/packages/hydrogen/src/foundation/useQuery/hooks.ts b/packages/hydrogen/src/foundation/useQuery/hooks.ts index 02ee7c9314..331e23832b 100644 --- a/packages/hydrogen/src/foundation/useQuery/hooks.ts +++ b/packages/hydrogen/src/foundation/useQuery/hooks.ts @@ -3,7 +3,6 @@ import { getLoggerWithContext, collectQueryCacheControlHeaders, collectQueryTimings, - logCacheApiStatus, } from '../../utilities/log'; import { deleteItemFromCache, @@ -11,10 +10,10 @@ import { getItemFromCache, isStale, setItemInCache, -} from '../../framework/cache'; -import {hashKey} from '../../utilities/hash'; +} from '../../framework/cache-sub-request'; import {runDelayedFunction} from '../../framework/runtime'; import {useRequestCacheData, useServerRequest} from '../ServerRequestProvider'; +import {CacheSeconds} from '../../framework/CachingStrategy'; export interface HydrogenUseQueryOptions { /** The [caching strategy](https://shopify.dev/custom-storefronts/hydrogen/framework/cache#caching-strategies) to help you @@ -90,7 +89,6 @@ function cachedQueryFnBuilder( // to prevent losing the current React cycle. const request = useServerRequest(); const log = getLoggerWithContext(request); - const hashedKey = hashKey(key); const cacheResponse = await getItemFromCache(key); @@ -110,16 +108,20 @@ function cachedQueryFnBuilder( /** * Important: Do this async */ - if (isStale(response, resolvedQueryOptions?.cache)) { - logCacheApiStatus('STALE', hashedKey); - const lockKey = `lock-${key}`; + if (isStale(key, response)) { + const lockKey = ['lock', ...(typeof key === 'string' ? [key] : key)]; runDelayedFunction(async () => { - logCacheApiStatus('UPDATING', hashedKey); const lockExists = await getItemFromCache(lockKey); if (lockExists) return; - await setItemInCache(lockKey, true); + await setItemInCache( + lockKey, + true, + CacheSeconds({ + maxAge: 10, + }) + ); try { const output = await generateNewOutput(); diff --git a/packages/hydrogen/src/framework/Hydration/ServerComponentResponse.server.ts b/packages/hydrogen/src/framework/Hydration/ServerComponentResponse.server.ts index bdd1a578b5..3a576ebbc2 100644 --- a/packages/hydrogen/src/framework/Hydration/ServerComponentResponse.server.ts +++ b/packages/hydrogen/src/framework/Hydration/ServerComponentResponse.server.ts @@ -6,7 +6,7 @@ import React from 'react'; export class ServerComponentResponse extends Response { private wait = false; - private cacheOptions?: CachingStrategy; + private cacheOptions: CachingStrategy = CacheSeconds(); public customStatus?: {code?: number; text?: string}; @@ -32,7 +32,7 @@ export class ServerComponentResponse extends Response { } get cacheControlHeader(): string { - return generateCacheControlHeader(this.cacheOptions || CacheSeconds()); + return generateCacheControlHeader(this.cacheOptions); } writeHead({ diff --git a/packages/hydrogen/src/framework/cache-sub-request.ts b/packages/hydrogen/src/framework/cache-sub-request.ts new file mode 100644 index 0000000000..1542686de4 --- /dev/null +++ b/packages/hydrogen/src/framework/cache-sub-request.ts @@ -0,0 +1,95 @@ +import type {QueryKey, CachingStrategy, AllCacheOptions} from '../types'; +import {getCache} from './runtime'; +import {hashKey} from '../utilities/hash'; +import * as CacheApi from './cache'; +import {CacheSeconds} from './CachingStrategy'; + +/** + * Wrapper Cache functions for sub queries + */ + +/** + * Cache API is weird. We just need a full URL, so we make one up. + */ +function getKeyUrl(key: string) { + return `https://shopify.dev/?${key}`; +} + +function getCacheOption(userCacheOptions?: CachingStrategy): AllCacheOptions { + return userCacheOptions || CacheSeconds(); +} + +export function generateSubRequestCacheControlHeader( + userCacheOptions?: CachingStrategy +): string { + return CacheApi.generateDefaultCacheControlHeader( + getCacheOption(userCacheOptions) + ); +} + +/** + * Get an item from the cache. If a match is found, returns a tuple + * containing the `JSON.parse` version of the response as well + * as the response itself so it can be checked for staleness. + */ +export async function getItemFromCache( + key: QueryKey +): Promise { + const cache = getCache(); + + if (!cache) { + return; + } + + const url = getKeyUrl(hashKey(key)); + const request = new Request(url); + + const response = await CacheApi.getItemFromCache(request); + + if (!response) { + return; + } + + return [await response.json(), response]; +} + +/** + * Put an item into the cache. + */ +export async function setItemInCache( + key: QueryKey, + value: any, + userCacheOptions?: CachingStrategy +) { + const cache = getCache(); + if (!cache) { + return; + } + + const url = getKeyUrl(hashKey(key)); + const request = new Request(url); + const response = new Response(JSON.stringify(value)); + + await CacheApi.setItemInCache( + request, + response, + getCacheOption(userCacheOptions) + ); +} + +export async function deleteItemFromCache(key: QueryKey) { + const cache = getCache(); + if (!cache) return; + + const url = getKeyUrl(hashKey(key)); + const request = new Request(url); + + await CacheApi.deleteItemFromCache(request); +} + +/** + * Manually check the response to see if it's stale. + */ +export function isStale(key: QueryKey, response: Response) { + return CacheApi.isStale(new Request(getKeyUrl(hashKey(key))), response); +} diff --git a/packages/hydrogen/src/framework/cache.ts b/packages/hydrogen/src/framework/cache.ts index 9056fb5428..87d1d895bb 100644 --- a/packages/hydrogen/src/framework/cache.ts +++ b/packages/hydrogen/src/framework/cache.ts @@ -1,10 +1,9 @@ -import type {QueryKey, CachingStrategy} from '../types'; +import type {CachingStrategy} from '../types'; import {getCache} from './runtime'; import { CacheSeconds, generateCacheControlHeader, } from '../framework/CachingStrategy'; -import {hashKey} from '../utilities/hash'; import {logCacheApiStatus} from '../utilities/log'; function getCacheControlSetting( @@ -21,62 +20,49 @@ function getCacheControlSetting( } } -export function generateSubRequestCacheControlHeader( +export function generateDefaultCacheControlHeader( userCacheOptions?: CachingStrategy ): string { return generateCacheControlHeader(getCacheControlSetting(userCacheOptions)); } -/** - * Cache API is weird. We just need a full URL, so we make one up. - */ -function getKeyUrl(key: string) { - return `https://shopify.dev/?${key}`; -} - /** * Get an item from the cache. If a match is found, returns a tuple * containing the `JSON.parse` version of the response as well * as the response itself so it can be checked for staleness. */ export async function getItemFromCache( - key: QueryKey -): Promise { + request: Request +): Promise { const cache = getCache(); if (!cache) { return; } - const url = getKeyUrl(hashKey(key)); - const request = new Request(url); - const response = await cache.match(request); if (!response) { - logCacheApiStatus('MISS', url); + logCacheApiStatus('MISS', request.url); return; } - logCacheApiStatus('HIT', url); + logCacheApiStatus('HIT', request.url); - return [await response.json(), response]; + return response; } /** * Put an item into the cache. */ export async function setItemInCache( - key: QueryKey, - value: any, - userCacheOptions?: CachingStrategy + request: Request, + response: Response, + userCacheOptions: CachingStrategy ) { const cache = getCache(); if (!cache) { return; } - const url = getKeyUrl(hashKey(key)); - const request = new Request(url); - /** * We are manually managing staled request by adding this workaround. * Why? cache control header support is dependent on hosting platform @@ -115,49 +101,70 @@ export async function setItemInCache( * * `isStale` function will use the above information to test for stale-ness of a cached response */ + const cacheControl = getCacheControlSetting(userCacheOptions); - const headers = new Headers({ - 'cache-control': generateSubRequestCacheControlHeader( + + // The padded cache-control to mimic stale-while-revalidate + request.headers.set( + 'cache-control', + generateDefaultCacheControlHeader( getCacheControlSetting(cacheControl, { maxAge: (cacheControl.maxAge || 0) + (cacheControl.staleWhileRevalidate || 0), }) - ), - 'cache-put-date': new Date().toUTCString(), - }); - - const response = new Response(JSON.stringify(value), {headers}); - - logCacheApiStatus('PUT', url); + ) + ); + // The cache-control we want to set on response + const cacheControlString = generateDefaultCacheControlHeader( + getCacheControlSetting(cacheControl) + ); + + // CF will override cache-control, so we need to keep a + // non-modified real-cache-control + response.headers.set('cache-control', cacheControlString); + response.headers.set('real-cache-control', cacheControlString); + response.headers.set('cache-put-date', new Date().toUTCString()); + + logCacheApiStatus('PUT', request.url); await cache.put(request, response); } -export async function deleteItemFromCache(key: QueryKey) { +export async function deleteItemFromCache(request: Request) { const cache = getCache(); if (!cache) return; - const url = getKeyUrl(hashKey(key)); - const request = new Request(url); - - logCacheApiStatus('DELETE', url); + logCacheApiStatus('DELETE', request.url); await cache.delete(request); } /** * Manually check the response to see if it's stale. */ -export function isStale( - response: Response, - userCacheOptions?: CachingStrategy -) { - const responseMaxAge = getCacheControlSetting(userCacheOptions).maxAge || 0; +export function isStale(request: Request, response: Response) { const responseDate = response.headers.get('cache-put-date'); + const cacheControl = response.headers.get('real-cache-control'); + let responseMaxAge = 0; + + if (cacheControl) { + const maxAgeMatch = cacheControl.match(/max-age=(\d*)/); + if (maxAgeMatch && maxAgeMatch.length > 1) { + responseMaxAge = parseFloat(maxAgeMatch[1]); + } + } - if (!responseDate) return false; + if (!responseDate) { + return false; + } const ageInMs = new Date().valueOf() - new Date(responseDate as string).valueOf(); const age = ageInMs / 1000; - return age > responseMaxAge; + const result = age > responseMaxAge; + + if (result) { + logCacheApiStatus('STALE', request.url); + } + + return result; } diff --git a/packages/hydrogen/src/framework/cache/in-memory.ts b/packages/hydrogen/src/framework/cache/in-memory.ts index 733525add2..ffc45630fc 100644 --- a/packages/hydrogen/src/framework/cache/in-memory.ts +++ b/packages/hydrogen/src/framework/cache/in-memory.ts @@ -1,5 +1,3 @@ -import {logCacheApiStatus} from '../../utilities/log'; - type CacheMatch = { value: Response; date: Date; @@ -17,7 +15,6 @@ export class InMemoryCache { } put(request: Request, response: Response) { - logCacheApiStatus('PUT-dev', request.url); this.store.set(request.url, { value: response, date: new Date(), @@ -28,7 +25,6 @@ export class InMemoryCache { const match = this.store.get(request.url); if (!match) { - logCacheApiStatus('MISS-dev', request.url); return; } @@ -47,7 +43,6 @@ export class InMemoryCache { const isMiss = age > maxAge + swr; if (isMiss) { - logCacheApiStatus('MISS-dev', request.url); this.store.delete(request.url); return; } @@ -57,7 +52,6 @@ export class InMemoryCache { const headers = new Headers(value.headers); headers.set('cache', isStale ? 'STALE' : 'HIT'); headers.set('date', date.toUTCString()); - logCacheApiStatus(`${headers.get('cache')}-dev`, request.url); const response = new Response(value.body, { headers, @@ -68,7 +62,6 @@ export class InMemoryCache { delete(request: Request) { this.store.delete(request.url); - logCacheApiStatus('DELETE-dev', request.url); } keys(request?: Request) { diff --git a/packages/hydrogen/src/framework/tests/cache.test.ts b/packages/hydrogen/src/framework/tests/cache.test.ts index 0f5afe460e..9a19026606 100644 --- a/packages/hydrogen/src/framework/tests/cache.test.ts +++ b/packages/hydrogen/src/framework/tests/cache.test.ts @@ -1,4 +1,4 @@ -import {generateSubRequestCacheControlHeader} from '../cache'; +import {generateSubRequestCacheControlHeader} from '../cache-sub-request'; import {CacheSeconds, NoStore} from '../CachingStrategy'; describe('generateSubRequestCacheControlHeader', () => { diff --git a/packages/hydrogen/src/utilities/log/log-cache-api-status.ts b/packages/hydrogen/src/utilities/log/log-cache-api-status.ts index 25cebad9de..49a1ca9103 100644 --- a/packages/hydrogen/src/utilities/log/log-cache-api-status.ts +++ b/packages/hydrogen/src/utilities/log/log-cache-api-status.ts @@ -7,5 +7,16 @@ export function logCacheApiStatus(status: string | null, url: string) { return; } - log.debug(gray(`[Cache] ${status?.padEnd(8)} query ${findQueryName(url)}`)); + let queryName: string | undefined; + if (/shopify\.dev/.test(url)) { + queryName = findQueryName(url); + } + + log.debug( + gray( + `[Cache] ${status?.padEnd(8)} ${ + queryName ? `query ${queryName}` : decodeURIComponent(url) + }` + ) + ); }