diff --git a/packages/react-meteor-data/suspense/useTracker.tests.js b/packages/react-meteor-data/suspense/useTracker.tests.js index 5a37f9ea..13397eb2 100644 --- a/packages/react-meteor-data/suspense/useTracker.tests.js +++ b/packages/react-meteor-data/suspense/useTracker.tests.js @@ -24,11 +24,13 @@ const TestSuspense = ({ children }) => { const trackerVariants = [ { label: 'default', - useTrackerFn: (key, fn, skipUpdate) => useTracker(key, fn, skipUpdate), + useTrackerFn: (key, fn, skipUpdate, _deps) => + useTracker(key, fn, skipUpdate), }, { label: 'with deps', - useTrackerFn: (key, fn, skipUpdate) => useTracker(key, fn, [], skipUpdate), + useTrackerFn: (key, fn, skipUpdate, deps = []) => + useTracker(key, fn, deps, skipUpdate), }, ]; @@ -85,151 +87,163 @@ runForVariants( } ); -Meteor.isServer && runForVariants( - 'suspense/useTracker - Test proper cache invalidation', - async function (test, useTrackerFn) { - const { Coll, simpleFetch } = setupTest(); +Meteor.isClient && + runForVariants( + 'suspense/useTracker - Data query validation with Strict Mode', + async function (test, useTrackerFn) { + const { simpleFetch } = setupTest({ id: 0, name: 'a' }); - let returnValue; + const Test = () => { + const docs = useTrackerFn('TestDocs', simpleFetch); - const Test = () => { - returnValue = useTrackerFn('TestDocs', simpleFetch); - return null; - }; + return
{docs[0]?.name}
; + }; - // first return promise - renderToString( - - - - ); - // wait promise - await new Promise((resolve) => setTimeout(resolve, 100)); - // return data - renderToString( - - - - ); + const { findByText } = render(, { + container: document.createElement('container'), + wrapper: TestSuspense, + reactStrictMode: true, + }); - test.equal( - returnValue[0].updated, - 0, - 'Return value should be an array with initial value as find promise resolved' - ); + test.isTrue(await findByText('a'), 'Need to return data'); - Coll.updateAsync({ id: 0 }, { $inc: { updated: 1 } }); - await new Promise((resolve) => setTimeout(resolve, 100)); + await clearCache(); + } + ); - // second return promise - renderToString( - - - - ); +Meteor.isServer && + runForVariants( + 'suspense/useTracker - Test proper cache invalidation', + async function (test, useTrackerFn) { + const { Coll, simpleFetch } = setupTest(); - test.equal( - returnValue[0].updated, - 0, - 'Return value should still not updated as second find promise unresolved' - ); + let returnValue; - // wait promise - await new Promise((resolve) => setTimeout(resolve, 100)); - // return data - renderToString( - - - - ); - renderToString( - - - - ); - renderToString( - - - - ); + const Test = () => { + returnValue = useTrackerFn('TestDocs', simpleFetch); + return null; + }; - test.equal( - returnValue[0].updated, - 1, - 'Return value should be an array with one document with value updated' - ); + // first return promise + renderToString( + + + + ); + // wait promise + await new Promise((resolve) => setTimeout(resolve, 100)); + // return data + renderToString( + + + + ); - await clearCache(); - } -); + test.equal( + returnValue[0].updated, + 0, + 'Return value should be an array with initial value as find promise resolved' + ); -Meteor.isClient && runForVariants( - 'suspense/useTracker - Test responsive behavior', - async function (test, useTrackerFn) { - const { Coll, simpleFetch } = setupTest(); + Coll.updateAsync({ id: 0 }, { $inc: { updated: 1 } }); + await new Promise((resolve) => setTimeout(resolve, 100)); - let returnValue; + // second return promise + renderToString( + + + + ); - const Test = () => { - returnValue = useTrackerFn('TestDocs', simpleFetch); - return null; - }; + test.equal( + returnValue[0].updated, + 0, + 'Return value should still not updated as second find promise unresolved' + ); - // first return promise - renderToString( - - - - ); - // wait promise - await new Promise((resolve) => setTimeout(resolve, 100)); - // return data - renderToString( - - - - ); + // wait promise + await new Promise((resolve) => setTimeout(resolve, 100)); + // return data + renderToString( + + + + ); + renderToString( + + + + ); + renderToString( + + + + ); - test.equal( - returnValue[0].updated, - 0, - 'Return value should be an array with initial value as find promise resolved' - ); + test.equal( + returnValue[0].updated, + 1, + 'Return value should be an array with one document with value updated' + ); - Coll.updateAsync({ id: 0 }, { $inc: { updated: 1 } }); - - // second await promise - renderToString( - - - - ); + await clearCache(); + } + ); - test.equal( - returnValue[0].updated, - 0, - 'Return value should still not updated as second find promise unresolved' - ); +Meteor.isClient && + runForVariants( + 'suspense/useTracker - Test responsive behavior', + async function (test, useTrackerFn) { + const { Coll, simpleFetch } = setupTest(); - // wait promise - await new Promise((resolve) => setTimeout(resolve, 100)); + const Test = () => { + const docs = useTrackerFn('TestDocs', simpleFetch); + return
{docs[0]?.updated}
; + }; - // return data - renderToString( - - - - ); + const { findByText } = render(, { + container: document.createElement('container'), + wrapper: TestSuspense, + reactStrictMode: true, + }); - test.equal( - returnValue[0].updated, - 1, - 'Return value should be an array with one document with value updated' - ); + test.isTrue(await findByText('0'), 'Need to return data'); - await clearCache(); - } -); + Coll.updateAsync({ id: 0 }, { $inc: { updated: 1 } }); + + test.isTrue(await findByText('1'), 'Need to return data'); + + await clearCache(); + } + ); + +Meteor.isClient && + runForVariants( + 'suspense/useTracker - Test responsive behavior with Strict Mode', + async function (test, useTrackerFn) { + const { Coll, simpleFetch } = setupTest({ id: 0, name: 'a' }); + + const Test = () => { + const docs = useTrackerFn('TestDocs', simpleFetch); + + return
{docs[0]?.name}
; + }; + + const { findByText } = render(, { + container: document.createElement('container'), + wrapper: TestSuspense, + reactStrictMode: true, + }); + + test.isTrue(await findByText('a'), 'Need to return data'); + + Coll.updateAsync({ id: 0 }, { $set: { name: 'b' } }); + + test.isTrue(await findByText('b'), 'Need to return data'); + + await clearCache(); + } + ); Meteor.isClient && runForVariants( @@ -454,3 +468,38 @@ Meteor.isClient && test.isTrue(true, 'should handle unmount correctly in Strict Mode'); } ); + +Meteor.isClient && + runForVariants( + 'suspense/useTracker - test query condition change', + async function (test, useTrackerFn) { + const { Coll } = setupTest(null); + Coll.insertAsync({ id: 0, name: 'a' }); + Coll.insertAsync({ id: 0, name: 'b' }); + + const Test = (props) => { + const docs = useTrackerFn( + 'TestDocs', + () => Coll.find({ name: props.name }).fetchAsync(), + null, + [props.name] + ); + + return
{docs[0]?.name}
; + }; + + const { rerender, findByText } = render(, { + container: document.createElement('container'), + wrapper: TestSuspense, + reactStrictMode: true, + }); + + test.isTrue(await findByText('a'), 'Need to return data'); + + rerender(); + + test.isTrue(await findByText('b'), 'Need to return data'); + + await clearCache(); + } + ); diff --git a/packages/react-meteor-data/suspense/useTracker.ts b/packages/react-meteor-data/suspense/useTracker.ts index 9f6367f9..cc8b0bb1 100644 --- a/packages/react-meteor-data/suspense/useTracker.ts +++ b/packages/react-meteor-data/suspense/useTracker.ts @@ -1,4 +1,4 @@ -import { strictDeepEqual } from 'fast-equals' +import { deepEqual, strictDeepEqual } from 'fast-equals' import { Tracker } from 'meteor/tracker' import { type EJSON } from 'meteor/ejson' import { type DependencyList, useEffect, useMemo, useReducer, useRef } from 'react' @@ -89,6 +89,7 @@ export function useTrackerSuspenseNoDeps(key: string, reactiveFn: IReac isMounted: boolean computation?: Tracker.Computation trackerData: any + cleanupTimoutId?: number }>({ isMounted: false, trackerData: null @@ -104,7 +105,7 @@ export function useTrackerSuspenseNoDeps(key: string, reactiveFn: IReac Tracker.autorun(async (comp: Tracker.Computation) => { if (refs.computation) { refs.computation.stop() - delete refs.computation + refs.computation = undefined } refs.computation = comp @@ -114,6 +115,14 @@ export function useTrackerSuspenseNoDeps(key: string, reactiveFn: IReac if (comp.firstRun) { // Always run the reactiveFn on firstRun refs.trackerData = data + + // The NoDeps version cannot detect variable changes, so we deep compare the value to avoid + // frequently throwing Promises and pausing suspense. Only on actual value change, we throw again. + const cached = cacheMap.get(key) + if (cached && !deepEqual(cached.result, await data)) { + cacheMap.delete(key) + refs.isMounted && forceUpdate() + } } else { const dataResult = await data; @@ -129,14 +138,22 @@ export function useTrackerSuspenseNoDeps(key: string, reactiveFn: IReac // Let subsequent renders know we are mounted (render is committed). refs.isMounted = true - // stop the computation on unmount - return () => { - if (refs.computation) { - refs.computation.stop() - delete refs.computation - } + // In strict mode (development only), `useEffect` may run 1-2 times. + // To avoid this, check the `timeout` to ensure cleanup only occurs after unmount. + if (refs.cleanupTimoutId) { + clearTimeout(refs.cleanupTimoutId) + refs.cleanupTimoutId = undefined + } - refs.isMounted = false + return () => { + refs.cleanupTimoutId = setTimeout(() => { + if (refs.computation) { + refs.computation.stop() + refs.computation = undefined + } + refs.isMounted = false + refs.cleanupTimoutId = undefined + }, 0) } }, []) @@ -152,6 +169,7 @@ export const useTrackerSuspenseWithDeps = isMounted: boolean trackerData?: Promise computation?: Tracker.Computation + cleanupTimoutId?: number }>({ reactiveFn, isMounted: false, @@ -168,7 +186,7 @@ export const useTrackerSuspenseWithDeps = () => Tracker.autorun(async (comp: Tracker.Computation) => { if (refs.computation) { refs.computation.stop() - delete refs.computation + refs.computation = undefined } refs.computation = comp @@ -177,6 +195,12 @@ export const useTrackerSuspenseWithDeps = if (comp.firstRun) { refs.trackerData = data + + // When deps change, re-throw the Promise to get new data. + const cached = cacheMap.get(key) + if (cached && !strictDeepEqual(cached.deps, deps)) { + cacheMap.delete(key) + } } else { const dataResult = await data; @@ -194,13 +218,22 @@ export const useTrackerSuspenseWithDeps = // Let subsequent renders know we are mounted (render is committed). refs.isMounted = true - return () => { - if (refs.computation) { - refs.computation.stop() - delete refs.computation - } + // In strict mode (development only), `useEffect` may run 1-2 times. + // To avoid this, check the `timeout` to ensure cleanup only occurs after unmount. + if (refs.cleanupTimoutId) { + clearTimeout(refs.cleanupTimoutId) + refs.cleanupTimoutId = undefined + } - refs.isMounted = false + return () => { + refs.cleanupTimoutId = setTimeout(() => { + if (refs.computation) { + refs.computation.stop() + refs.computation = undefined + } + refs.isMounted = false + refs.cleanupTimoutId = undefined + }, 0) } }, deps)