Concurrency issue with useFind

[Poyoman39]

I have found a problem with the current implementation of useFindClient.

A fetchData is run synchronously at first render, then useEffect run cursor.observe({ which watch for changes on cursor.

On my component there is a concurrency issue with changes happening between the first render and the useEffect (so between the first fetchData and the observer creation)

[PedroMarianoAlmeida]

I was able to create a test that catch this error:

// File -> packages/react-meteor-data/useFind.tests.js
Tinytest.addAsync(
    'useFind - Immediate update before effect registration (race condition test)',
    async function (test, completed) {
      const container = document.createElement('div');
      document.body.appendChild(container);

      const TestDocs = new Mongo.Collection(null);
      // Insert a single document.
      TestDocs.insert({ id: 1, val: 'initial' });

      const Test = () => {
        const docs = useFind(() => TestDocs.find(), []);
        return (
          <div data-testid="doc-value">
            {docs && docs[0] && docs[0].val}
          </div>
        );
      };

      // Render the component.
      ReactDOM.render(<Test />, container);

      // Immediately update the document (this should occur
      // after the synchronous fetch in the old code but before the effect attaches).
      TestDocs.update({ id: 1 }, { $set: { val: 'updated' } });

      // Wait until the rendered output reflects the update.
      await waitFor(() => {
        const node = container.querySelector('[data-testid="doc-value"]');
        if (!node || !node.textContent.includes('updated')) {
          throw new Error('Updated value not rendered yet');
        }
      }, { container, timeout: 500 });

      test.ok(
        container.innerHTML.includes('updated'),
        'Document should display updated value; the old code would fail to capture this update.'
      );

      document.body.removeChild(container);
      completed();
    }
  );

@Poyoman39 and I tried to create solutions but doesn't look like there are good ones
But at least now it is easy to reproduce

[StorytellerCZ]

Probably best to add a test like this to #419.

[Poyoman39]

@Grubba27 this issue should remain opened ? To me #428 only implement tests for this issue

[PedroMarianoAlmeida]

I believe this Issue should stay open, and then @Poyoman39 PR #419 should be rebased with master and add my test

(I propose a fix, but it was not good, so I kept only the test)

[Poyoman39]

I finaly succeed to run tests 🙌 react-meteor-data-harness. This PR pass all the tests :)

[StorytellerCZ]

That is #436 correct?

[Poyoman39]

@StorytellerCZ no this would be for a useTracker improvement ... still working on it ;)