Fix for adding event listeners while in a write transaction (#4446)

Fix for the #4375 Add failing test for listener issue Make use of setImmediate to ensure event listeners are added after write transactions have completed. Update useQueryHook tests to give the event loop time to finish and add a listener. Fix a test in useObject render to allow event listners to fire after writes. Only use `setImmediate` if realm is in a write transaction. Update comments to be more clear.
realm · Apr 25, 2022 · 69fcf8e · 69fcf8e
1 parent f02968c
commit 69fcf8e
Show file tree

Hide file tree

Showing 8 changed files with 121 additions and 23 deletions.
diff --git a/packages/realm-react/CHANGELOG.md b/packages/realm-react/CHANGELOG.md
@@ -1,3 +1,17 @@
+x.x.x Release notes (yyyy-MM-dd)
+=============================================================
+### Enhancements
+* None.
+
+### Fixed
+* Fixed potential "Cannot create asynchronous query while in a write transaction" error with `useObject` due to adding event listeners while in a write transaction ([#4375](https://github.com/realm/realm-js/issues/4375), since v0.1.0)
+
+### Compatibility
+* None.
+
+### Internal
+* None.
+
 0.2.1 Release notes (2022-03-24)
 =============================================================
 ### Enhancements

diff --git a/packages/realm-react/src/__tests__/useObjectRender.test.tsx b/packages/realm-react/src/__tests__/useObjectRender.test.tsx
@@ -283,6 +283,7 @@ describe("useObject: rendering objects with a Realm.List property", () => {
         testRealm.write(() => {
           object.favoriteItem = object.items[0];
         });
+        forceSynchronousNotifications(testRealm);
       });
 
       expect(getByTestId(`favoriteItem-${object.items[0].name}`)).toBeTruthy();
@@ -291,6 +292,7 @@ describe("useObject: rendering objects with a Realm.List property", () => {
         testRealm.write(() => {
           object.favoriteItem = object.items[1];
         });
+        forceSynchronousNotifications(testRealm);
       });
 
       expect(getByTestId(`favoriteItem-${object.items[1].name}`)).toBeTruthy();
@@ -299,6 +301,7 @@ describe("useObject: rendering objects with a Realm.List property", () => {
         testRealm.write(() => {
           object.items[1].name = "apple";
         });
+        forceSynchronousNotifications(testRealm);
       });
 
       expect(getByTestId(`favoriteItem-${object.items[1].name}`)).toBeTruthy();

diff --git a/packages/realm-react/src/__tests__/useQueryHook.test.tsx b/packages/realm-react/src/__tests__/useQueryHook.test.tsx
@@ -78,9 +78,11 @@ describe("useQueryHook", () => {
     });
     realm.close();
   });
+
   afterEach(() => {
     Realm.clearTestState();
   });
+
   it("can retrieve collections using useQuery", () => {
     const { result } = renderHook(() => useQuery<IDog>("dog"));
     const collection = result.current;

diff --git a/packages/realm-react/src/__tests__/useQueryRender.test.tsx b/packages/realm-react/src/__tests__/useQueryRender.test.tsx
@@ -75,20 +75,20 @@ const tagRenderCounter = jest.fn();
 
 let testRealm: Realm = new Realm(configuration);
 
-const { useQuery, RealmProvider, useRealm } = createRealmContext(configuration);
+const { useQuery, useObject, RealmProvider, useRealm } = createRealmContext(configuration);
 
 enum QueryType {
   filtered,
   sorted,
   normal,
 }
 
-const App = ({ queryType = QueryType.normal }) => {
+const App = ({ queryType = QueryType.normal, useUseObject = false }) => {
   return (
     <RealmProvider>
       <SetupComponent>
         <View testID="testContainer">
-          <TestComponent queryType={queryType} />
+          <TestComponent queryType={queryType} useUseObject={useUseObject} />
         </View>
       </SetupComponent>
     </RealmProvider>
@@ -113,16 +113,24 @@ const SetupComponent = ({ children }: { children: JSX.Element }): JSX.Element |
   return children;
 };
 
+const UseObjectItemComponent: React.FC<{ item: Item & Realm.Object }> = React.memo(({ item }) => {
+  // Testing that useObject also works and properly handles renders
+  const localItem = useObject(Item, item.id);
+  if (!localItem) {
+    return null;
+  }
+  return <ItemComponent item={localItem}></ItemComponent>;
+});
+
 const ItemComponent: React.FC<{ item: Item & Realm.Object }> = React.memo(({ item }) => {
   itemRenderCounter();
   const realm = useRealm();
   const renderItem = useCallback(({ item }) => <TagComponent tag={item} />, []);
-
   const keyExtractor = useCallback((item) => `tag-${item.id}`, []);
 
   return (
     <View testID={`result${item.id}`}>
-      <View testID={`name${item.id}`}>
+      <View testID={`name${item.name}`}>
         <Text>{item.name}</Text>
       </View>
       <TextInput
@@ -169,7 +177,7 @@ const TagComponent: React.FC<{ tag: Tag & Realm.Object }> = React.memo(({ tag })
 const FILTER_ARGS: [string] = ["id < 20"];
 const SORTED_ARGS: [string, boolean] = ["id", true];
 
-const TestComponent = ({ queryType }: { queryType: QueryType }) => {
+const TestComponent = ({ queryType, useUseObject }: { queryType: QueryType; useUseObject: boolean }) => {
   const collection = useQuery(Item);
 
   const result = useMemo(() => {
@@ -183,7 +191,10 @@ const TestComponent = ({ queryType }: { queryType: QueryType }) => {
     }
   }, [queryType, collection]);
 
-  const renderItem = useCallback(({ item }) => <ItemComponent item={item} />, []);
+  const renderItem = useCallback(
+    ({ item }) => (useUseObject ? <UseObjectItemComponent item={item} /> : <ItemComponent item={item} />),
+    [useUseObject],
+  );
 
   const keyExtractor = useCallback((item) => item.id, []);
 
@@ -201,21 +212,26 @@ function getTestCollection(queryType: QueryType) {
   }
 }
 
-async function setupTest(queryType: QueryType) {
-  const renderResult = render(<App queryType={queryType} />);
+type setupOptions = {
+  queryType?: QueryType;
+  useUseObject?: boolean;
+};
+
+const setupTest = async ({ queryType = QueryType.normal, useUseObject = false }: setupOptions) => {
+  const renderResult = render(<App queryType={queryType} useUseObject={useUseObject} />);
   await waitFor(() => renderResult.getByTestId("testContainer"));
 
   const collection = getTestCollection(queryType);
   expect(itemRenderCounter).toHaveBeenCalledTimes(10);
 
   return { ...renderResult, collection };
-}
+};
 
 describe.each`
   queryTypeName | queryType
-  ${"sorted"}   | ${QueryType.sorted}
   ${"normal"}   | ${QueryType.normal}
   ${"filtered"} | ${QueryType.filtered}
+  ${"sorted"}   | ${QueryType.sorted}
 `("useQueryRender: $queryTypeName", ({ queryType }) => {
   beforeEach(() => {
     testRealm = new Realm(configuration);
@@ -234,7 +250,7 @@ describe.each`
     expect(itemRenderCounter).toHaveBeenCalledTimes(10);
   });
   it("change to data will rerender", async () => {
-    const { getByTestId, getByText, collection } = await setupTest(queryType);
+    const { getByTestId, getByText, collection } = await setupTest({ queryType });
 
     const firstItem = collection[0];
     const id = firstItem.id;
@@ -246,9 +262,10 @@ describe.each`
 
     fireEvent.changeText(input as ReactTestInstance, "apple");
 
-    // TODO: This line throws an `act` warning.  Keep an eye on this issue and see if there's a solution
-    // https://github.com/callstack/react-native-testing-library/issues/379
-    await waitFor(() => getByText("apple"));
+    // Wait for change events to finish their callbacks
+    await act(async () => {
+      forceSynchronousNotifications(testRealm);
+    });
 
     expect(nameElement).toHaveTextContent("apple");
     expect(itemRenderCounter).toHaveBeenCalledTimes(11);
@@ -257,7 +274,7 @@ describe.each`
   // TODO:  This is a known issue that we have to live with until it is possible
   // to retrieve the objectId from a deleted object in a listener callback
   it.skip("handles deletions", async () => {
-    const { getByTestId, collection } = await setupTest(queryType);
+    const { getByTestId, collection } = await setupTest({ queryType });
 
     const firstItem = collection[0];
     const id = firstItem.id;
@@ -277,7 +294,7 @@ describe.each`
     expect(itemRenderCounter).toHaveBeenCalledTimes(11);
   });
   it("an implicit update to an item in the FlatList view area causes a rerender", async () => {
-    const { collection } = await setupTest(queryType);
+    const { collection } = await setupTest({ queryType });
 
     testRealm.write(() => {
       collection[0].name = "apple";
@@ -292,7 +309,7 @@ describe.each`
   });
 
   it("does not rerender if the update is out of the FlatList view area", async () => {
-    const { collection } = await setupTest(queryType);
+    const { collection } = await setupTest({ queryType });
 
     testRealm.write(() => {
       const lastIndex = collection.length - 1;
@@ -306,7 +323,7 @@ describe.each`
     expect(itemRenderCounter).toHaveBeenCalledTimes(10);
   });
   it("collection objects rerender on changes to their linked objects", async () => {
-    const { collection, getByText, queryByText, debug } = await setupTest(queryType);
+    const { collection, getByText, queryByText } = await setupTest({ queryType });
 
     // Insert some tags into visible Items
     testRealm.write(() => {
@@ -354,4 +371,35 @@ describe.each`
 
     expect(queryByText("756c")).toBeNull();
   });
+  // This replicates the issue https://github.com/realm/realm-js/issues/4375
+  it("will handle multiple async transactions", async () => {
+    const { queryByTestId } = await setupTest({ queryType, useUseObject: true });
+    const performTest = async () => {
+      testRealm.write(() => {
+        testRealm.deleteAll();
+      });
+      let i = 0;
+      while (i < 10) {
+        await new Promise((resolve) => setTimeout(resolve, 10));
+        const id = i;
+        testRealm.write(() => {
+          testRealm.create(Item, { id, name: `${id}` }, Realm.UpdateMode.Modified);
+        });
+        await new Promise((resolve) => setTimeout(resolve, 0));
+        testRealm.write(() => {
+          const item = testRealm.objectForPrimaryKey(Item, id);
+          if (item) {
+            item.name = `${id + 100}`;
+          }
+        });
+        i++;
+      }
+    };
+
+    await act(async () => {
+      await performTest();
+    });
+
+    await waitFor(() => queryByTestId(`name${109}`));
+  });
 });
diff --git a/packages/realm-react/src/cachedCollection.ts b/packages/realm-react/src/cachedCollection.ts
@@ -31,6 +31,10 @@ type CachedCollectionArgs<T> = {
    * The {@link Realm.Collection} to proxy
    */
   collection: Realm.Collection<T>;
+  /**
+   * The {@link Realm} instance
+   */
+  realm: Realm;
   /**
    * Callback which is called whenever an object in the collection changes
    */
@@ -63,6 +67,7 @@ type CachedCollectionArgs<T> = {
  */
 export function createCachedCollection<T extends Realm.Object>({
   collection,
+  realm,
   updateCallback,
   objectCache = new Map(),
   isDerived = false,
@@ -77,6 +82,7 @@ export function createCachedCollection<T extends Realm.Object>({
             const col: Realm.Results<T & Realm.Object> = Reflect.apply(value, target, args);
             const { collection: newCol } = createCachedCollection({
               collection: col,
+              realm,
               updateCallback,
               objectCache,
               isDerived: true,
@@ -158,7 +164,15 @@ export function createCachedCollection<T extends Realm.Object>({
   };
 
   if (!isDerived) {
-    cachedCollectionResult.addListener(listenerCallback);
+    // If we are in a transaction, then push adding the listener to the event loop.  This will allow the write transaction to finish.
+    // see https://github.com/realm/realm-js/issues/4375
+    if (realm.isInTransaction) {
+      setImmediate(() => {
+        collection.addListener(listenerCallback);
+      });
+    } else {
+      collection.addListener(listenerCallback);
+    }
   }
 
   const tearDown = () => {

diff --git a/packages/realm-react/src/cachedObject.ts b/packages/realm-react/src/cachedObject.ts
@@ -26,6 +26,10 @@ type CachedObjectArgs<T> = {
    * The {@link Realm.Object} to proxy
    */
   object: T | null;
+  /**
+   * The {@link Realm} instance
+   */
+  realm: Realm;
   /**
    * Callback function called whenver the object changes. Used to force a component
    * using the {@link useObject} hook to re-render.
@@ -47,6 +51,7 @@ type CachedObjectArgs<T> = {
  */
 export function createCachedObject<T extends Realm.Object>({
   object,
+  realm,
   updateCallback,
 }: CachedObjectArgs<T>): { object: T | null; tearDown: () => void } {
   const listCaches = new Map();
@@ -75,7 +80,7 @@ export function createCachedObject<T extends Realm.Object>({
           // only the modified children of the list component actually re-render.
           return new Proxy(listCaches.get(key), {});
         }
-        const { collection, tearDown } = createCachedCollection({ collection: value, updateCallback });
+        const { collection, tearDown } = createCachedCollection({ collection: value, realm, updateCallback });
         // Add to a list of teardowns which will be invoked when the cachedObject's teardown is called
         listTearDowns.push(tearDown);
         // Store the proxied list into a map to persist the cachedCollection
@@ -104,7 +109,18 @@ export function createCachedObject<T extends Realm.Object>({
     }
   };
 
-  object.addListener(listenerCallback);
+  // We cannot add a listener to an invalid object
+  if (object.isValid()) {
+    // If we are in a transaction, then push adding the listener to the event loop.  This will allow the write transaction to finish.
+    // see https://github.com/realm/realm-js/issues/4375
+    if (realm.isInTransaction) {
+      setImmediate(() => {
+        object.addListener(listenerCallback);
+      });
+    } else {
+      object.addListener(listenerCallback);
+    }
+  }
 
   const tearDown = () => {
     object.removeListener(listenerCallback);

diff --git a/packages/realm-react/src/useObject.tsx b/packages/realm-react/src/useObject.tsx
@@ -59,6 +59,7 @@ export function createUseObject(useRealm: () => Realm) {
       () =>
         createCachedObject({
           object: realm.objectForPrimaryKey(type, primaryKey) ?? null,
+          realm,
           updateCallback: forceRerender,
         }),
       [type, realm, primaryKey],

diff --git a/packages/realm-react/src/useQuery.tsx b/packages/realm-react/src/useQuery.tsx
@@ -56,7 +56,7 @@ export function createUseQuery(useRealm: () => Realm) {
 
     // Wrap the cachedObject in useMemo, so we only replace it with a new instance if `primaryKey` or `type` change
     const { collection, tearDown } = useMemo(
-      () => createCachedCollection({ collection: realm.objects(type), updateCallback: forceRerender }),
+      () => createCachedCollection({ collection: realm.objects(type), realm, updateCallback: forceRerender }),
       [type, realm],
     );