From 69fcf8e5fb73279dd69df8b1281a576edbe061ba Mon Sep 17 00:00:00 2001 From: Andrew Meyer Date: Mon, 25 Apr 2022 09:56:49 +0200 Subject: [PATCH] Fix for adding event listeners while in a write transaction (#4446) Fix for the https://github.com/realm/realm-js/issues/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. --- packages/realm-react/CHANGELOG.md | 14 +++ .../src/__tests__/useObjectRender.test.tsx | 3 + .../src/__tests__/useQueryHook.test.tsx | 2 + .../src/__tests__/useQueryRender.test.tsx | 86 +++++++++++++++---- packages/realm-react/src/cachedCollection.ts | 16 +++- packages/realm-react/src/cachedObject.ts | 20 ++++- packages/realm-react/src/useObject.tsx | 1 + packages/realm-react/src/useQuery.tsx | 2 +- 8 files changed, 121 insertions(+), 23 deletions(-) diff --git a/packages/realm-react/CHANGELOG.md b/packages/realm-react/CHANGELOG.md index c38729fb52..8c09c91d08 100644 --- 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 index a88473d0d4..e115096e36 100644 --- 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 index 339275bc5d..4789a7d7b9 100644 --- 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("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 index aa5bb47bb2..b47f3b3e50 100644 --- a/packages/realm-react/src/__tests__/useQueryRender.test.tsx +++ b/packages/realm-react/src/__tests__/useQueryRender.test.tsx @@ -75,7 +75,7 @@ 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, @@ -83,12 +83,12 @@ enum QueryType { normal, } -const App = ({ queryType = QueryType.normal }) => { +const App = ({ queryType = QueryType.normal, useUseObject = false }) => { return ( - + @@ -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 ; +}); + const ItemComponent: React.FC<{ item: Item & Realm.Object }> = React.memo(({ item }) => { itemRenderCounter(); const realm = useRealm(); const renderItem = useCallback(({ item }) => , []); - const keyExtractor = useCallback((item) => `tag-${item.id}`, []); return ( - + {item.name} = 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 }) => , []); + const renderItem = useCallback( + ({ item }) => (useUseObject ? : ), + [useUseObject], + ); const keyExtractor = useCallback((item) => item.id, []); @@ -201,21 +212,26 @@ function getTestCollection(queryType: QueryType) { } } -async function setupTest(queryType: QueryType) { - const renderResult = render(); +type setupOptions = { + queryType?: QueryType; + useUseObject?: boolean; +}; + +const setupTest = async ({ queryType = QueryType.normal, useUseObject = false }: setupOptions) => { + const renderResult = render(); 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 index 4a9d0de352..9cfdf159bf 100644 --- a/packages/realm-react/src/cachedCollection.ts +++ b/packages/realm-react/src/cachedCollection.ts @@ -31,6 +31,10 @@ type CachedCollectionArgs = { * The {@link Realm.Collection} to proxy */ collection: Realm.Collection; + /** + * The {@link Realm} instance + */ + realm: Realm; /** * Callback which is called whenever an object in the collection changes */ @@ -63,6 +67,7 @@ type CachedCollectionArgs = { */ export function createCachedCollection({ collection, + realm, updateCallback, objectCache = new Map(), isDerived = false, @@ -77,6 +82,7 @@ export function createCachedCollection({ const col: Realm.Results = Reflect.apply(value, target, args); const { collection: newCol } = createCachedCollection({ collection: col, + realm, updateCallback, objectCache, isDerived: true, @@ -158,7 +164,15 @@ export function createCachedCollection({ }; 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 index 14e6f76f97..bfaead4a80 100644 --- a/packages/realm-react/src/cachedObject.ts +++ b/packages/realm-react/src/cachedObject.ts @@ -26,6 +26,10 @@ type CachedObjectArgs = { * 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 = { */ export function createCachedObject({ object, + realm, updateCallback, }: CachedObjectArgs): { object: T | null; tearDown: () => void } { const listCaches = new Map(); @@ -75,7 +80,7 @@ export function createCachedObject({ // 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({ } }; - 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 index d058d7aadc..57c4ac88b5 100644 --- 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 index a9b547b562..a9ec3a41a4 100644 --- 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], );