Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Apr 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions packages/realm-react/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
3 changes: 3 additions & 0 deletions packages/realm-react/src/__tests__/useObjectRender.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions packages/realm-react/src/__tests__/useQueryHook.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
86 changes: 67 additions & 19 deletions packages/realm-react/src/__tests__/useQueryRender.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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
Expand Down Expand Up @@ -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(() => {
Expand All @@ -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, []);

Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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";
Expand All @@ -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;
Expand All @@ -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(() => {
Expand Down Expand Up @@ -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}`;
}
});
tomduncalf marked this conversation as resolved.
Show resolved Hide resolved
i++;
}
};

await act(async () => {
await performTest();
});

await waitFor(() => queryByTestId(`name${109}`));
});
});
16 changes: 15 additions & 1 deletion packages/realm-react/src/cachedCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -63,6 +67,7 @@ type CachedCollectionArgs<T> = {
*/
export function createCachedCollection<T extends Realm.Object>({
collection,
realm,
updateCallback,
objectCache = new Map(),
isDerived = false,
Expand All @@ -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,
Expand Down Expand Up @@ -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 = () => {
Expand Down
20 changes: 18 additions & 2 deletions packages/realm-react/src/cachedObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions packages/realm-react/src/useObject.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export function createUseObject(useRealm: () => Realm) {
() =>
createCachedObject({
object: realm.objectForPrimaryKey(type, primaryKey) ?? null,
realm,
updateCallback: forceRerender,
}),
[type, realm, primaryKey],
Expand Down
2 changes: 1 addition & 1 deletion packages/realm-react/src/useQuery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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],
);

Expand Down