cache-control and stale-while-revalidate fixes

.changeset/four-bags-judge.md

@@ -0,0 +1,7 @@
+---
+'@shopify/hydrogen': minor
+---
+
+Fixes:
+
+- Sub queries was not revalidating properly

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);

packages/hydrogen/src/foundation/useQuery/hooks.ts

@@ -3,18 +3,17 @@ import {
   getLoggerWithContext,
   collectQueryCacheControlHeaders,
   collectQueryTimings,
-  logCacheApiStatus,
 } from '../../utilities/log';
 import {
   deleteItemFromCache,
   generateSubRequestCacheControlHeader,
   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<T>(
     // 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<T>(
       /**
        * 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();
 

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({

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<undefined | [any, Response]> {
+  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);
+}

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<undefined | [any, Response]> {
+  request: Request
+): Promise<Response | undefined> {
   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;
 }