diff --git a/.changeset/angry-llamas-walk.md b/.changeset/angry-llamas-walk.md new file mode 100644 index 00000000000..96c4bf76594 --- /dev/null +++ b/.changeset/angry-llamas-walk.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Fixes race conditions in useReactiveVar that may prevent updates to the reactive variable from propagating through the hook. diff --git a/src/react/hooks/__tests__/useReactiveVar.test.tsx b/src/react/hooks/__tests__/useReactiveVar.test.tsx index 08cbf3ad41e..3c5e8afdbf3 100644 --- a/src/react/hooks/__tests__/useReactiveVar.test.tsx +++ b/src/react/hooks/__tests__/useReactiveVar.test.tsx @@ -323,5 +323,59 @@ describe("useReactiveVar Hook", () => { resolve(); } ); + + itAsync( + "should survive many rerenderings despite racing asynchronous updates", + (resolve, reject) => { + const rv = makeVar(0); + + function App() { + const value = useReactiveVar(rv); + return ( +
+

{value}

+
+ ); + } + + const goalCount = 1000; + let updateCount = 0; + let stopped = false; + + function spam() { + if (stopped) return; + try { + if (++updateCount <= goalCount) { + act(() => { + rv(updateCount); + setTimeout(spam, Math.random() * 10); + }); + } else { + stopped = true; + expect(rv()).toBe(goalCount); + screen + .findByText(String(goalCount)) + .then((element) => { + expect(element.nodeName.toLowerCase()).toBe("h1"); + }) + .then(resolve, reject); + } + } catch (e) { + stopped = true; + reject(e); + } + } + spam(); + spam(); + spam(); + spam(); + + render( + + + + ); + } + ); }); }); diff --git a/src/react/hooks/useReactiveVar.ts b/src/react/hooks/useReactiveVar.ts index a7ea777d9fe..2d14c12cd63 100644 --- a/src/react/hooks/useReactiveVar.ts +++ b/src/react/hooks/useReactiveVar.ts @@ -1,26 +1,24 @@ import * as React from "react"; import type { ReactiveVar } from "../../core/index.js"; +import { useSyncExternalStore } from "./useSyncExternalStore.js"; export function useReactiveVar(rv: ReactiveVar): T { - const value = rv(); - - // We don't actually care what useState thinks the value of the variable - // is, so we take only the update function from the returned array. - const setValue = React.useState(value)[1]; - - // We subscribe to variable updates on initial mount and when the value has - // changed. This avoids a subtle bug in React.StrictMode where multiple - // listeners are added, leading to inconsistent updates. - React.useEffect(() => { - const probablySameValue = rv(); - if (value !== probablySameValue) { - // If the value of rv has already changed, we don't need to listen for the - // next change, because we can report this change immediately. - setValue(probablySameValue); - } else { - return rv.onNextChange(setValue); - } - }, [value]); - - return value; + return useSyncExternalStore( + React.useCallback( + (update) => { + // By reusing the same onNext function in the nested call to + // rv.onNextChange(onNext), we can keep using the initial clean-up function + // returned by rv.onNextChange(function onNext(v){...}), without having to + // register the new clean-up function (returned by the nested + // rv.onNextChange(onNext)) with yet another callback. + return rv.onNextChange(function onNext() { + update(); + rv.onNextChange(onNext); + }); + }, + [rv] + ), + rv, + rv + ); }