From 6951415ba855a6663fed844b5e3734b3ce2c63bb Mon Sep 17 00:00:00 2001 From: Vu Phan <75567967+vuphanse@users.noreply.github.com> Date: Mon, 2 Mar 2026 22:35:15 +0700 Subject: [PATCH] Reuse doc refs when useFind's cursor deps change and add coverage for race/ordering cases --- packages/react-meteor-data/useFind.tests.js | 298 +++++++++++++++++++- packages/react-meteor-data/useFind.ts | 89 ++++-- 2 files changed, 363 insertions(+), 24 deletions(-) diff --git a/packages/react-meteor-data/useFind.tests.js b/packages/react-meteor-data/useFind.tests.js index 37f27e89..8c6fa662 100644 --- a/packages/react-meteor-data/useFind.tests.js +++ b/packages/react-meteor-data/useFind.tests.js @@ -1,5 +1,5 @@ /* global Meteor, Tinytest */ -import React, { memo, useState } from 'react' +import React, { memo, useEffect, useLayoutEffect, useState } from 'react' import ReactDOM from 'react-dom' import { waitFor } from '@testing-library/react' import { Mongo } from 'meteor/mongo' @@ -150,6 +150,302 @@ if (Meteor.isClient) { completed(); } ); + + Tinytest.addAsync( + 'useFind - reuses document references when deps recreate the cursor', + async function (test, completed) { + const container = document.createElement('div') + + const TestDocs = new Mongo.Collection(null) + for (let i = 0; i < 5; i++) { + TestDocs.insert({ id: i }) + } + + let renders = 0 + const MemoizedItem = memo(({ doc }) => { + renders++ + return
  • {doc.id}
    • + }) + + const Test = () => { + const [rerendered, setRerendered] = useState(false) + const docs = useFind(() => TestDocs.find({}, { sort: { id: 1 } }), [rerendered]) + + useEffect(() => { + if (!rerendered) { + setRerendered(true) + } + }, [rerendered]) + + return ( +
    +
    {String(rerendered)}
    + +
    + ) + } + + ReactDOM.render(, container) + test.equal(renders, 5, 'Initial renders should occur once per document') + + await waitFor(() => { + const flag = container.querySelector('[data-testid="rerender-flag"]') + if (!flag || flag.textContent !== 'true') { + throw new Error('Component has not rerendered yet') + } + }, { container, timeout: 250 }) + + test.equal( + renders, + 5, + 'Document references should remain stable when deps recreate the cursor' + ) + + completed() + } + ) + + Tinytest.addAsync( + 'useFind - recreating cursor with modified documents re-renders memoized items', + async function (test, completed) { + const container = document.createElement('div') + + const PrimaryDocs = new Mongo.Collection(null) + const SecondaryDocs = new Mongo.Collection(null) + + for (let i = 0; i < 4; i++) { + PrimaryDocs.insert({ _id: `doc-${i}`, id: i, label: `primary-${i}` }) + SecondaryDocs.insert({ _id: `doc-${i}`, id: i, label: `secondary-${i}` }) + } + + let renders = 0 + const MemoizedItem = memo(({ doc }) => { + renders++ + return
  • {doc.label}
    • + }) + + const Test = () => { + const [useAlternate, setUseAlternate] = useState(false) + const collection = useAlternate ? SecondaryDocs : PrimaryDocs + const docs = useFind(() => collection.find({}, { sort: { id: 1 } }), [useAlternate]) + + useEffect(() => { + if (!useAlternate) { + setUseAlternate(true) + } + }, [useAlternate]) + + return ( + + ) + } + + ReactDOM.render(, container) + test.equal(renders, 4, 'Initial renders should occur once per document') + + await waitFor(() => { + if (!container.textContent?.includes('secondary-0')) { + throw new Error('Alternate collection has not rendered yet') + } + }, { container, timeout: 500 }) + + test.equal( + renders, + 8, + 'Documents should rerender when deps recreate the cursor with modified data' + ) + + completed() + } + ) + + Tinytest.addAsync( + 'useFind - recreating cursor with different sort order updates ordering', + async function (test, completed) { + const container = document.createElement('div') + + const TestDocs = new Mongo.Collection(null) + for (let i = 0; i < 4; i++) { + TestDocs.insert({ id: i }) + } + + const Test = () => { + const [ascending, setAscending] = useState(true) + const docs = useFind( + () => TestDocs.find({}, { sort: { id: ascending ? 1 : -1 } }), + [ascending] + ) + + useEffect(() => { + if (ascending) { + setAscending(false) + } + }, [ascending]) + + return ( +
    + {docs.map(doc => ( +
    {doc.id}
    + ))} +
    + ) + } + + ReactDOM.render(, container) + + const getIds = () => Array.from( + container.querySelectorAll('[data-testid="doc-id"]') + ).map(node => node.textContent) + + test.equal(getIds(), ['0', '1', '2', '3'], 'Initial render should be ascending') + + await waitFor(() => { + const ids = getIds() + if (ids[0] !== '3') { + throw new Error('Documents have not been reordered yet') + } + }, { container, timeout: 500 }) + + test.equal( + getIds(), + ['3', '2', '1', '0'], + 'Documents should be rendered in descending order after deps change' + ) + + completed() + } + ) + + Tinytest.addAsync( + 'useFind - recreating cursor with different projection re-renders memoized items', + async function (test, completed) { + const container = document.createElement('div') + + const TestDocs = new Mongo.Collection(null) + for (let i = 0; i < 3; i++) { + TestDocs.insert({ _id: `doc-${i}`, id: i, label: `label-${i}`, detail: `detail-${i}` }) + } + + let renders = 0 + const MemoizedItem = memo(({ doc }) => { + renders++ + return
  • {doc.label}::{doc.detail || 'none'}
    • + }) + + const Test = () => { + const [includeDetail, setIncludeDetail] = useState(false) + const docs = useFind( + () => TestDocs.find( + {}, + { + sort: { id: 1 }, + transform: doc => { + if (!includeDetail) { + return { _id: doc._id, label: doc.label } + } + return { _id: doc._id, label: doc.label, detail: doc.detail } + } + } + ), + [includeDetail] + ) + + useEffect(() => { + if (!includeDetail) { + setIncludeDetail(true) + } + }, [includeDetail]) + + return ( +
      + {docs.map(doc => ( + + ))} +
    + ) + } + + ReactDOM.render(, container) + test.equal(renders, 3, 'Initial renders should occur once per document') + + await waitFor(() => { + if (!container.textContent?.includes('detail-0')) { + throw new Error('Projected detail fields have not rendered yet') + } + }, { container, timeout: 500 }) + + test.equal( + renders, + 6, + 'Documents should rerender when cursor projection adds new fields' + ) + + completed() + } + ) + + Tinytest.addAsync( + 'useFind - handles cursor recreation without duplicating documents', + async function (test, completed) { + const container = document.createElement('div') + document.body.appendChild(container) + + const TestDocs = new Mongo.Collection(null) + TestDocs.insert({ id: 'a', label: 'a' }) + + const Test = () => { + const [rerendered, setRerendered] = useState(false) + const docs = useFind(() => TestDocs.find({}, { sort: { id: 1 } }), [rerendered]) + + useEffect(() => { + setRerendered(true) + }, []) + + useLayoutEffect(() => { + if (!rerendered || TestDocs.findOne({ id: 'b' })) { + return + } + + TestDocs.insert({ id: 'b', label: 'b' }) + }, [rerendered]) + + return ( +
    + {docs && docs.map(doc => ( +
    {doc.id}
    + ))} +
    + ) + } + + ReactDOM.render(, container) + + const getIds = () => Array.from(container.querySelectorAll('[data-testid=\"doc-id\"]')).map(node => node.textContent) + + await waitFor(() => { + const ids = getIds() + if (ids.length !== 2) { + throw new Error('Expected two documents') + } + if (ids[0] !== 'a' || ids[1] !== 'b') { + throw new Error('Unexpected document order') + } + }, { container, timeout: 500 }) + + test.equal(getIds(), ['a', 'b'], 'Documents should not duplicate when deps change') + + document.body.removeChild(container) + completed() + } + ) } else { } diff --git a/packages/react-meteor-data/useFind.ts b/packages/react-meteor-data/useFind.ts index 59244e3d..ee7d57db 100644 --- a/packages/react-meteor-data/useFind.ts +++ b/packages/react-meteor-data/useFind.ts @@ -1,6 +1,7 @@ import { Meteor } from 'meteor/meteor' import { Mongo } from 'meteor/mongo' -import { useReducer, useMemo, useEffect, Reducer, DependencyList, useRef } from 'react' +import { EJSON } from 'meteor/ejson' +import { useReducer, useMemo, useLayoutEffect, Reducer, DependencyList, useRef } from 'react' import { Tracker } from 'meteor/tracker' type useFindActions = @@ -71,23 +72,58 @@ const fetchData = (cursor: Mongo.Cursor) => { return data } -const useSyncEffect = (effect, deps) => { - const [cleanup, timeoutId] = useMemo( - () => { - const cleanup = effect(); - const timeoutId = setTimeout(cleanup, 1000); - return [cleanup, timeoutId]; - }, - deps - ); +const getDocId = (doc: any) => { + if (doc && typeof doc === 'object' && '_id' in doc) { + return (doc as any)._id + } + return undefined +} + +const mergeInitialData = (currentData: T[], nextData: T[]): T[] | null => { + if (!currentData.length && !nextData.length) { + return null + } + + // Track docs by _id so we can reuse references for untouched documents. + const currentDocsById = new Map() + currentData.forEach(doc => { + const id = getDocId(doc) + if (id !== undefined) { + currentDocsById.set(id, doc) + } + }) + + let changed = currentData.length !== nextData.length + const mergedData = nextData.map((doc, index) => { + const id = getDocId(doc) + if (id === undefined) { + if (!changed && doc !== currentData[index]) { + changed = true + } + return doc + } - useEffect(() => { - clearTimeout(timeoutId); + const previousDoc = currentDocsById.get(id) + if (!previousDoc) { + changed = true + return doc + } + + // Detect order changes. + if (!changed && previousDoc !== currentData[index]) { + changed = true + } - return cleanup; - }, [cleanup]); -}; + if (doc === previousDoc || EJSON.equals(doc as any, previousDoc as any)) { + return previousDoc + } + + changed = true + return doc + }) + return changed ? mergedData : null +} const useFindClient = (factory: () => (Mongo.Cursor | undefined | null), deps: DependencyList = []) => { const cursor = useMemo(() => { @@ -112,13 +148,21 @@ const useFindClient = (factory: () => (Mongo.Cursor | undefined | nu } ) - useSyncEffect(() => { + const cleanupRef = useRef<(() => void) | null>(null); + + useLayoutEffect(() => { + cleanupRef.current?.(); // stop previous observer now + if (!(cursor instanceof Mongo.Cursor)) { - return + cleanupRef.current = null; + return; } - + const initialData = fetchData(cursor); - dispatch({ type: 'refresh', data: initialData }); + const mergedData = mergeInitialData(data, initialData) + if (mergedData) { + dispatch({ type: 'refresh', data: mergedData }); + } const observer = cursor.observe({ addedAt(document, atIndex, before) { @@ -135,11 +179,10 @@ const useFindClient = (factory: () => (Mongo.Cursor | undefined | nu }, // @ts-ignore _suppress_initial: true - }) + }); - return () => { - observer.stop() - } + cleanupRef.current = () => observer.stop(); + return cleanupRef.current; }, [cursor]); return cursor ? data : cursor