Refactor and test cache

.changeset/plenty-clocks-hide.md

@@ -0,0 +1,5 @@
+---
+'@shopify/hydrogen': patch
+---
+
+Fix a small rounding issue when checking stale-while-revalidate timing.

packages/hydrogen/src/cache/api.ts

@@ -108,11 +108,7 @@ async function setItem(
    *
    * Store the following information in the response header:
    *
-   *   cache-put-date   - UTC time string of when this request is PUT into cache
-   *
-   * Note on `cache-put-date`: The `response.headers.get('date')` isn't static. I am
-   * not positive what date this is returning but it is never over 500 ms
-   * after subtracting from the current timestamp.
+   *   cache-put-date   - Timestamp string of when this request is PUT into cache
    *
    * `isStale` function will use the above information to test for stale-ness of a cached response
    */
@@ -135,7 +131,7 @@ async function setItem(
   // cache-control is still necessary for mini-oxygen
   response.headers.set('cache-control', paddedCacheControlString);
   response.headers.set('real-cache-control', cacheControlString);
-  response.headers.set('cache-put-date', new Date().toUTCString());
+  response.headers.set('cache-put-date', String(Date.now()));
 
   logCacheApiStatus('PUT', request, response);
   await cache.put(request, response);
@@ -159,8 +155,7 @@ function calculateAge(response: Response, responseDate: string) {
     }
   }
 
-  const ageInMs =
-    new Date().valueOf() - new Date(responseDate as string).valueOf();
+  const ageInMs = Date.now() - Number(responseDate as string);
   return [ageInMs / 1000, responseMaxAge];
 }
 

packages/hydrogen/src/with-cache.doc.ts → packages/hydrogen/src/cache/create-with-cache.doc.ts

@@ -6,20 +6,20 @@ const data: ReferenceEntityTemplateSchema = {
   subCategory: 'caching',
   isVisualComponent: false,
   related: [],
-  description: `Creates a utility function that executes an asynchronous operation \n like \`fetch\` and caches the result according to the strategy provided.\nUse this to call any third-party APIs from loaders or actions.\nBy default, it uses the \`CacheShort\` strategy.`,
+  description: `Creates a utility function that executes an asynchronous operation \n like \`fetch\` and caches the result according to the strategy provided.\nUse this to call any third-party APIs from loaders or actions.`,
   type: 'utility',
   defaultExample: {
     description: 'I am the default example',
     codeblock: {
       tabs: [
         {
           title: 'JavaScript',
-          code: './with-cache.example.js',
+          code: './create-with-cache.example.js',
           language: 'js',
         },
         {
           title: 'TypeScript',
-          code: './with-cache.example.ts',
+          code: './create-with-cache.example.ts',
           language: 'ts',
         },
       ],

packages/hydrogen/src/with-cache.example.js → packages/hydrogen/src/cache/create-with-cache.example.js

@@ -16,21 +16,35 @@ export default {
     // Create a custom utility to query a third-party API:
     const fetchMyCMS = (query) => {
       // Prefix the cache key and make it unique based on arguments.
-      return withCache(['my-cms', query], CacheLong(), async () => {
-        return await (
-          await fetch('my-cms.com/api', {
-            method: 'POST',
-            body: query,
-          })
-        ).json();
+      return withCache(['my-cms', query], CacheLong(), async (params) => {
+        const response = await fetch('my-cms.com/api', {
+          method: 'POST',
+          body: query,
+        });
+
+        // Throw if the response is unsuccessful
+        if (!response.ok) throw new Error(response.statusText);
+
+        // Assuming the API returns errors in the body:
+        const {data, error} = await response.json();
+
+        // Validate data and throw to avoid caching errors.
+        if (error || !data) throw new Error(error ?? 'Missing data');
+
+        // Optionally, add extra information to show
+        // in the Subrequest Profiler utility.
+        params.addDebugData({displayName: 'My CMS query', response});
+
+        return data;
       });
     };
 
     const handleRequest = createRequestHandler({
       build: remixBuild,
       mode: process.env.NODE_ENV,
       getLoadContext: () => ({
-        /* ... */
+        // If you have one, update your env.d.ts to
+        // include `fetchMyCMS` in `AppLoadContext`.
         fetchMyCMS,
       }),
     });

packages/hydrogen/src/with-cache.example.ts → packages/hydrogen/src/cache/create-with-cache.example.ts

@@ -20,21 +20,37 @@ export default {
     // Create a custom utility to query a third-party API:
     const fetchMyCMS = (query: string) => {
       // Prefix the cache key and make it unique based on arguments.
-      return withCache(['my-cms', query], CacheLong(), async () => {
-        return await (
-          await fetch('my-cms.com/api', {
-            method: 'POST',
-            body: query,
-          })
-        ).json();
+      return withCache(['my-cms', query], CacheLong(), async (params) => {
+        const response = await fetch('my-cms.com/api', {
+          method: 'POST',
+          body: query,
+        });
+
+        // Throw if the response is unsuccessful
+        if (!response.ok) throw new Error(response.statusText);
+
+        const {data, error} = (await response.json()) as {
+          data: unknown;
+          error?: string;
+        };
+
+        // Validate data and throw to avoid caching errors.
+        if (error || !data) throw new Error(error ?? 'Missing data');
+
+        // Optionally, add extra information to show
+        // in the Subrequest Profiler utility.
+        params.addDebugData({displayName: 'My CMS query', response});
+
+        return data;
       });
     };
 
     const handleRequest = createRequestHandler({
       build: remixBuild,
       mode: process.env.NODE_ENV,
       getLoadContext: () => ({
-        // Make sure to update remix.env.d.ts to include `fetchMyCMS`
+        // Make sure to update env.d.ts to
+        // include `fetchMyCMS` in `AppLoadContext`.
         fetchMyCMS,
       }),
     });

packages/hydrogen/src/cache/create-with-cache.test.ts

@@ -0,0 +1,116 @@
+import {describe, beforeEach, it, expect, vi} from 'vitest';
+import {type WithCache, createWithCache} from './create-with-cache';
+import {InMemoryCache} from './in-memory';
+import {getItemFromCache} from './sub-request';
+import {CacheNone, CacheShort} from './strategies';
+
+describe('createWithCache', () => {
+  const KEY = 'my-key';
+  const VALUE = 'my-value';
+  const actionFn = vi.fn(() => VALUE);
+  const waitUntil = vi.fn(() => {});
+  let cache: InMemoryCache;
+  let withCache: WithCache;
+
+  beforeEach(() => {
+    vi.useFakeTimers();
+    cache = new InMemoryCache();
+    withCache = createWithCache({cache, waitUntil});
+    actionFn.mockClear();
+    waitUntil.mockClear();
+    return () => vi.useRealTimers();
+  });
+
+  it('creates a valid withCache function', () => {
+    expect(withCache).toBeInstanceOf(Function);
+  });
+
+  it('skips cache for no-cache policy', async () => {
+    await expect(withCache(KEY, CacheNone(), actionFn)).resolves.toEqual(VALUE);
+
+    expect(waitUntil).toHaveBeenCalledTimes(0);
+    expect(actionFn).toHaveBeenCalledTimes(1);
+    await expect(getItemFromCache(cache, KEY)).resolves.toEqual(undefined);
+
+    await expect(withCache(KEY, CacheNone(), actionFn)).resolves.toEqual(VALUE);
+
+    // No cache, always calls the action function:
+    expect(waitUntil).toHaveBeenCalledTimes(0);
+    expect(actionFn).toHaveBeenCalledTimes(2);
+    await expect(getItemFromCache(cache, KEY)).resolves.toEqual(undefined);
+  });
+
+  it('skips cache when throwing', async () => {
+    actionFn.mockImplementationOnce(() => {
+      return Promise.resolve().then(() => {
+        throw new Error('test');
+      });
+    });
+
+    await expect(withCache(KEY, CacheShort(), actionFn)).rejects.toThrowError(
+      'test',
+    );
+
+    expect(waitUntil).toHaveBeenCalledTimes(0);
+    expect(actionFn).toHaveBeenCalledTimes(1);
+    await expect(getItemFromCache(cache, KEY)).resolves.toEqual(undefined);
+  });
+
+  it('stores results in the cache', async () => {
+    const strategy = CacheShort({maxAge: 1, staleWhileRevalidate: 9});
+    await expect(withCache(KEY, strategy, actionFn)).resolves.toEqual(VALUE);
+
+    expect(waitUntil).toHaveBeenCalledTimes(1);
+    expect(actionFn).toHaveBeenCalledTimes(1);
+    await expect(getItemFromCache(cache, KEY)).resolves.toContainEqual({
+      value: VALUE,
+    });
+
+    // Less than 1 sec of the cache duration:
+    vi.advanceTimersByTime(999);
+
+    await expect(withCache(KEY, strategy, actionFn)).resolves.toEqual(VALUE);
+
+    // Cache hit, nothing to update:
+    expect(waitUntil).toHaveBeenCalledTimes(1);
+    expect(actionFn).toHaveBeenCalledTimes(1);
+    await expect(getItemFromCache(cache, KEY)).resolves.toContainEqual({
+      value: VALUE,
+    });
+  });
+
+  it('applies stale-while-revalidate', async () => {
+    const strategy = CacheShort({maxAge: 1, staleWhileRevalidate: 9});
+    await expect(withCache(KEY, strategy, actionFn)).resolves.toEqual(VALUE);
+
+    expect(waitUntil).toHaveBeenCalledTimes(1);
+    expect(actionFn).toHaveBeenCalledTimes(1);
+    await expect(getItemFromCache(cache, KEY)).resolves.toContainEqual({
+      value: VALUE,
+    });
+
+    // More than 1 sec of the cache duration:
+    vi.advanceTimersByTime(3000);
+
+    await expect(withCache(KEY, strategy, actionFn)).resolves.toEqual(VALUE);
+
+    // Cache stale, call the action function again for SWR:
+    expect(waitUntil).toHaveBeenCalledTimes(2);
+    expect(actionFn).toHaveBeenCalledTimes(2);
+    await expect(getItemFromCache(cache, KEY)).resolves.toContainEqual({
+      value: VALUE,
+    });
+
+    // Make the cache expire. Note: We add a padded maxAge to the cache control
+    // header to support SWR in Oxygen/CFW. Our InMemoryCache doesn't understand
+    // this padded maxAge, so we need to advance timers considering the padded
+    // value: maxAge + (2 * SWR) => 19 sec.
+    vi.advanceTimersByTime(19001);
+    await expect(getItemFromCache(cache, KEY)).resolves.toEqual(undefined);
+
+    // Cache is expired, call the action function again:
+    await expect(withCache(KEY, strategy, actionFn)).resolves.toEqual(VALUE);
+    expect(waitUntil).toHaveBeenCalledTimes(3);
+    expect(actionFn).toHaveBeenCalledTimes(3);
+  });
+});

packages/hydrogen/src/with-cache.ts → packages/hydrogen/src/cache/create-with-cache.ts

@@ -1,11 +1,11 @@
+import {type CachingStrategy} from './strategies';
+import {type CrossRuntimeRequest, getDebugHeaders} from '../utils/request';
+import {getCallerStackLine} from '../utils/callsites';
 import {
-  type CacheKey,
+  CacheActionFunctionParam,
+  CacheKey,
   runWithCache,
-  type CacheActionFunctionParam,
-} from './cache/fetch';
-import type {CachingStrategy} from './cache/strategies';
-import {type CrossRuntimeRequest, getDebugHeaders} from './utils/request';
-import {getCallerStackLine} from './utils/callsites';
+} from './run-with-cache';
 
 type CreateWithCacheOptions = {
   /** An instance that implements the [Cache API](https://developer.mozilla.org/en-US/docs/Web/API/Cache) */
@@ -20,7 +20,6 @@ type CreateWithCacheOptions = {
  * 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>({