Merge pull request #12 from /issues/9

Use `WeakRef` and `FinalizationRegistry` to avoid "leaking" values that are only referenced in the cache.

A few thoughts:
- I've set this on by default, but provided an option to opt-out because some users (Replay) won't want it.
- `WeakRef` only works with objects, so e.g. large string values may still "leak" unless explicitly evicted.
- I don't know if this makes sense to add to the streaming cache; maybe we just require explicit eviction there.
- I'm not sure how this impacts runtime performance of the cache.
- Browser support is [pretty good](https://caniuse.com/?search=weakref).

.github/workflows/jest.yml → .github/workflows/jest.browser.yml

@@ -2,7 +2,7 @@ name: "Jest unit tests"
 on: [pull_request]
 jobs:
   tests_e2e:
-    name: Run unit tests with Jest
+    name: Run Jest (browser based) unit tests
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3

.github/workflows/jest.node.yml

@@ -0,0 +1,15 @@
+name: "Jest unit tests"
+on: [pull_request]
+jobs:
+  tests_e2e:
+    name: Run Jest (node based) unit tests
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - uses: actions/setup-node@v3
+      - name: Initialize Yarn and install package dependencies
+        run: yarn
+      - name: Build NPM package
+        run: yarn prerelease
+      - name: Run tests
+        run: cd packages/suspense && yarn test:node

packages/suspense-website/index.tsx

@@ -6,10 +6,11 @@ import {
   CREATE_DEFERRED,
   CREATE_SINGLE_ENTRY_CACHE,
   CREATE_STREAMING_CACHE,
-  EXAMPLE_ABORT_A_REQUEST,
-  EXAMPLE_FETCH_WITH_STATUS,
-  EXAMPLE_MUTATING_A_CACHE_VALUE,
-  EXAMPLE_STREAMING_CACHE,
+  GUIDE_ABORT_A_REQUEST,
+  GUIDE_FETCH_WITH_STATUS,
+  GUIDE_MEMORY_MANAGEMENT,
+  GUIDE_MUTATING_A_CACHE_VALUE,
+  GUIDE_STREAMING_CACHE,
   IS_THENNABLE,
   USE_CACHE_STATUS,
   USE_STREAMING_CACHE,
@@ -25,6 +26,7 @@ import PageNotFoundRoute from "./src/routes/PageNotFound";
 import UseCacheStatusRoute from "./src/routes/api/useCacheStatus";
 import UseStreamingValuesRoute from "./src/routes/api/useStreamingValues";
 import AbortingRequestRoute from "./src/routes/examples/aborting-a-request";
+import MemoryManagementRoute from "./src/routes/examples/memory-management";
 import MutatingCacheValueRoute from "./src/routes/examples/mutating-a-cache-value";
 import RenderingStatusWhileFetchingRoute from "./src/routes/examples/rendering-status-while-fetching";
 import CreatingStreamingCacheRoute from "./src/routes/examples/streaming-cache";
@@ -51,19 +53,23 @@ root.render(
           element={<CreateStreamingCacheRoute />}
         />
         <Route
-          path={EXAMPLE_ABORT_A_REQUEST}
+          path={GUIDE_ABORT_A_REQUEST}
           element={<AbortingRequestRoute />}
         />
         <Route
-          path={EXAMPLE_FETCH_WITH_STATUS}
+          path={GUIDE_FETCH_WITH_STATUS}
           element={<RenderingStatusWhileFetchingRoute />}
         />
         <Route
-          path={EXAMPLE_MUTATING_A_CACHE_VALUE}
+          path={GUIDE_MEMORY_MANAGEMENT}
+          element={<MemoryManagementRoute />}
+        />
+        <Route
+          path={GUIDE_MUTATING_A_CACHE_VALUE}
           element={<MutatingCacheValueRoute />}
         />
         <Route
-          path={EXAMPLE_STREAMING_CACHE}
+          path={GUIDE_STREAMING_CACHE}
           element={<CreatingStreamingCacheRoute />}
         />
         <Route path={IS_THENNABLE} element={<IsThenableRoute />} />

packages/suspense-website/src/components/Note.module.css

@@ -55,6 +55,17 @@
   flex: 0 0 1.75rem !important;
 }
 
+.Content {
+  display: flex;
+  flex-direction: column;
+  gap: 0.5rem;
+}
+
+.Content * {
+  line-height: 1;
+  margin: 0;
+}
+
 .Note[data-type="note"] .Content,
 .Note[data-type="quote"] .Content {
   line-height: 1rem;

packages/suspense-website/src/examples/createCache/cacheWithoutWeakRef.ts

@@ -0,0 +1,11 @@
+import { CacheLoadOptions, createCache } from "suspense";
+
+const load = async () => null as any;
+
+// REMOVE_BEFORE
+
+createCache<[userId: string], JSON>({
+  config: { useWeakRef: false },
+  load,
+  // ...
+});

packages/suspense-website/src/examples/index.ts

@@ -31,6 +31,12 @@ const createCache = {
   cacheWithSignal: processExample(
     readFileSync(join(__dirname, "createCache", "cacheWithSignal.ts"), "utf8")
   ),
+  cacheWithoutWeakRef: processExample(
+    readFileSync(
+      join(__dirname, "createCache", "cacheWithoutWeakRef.ts"),
+      "utf8"
+    )
+  ),
   evict: processExample(
     readFileSync(join(__dirname, "createCache", "evict.ts"), "utf8")
   ),

packages/suspense-website/src/routes/Home.tsx

@@ -10,9 +10,10 @@ import {
   CREATE_DEFERRED,
   CREATE_SINGLE_ENTRY_CACHE,
   CREATE_STREAMING_CACHE,
-  EXAMPLE_ABORT_A_REQUEST,
-  EXAMPLE_FETCH_WITH_STATUS,
-  EXAMPLE_STREAMING_CACHE,
+  GUIDE_ABORT_A_REQUEST,
+  GUIDE_FETCH_WITH_STATUS,
+  GUIDE_MEMORY_MANAGEMENT,
+  GUIDE_STREAMING_CACHE,
   IS_THENNABLE,
   USE_CACHE_STATUS,
   USE_STREAMING_CACHE,
@@ -93,19 +94,24 @@ export default function Route() {
       <Block>
         <SubHeading title="Guides" />
         <ul>
+          <LinkListItem
+            children="Memory management"
+            to={GUIDE_MEMORY_MANAGEMENT}
+            type="plaintext"
+          />
           <LinkListItem
             children="Aborting a request"
-            to={EXAMPLE_ABORT_A_REQUEST}
+            to={GUIDE_ABORT_A_REQUEST}
             type="plaintext"
           />
           <LinkListItem
             children="Creating a streaming cache"
-            to={EXAMPLE_STREAMING_CACHE}
+            to={GUIDE_STREAMING_CACHE}
             type="plaintext"
           />
           <LinkListItem
             children="Rendering cache status"
-            to={EXAMPLE_FETCH_WITH_STATUS}
+            to={GUIDE_FETCH_WITH_STATUS}
             type="plaintext"
           />
         </ul>

packages/suspense-website/src/routes/api/createCache.tsx

@@ -5,7 +5,11 @@ import { createCache } from "../../examples";
 import Header from "../../components/Header";
 import SubHeading from "../../components/SubHeading";
 import { Link } from "react-router-dom";
-import { IS_THENNABLE, USE_CACHE_STATUS } from "../config";
+import {
+  GUIDE_MEMORY_MANAGEMENT,
+  IS_THENNABLE,
+  USE_CACHE_STATUS,
+} from "../config";
 import Note from "../../components/Note";
 import { ExternalLink } from "../../components/ExternalLink";
 
@@ -48,6 +52,11 @@ export default function Route() {
           that's provided to support cancellation.
         </p>
         <Code code={createCache.cacheWithSignal} />
+        <Note>
+          Caches use <code>WeakRef</code> and <code>FinalizationRegistry</code>{" "}
+          by default,{" "}
+          <Link to={GUIDE_MEMORY_MANAGEMENT}>but this is configurable</Link>.
+        </Note>
       </Block>
       <Block>
         <SubHeading title="Using a cache with suspense" />

packages/suspense-website/src/routes/config.ts

@@ -1,3 +1,4 @@
+// API
 export const CREATE_CACHE = "/createCache";
 export const CREATE_DEFERRED = "/createDeferred";
 export const CREATE_SINGLE_ENTRY_CACHE = "/createSingleEntryCache";
@@ -6,8 +7,9 @@ export const IS_THENNABLE = "/isThenable";
 export const USE_CACHE_STATUS = "/useCacheStatus";
 export const USE_STREAMING_CACHE = "/useStreamingValues";
 
-// Examples
-export const EXAMPLE_ABORT_A_REQUEST = "/example/abort-a-request";
-export const EXAMPLE_FETCH_WITH_STATUS = "/example/rendering-cache-status";
-export const EXAMPLE_MUTATING_A_CACHE_VALUE = "/example/mutating-a-cache-value";
-export const EXAMPLE_STREAMING_CACHE = "/example/writing-a-streaming-cache";
+// Guides
+export const GUIDE_ABORT_A_REQUEST = "/guide/abort-a-request";
+export const GUIDE_FETCH_WITH_STATUS = "/guide/rendering-cache-status";
+export const GUIDE_MEMORY_MANAGEMENT = "/guide/memory-management";
+export const GUIDE_MUTATING_A_CACHE_VALUE = "/guide/mutating-a-cache-value";
+export const GUIDE_STREAMING_CACHE = "/guide/writing-a-streaming-cache";

packages/suspense-website/src/routes/examples/memory-management.tsx

@@ -0,0 +1,49 @@
+import { Link } from "react-router-dom";
+import Block from "../../components/Block";
+import Code from "../../components/Code";
+import Container from "../../components/Container";
+import { ExternalLink } from "../../components/ExternalLink";
+import Header from "../../components/Header";
+import Note from "../../components/Note";
+import { createCache } from "../../examples";
+import { CREATE_CACHE } from "../config";
+
+export default function Route() {
+  return (
+    <Container>
+      <Block>
+        <Header title="memory management" />
+      </Block>
+      <Block>
+        <p>
+          Caches created with{" "}
+          <code>
+            <Link to={CREATE_CACHE}>createCache</Link>
+          </code>{" "}
+          use{" "}
+          <code>
+            <ExternalLink to="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakRef">
+              WeakRef
+            </ExternalLink>
+          </code>{" "}
+          and{" "}
+          <code>
+            <ExternalLink to="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/FinalizationRegistry">
+              FinalizationRegistry
+            </ExternalLink>
+          </code>{" "}
+          APIs to avoid leaking memory. This behavior can be disabled using the{" "}
+          <code>useWeakRef</code> configuration flag.
+        </p>
+        <Code code={createCache.cacheWithoutWeakRef} />
+        <Note type="warn">
+          <p>Caches that don't use weak refs may leak memory over time.</p>
+          <p>
+            To avoid this, use the <code>evict</code> method to remove entries
+            once you are done using them.
+          </p>
+        </Note>
+      </Block>
+    </Container>
+  );
+}

packages/suspense-website/src/routes/examples/streaming-cache.tsx

@@ -22,7 +22,6 @@ export default function Route() {
           to render a larger data set as it incrementally loads.
         </p>
         <p>Click the "start demo" button to fetch user data in the cache.</p>
-        {/*<Code code={demos.} />*/}
       </Block>
       <Block type="demo">
         <Demo />

packages/suspense/jest.config.node.js

@@ -0,0 +1,5 @@
+/** @type {import('ts-jest').JestConfigWithTsJest} */
+module.exports = {
+  preset: "ts-jest",
+  testMatch: ["**/*.test.node.{ts,tsx}"],
+};

packages/suspense/package.json

@@ -15,6 +15,8 @@
   "scripts": {
     "build": "parcel build",
     "test": "jest",
+    "test:node": "node --expose-gc node_modules/.bin/jest --config=jest.config.node.js",
+    "test:node:watch": "node --expose-gc node_modules/.bin/jest --config=jest.config.node.js --watch",
     "test:watch": "jest --watch",
     "watch": "parcel watch"
   },

packages/suspense/src/cache/createCache.test.node.ts

@@ -0,0 +1,66 @@
+import { createCache } from "./createCache";
+import { Cache, CacheLoadOptions } from "../types";
+import { requestGC, waitForGC } from "../utils/test";
+
+describe("createCache", () => {
+  let cache: Cache<[string], Object>;
+  let fetch: jest.Mock<Promise<Object> | Object, [string, CacheLoadOptions]>;
+
+  beforeEach(() => {
+    fetch = jest.fn();
+    fetch.mockImplementation((key: string) => {
+      if (key.startsWith("async")) {
+        return Promise.resolve(key);
+      } else if (key.startsWith("error")) {
+        return Promise.reject(key);
+      } else {
+        return key;
+      }
+    });
+  });
+
+  it("should use WeakRefs if requested", async () => {
+    cache = createCache<[string], Object>({
+      config: { useWeakRef: true },
+      load: fetch,
+    });
+
+    cache.cache(createObject(), "one");
+    cache.cache(createObject(), "two");
+    cache.cache(createObject(), "three");
+
+    expect(cache.getValueIfCached("one")).not.toBeUndefined();
+    expect(cache.getValueIfCached("two")).not.toBeUndefined();
+    expect(cache.getValueIfCached("three")).not.toBeUndefined();
+
+    await requestGC();
+    await waitForGC();
+
+    expect(cache.getValueIfCached("one")).toBeUndefined();
+    expect(cache.getValueIfCached("two")).toBeUndefined();
+    expect(cache.getValueIfCached("three")).toBeUndefined();
+  });
+
+  it("should not use WeakRefs if requested", async () => {
+    cache = createCache<[string], Object>({
+      config: { useWeakRef: false },
+      load: fetch,
+    });
+
+    cache.cache(createObject(), "one");
+    cache.cache(createObject(), "two");
+
+    expect(cache.getValueIfCached("one")).not.toBeUndefined();
+    expect(cache.getValueIfCached("two")).not.toBeUndefined();
+
+    await requestGC();
+    await waitForGC();
+
+    expect(cache.getValueIfCached("one")).not.toBeUndefined();
+    expect(cache.getValueIfCached("two")).not.toBeUndefined();
+  });
+});
+
+function createObject(): Object {
+  return {};
+}