Fix error occurring in useEffect on hmr

libs/isograph-react-disposable-state/package.json

@@ -25,6 +25,7 @@
   },
   "devDependencies": {
     "@types/react": "18.3.1",
+    "@types/react-test-renderer": "^18.3.0",
     "react-test-renderer": "^18.2.0",
     "typescript": "5.6.3"
   },

libs/isograph-react-disposable-state/src/useCachedResponsivePrecommitValue.test.tsx

@@ -2,7 +2,7 @@ import { describe, test, vi, expect, assert } from 'vitest';
 import { ParentCache } from './ParentCache';
 import { ItemCleanupPair } from '@isograph/disposable-types';
 import { useCachedResponsivePrecommitValue } from './useCachedResponsivePrecommitValue';
-import React from 'react';
+import React, { StrictMode, type ReactElement } from 'react';
 import { create } from 'react-test-renderer';
 import { CacheItem, CacheItemState } from './CacheItem';
 
@@ -51,9 +51,10 @@ function promiseAndResolver() {
 
 // The fact that sometimes we need to render in concurrent mode and sometimes
 // not is a bit worrisome.
-async function awaitableCreate(Component, isConcurrent) {
+async function awaitableCreate(Component: ReactElement, isConcurrent) {
   const element = create(
-    Component,
+    <StrictMode>{Component}</StrictMode>,
+    // @ts-expect-error
     isConcurrent ? { unstable_isConcurrent: true } : undefined,
   );
   await shortPromise();

libs/isograph-react-disposable-state/src/useCachedResponsivePrecommitValue.ts

@@ -51,6 +51,14 @@ export function useCachedResponsivePrecommitValue<T>(
   const lastCommittedParentCache = useRef<ParentCache<T> | null>(null);
 
   useEffect(() => {
+    // react reruns all `useEffect` in HMR since it doesn't know if the
+    // code inside of useEffect has changed. Since this is a library
+    // user can't change this code so we are safe to skip this rerun.
+    // This also prevents `useEffect` from running twice in Strict Mode.
+    if (lastCommittedParentCache.current === parentCache) {
+      return;
+    }
+
     lastCommittedParentCache.current = parentCache;
     // On commit, cacheItem may be disposed, because during the render phase,
     // we only temporarily retained the item, and the temporary retain could have

libs/isograph-react-disposable-state/src/useDisposableState.ts

@@ -22,7 +22,6 @@ export function useDisposableState<T = never>(
   const preCommitItem = useCachedResponsivePrecommitValue(
     parentCache,
     (pair) => {
-      itemCleanupPairRef.current?.[1]();
       itemCleanupPairRef.current = pair;
     },
   );
@@ -47,14 +46,23 @@ export function useDisposableState<T = never>(
     [stateFromDisposableStateHook],
   );
 
-  useEffect(function cleanupItemCleanupPairRefIfSetStateNotCalled() {
-    return () => {
-      if (itemCleanupPairRef.current !== null) {
-        itemCleanupPairRef.current[1]();
-        itemCleanupPairRef.current = null;
+  const lastCommittedParentCache = useRef<ParentCache<T> | null>(null);
+
+  useEffect(
+    function cleanupItemCleanupPairRefIfSetStateNotCalled() {
+      if (lastCommittedParentCache.current === parentCache) {
+        return;
       }
-    };
-  }, []);
+      lastCommittedParentCache.current = parentCache;
+
+      return () => {
+        if (itemCleanupPairRef.current !== null) {
+          itemCleanupPairRef.current[1]();
+        }
+      };
+    },
+    [parentCache],
+  );
 
   // Safety: we can be in one of three states. Pre-commit, in which case
   // preCommitItem is assigned, post-commit but before setState has been

libs/isograph-react-disposable-state/src/useLazyDisposableState.test.tsx

@@ -1,5 +1,5 @@
 import { ItemCleanupPair } from '@isograph/disposable-types';
-import React, { useEffect, useState } from 'react';
+import React, { StrictMode, useEffect, useState } from 'react';
 import { create } from 'react-test-renderer';
 import { describe, expect, test, vi } from 'vitest';
 import { ParentCache } from './ParentCache';
@@ -55,7 +55,12 @@ describe('useLazyDisposableState', async () => {
       return null;
     }
 
-    const root = create(<TestComponent />, { unstable_isConcurrent: true });
+    const root = create(
+      <StrictMode>
+        <TestComponent />
+      </StrictMode>,
+      { unstable_isConcurrent: true },
+    );
     await committed.promise;
     expect(cache1.disposeItem).toHaveBeenCalled();
     expect(cache1.cache.factory).toHaveBeenCalledOnce();

libs/isograph-react-disposable-state/src/useLazyDisposableState.ts

@@ -23,16 +23,24 @@ export function useLazyDisposableState<T>(
   state: T;
 } {
   const itemCleanupPairRef = useRef<ItemCleanupPair<T> | null>(null);
-
   const preCommitItem = useCachedResponsivePrecommitValue(
     parentCache,
     (pair) => {
-      itemCleanupPairRef.current?.[1]();
       itemCleanupPairRef.current = pair;
     },
   );
 
+  const lastCommittedParentCache = useRef<ParentCache<T> | null>(null);
   useEffect(() => {
+    // react reruns all `useEffect` in HMR since it doesn't know if the
+    // code inside of useEffect has changed. Since this is a library
+    // user can't change this code so we are safe to skip this rerun.
+    // This also prevents `useEffect` from running twice in Strict Mode.
+    if (lastCommittedParentCache.current === parentCache) {
+      return;
+    }
+    lastCommittedParentCache.current = parentCache;
+
     return () => {
       const cleanupFn = itemCleanupPairRef.current?.[1];
       // TODO confirm useEffect is called in order.
@@ -43,7 +51,7 @@ export function useLazyDisposableState<T>(
       }
       return cleanupFn();
     };
-  }, []);
+  }, [parentCache]);
 
   const returnedItem = preCommitItem?.state ?? itemCleanupPairRef.current?.[0];
 

libs/isograph-react-disposable-state/src/useUpdatableDisposableState.test.tsx

@@ -1,5 +1,5 @@
 import { describe, test, vi, expect } from 'vitest';
-import React from 'react';
+import React, { StrictMode } from 'react';
 import { create } from 'react-test-renderer';
 import {
   useUpdatableDisposableState,
@@ -45,7 +45,8 @@ function promiseAndResolver() {
 // not is a bit worrisome.
 async function awaitableCreate(Component, isConcurrent) {
   const element = create(
-    Component,
+    <StrictMode>{Component}</StrictMode>,
+
     isConcurrent ? { unstable_isConcurrent: true } : undefined,
   );
   await shortPromise();