zenstack icon indicating copy to clipboard operation
zenstack copied to clipboard

Bug - Tanstack Plugin V5 - Optimistic updates incorrectly modify unrelated entities within the cache.

Open roomman opened this issue 1 year ago • 1 comments

Description and expected behavior When performing an optimistic update to create a new record for a specific model, the new record is erroneously added to another model’s data in the cache.

Additional context The following test should be added to packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx on the dev branch in order to replicate the issue:

    it('should apply optimistic updates to user1 without affecting user2', async () => {
        const { queryClient, wrapper } = createWrapper();

        // populate the cache with two users each with a post
        const userData: any[] = [
            { id: '1', name: 'user1', posts: [{ id: '1', title: 'post1' }] },
            { id: '2', name: 'user2', posts: [{ id: '2', title: 'post2' }] },
        ];

        nock(BASE_URL)
            .get('/api/model/User/findMany')
            .query(true)
            .reply(200, () => {
                console.log('Querying data:', JSON.stringify(userData));
                return { data: userData };
            })
            .persist();

        const { result: userResult } = renderHook(
            () =>
                useModelQuery(
                    'User',
                    makeUrl('User', 'findMany'),
                    { include: { posts: true } },
                    { optimisticUpdate: true }
                ),
            {
                wrapper,
            }
        );

        await waitFor(() => {
            expect(userResult.current.data).toHaveLength(2);
        });

        // mock a post request to create a new post for user1
        nock(BASE_URL)
            .post('/api/model/Post/create')
            .reply(200, () => {
                console.log('Not mutating data');
                return { data: null };
            });

        const { result: mutationResult } = renderHook(
            () =>
                useModelMutation('Post', 'POST', makeUrl('Post', 'create'), modelMeta, {
                    optimisticUpdate: true,
                    invalidateQueries: false,
                }),
            {
                wrapper,
            }
        );

        act(() =>
            mutationResult.current.mutate({
                data: { title: 'post3', owner: { connect: { id: '1' } } },
            })
        );

        // assert that the post was created and connected to user1, and user2 was unaffected
        await waitFor(() => {
            const cacheData: any = queryClient.getQueryData(
                getQueryKey(
                    'User',
                    'findMany',
                    { include: { posts: true } },
                    { infinite: false, optimisticUpdate: true }
                )
            );
            const user1 = cacheData.find((user: any) => user.id === '1');
            expect(user1.posts).toHaveLength(2);
            expect(user1.posts.find((post: any) => post.title === 'post3')).toMatchObject({
                $optimistic: true,
                id: expect.any(String),
                title: 'post3',
                ownerId: '1',
            });

            const user2 = cacheData.find((user: any) => user.id === '2');
            expect(user2.posts).toHaveLength(1);
            expect(user2.posts.find((post: any) => post.title === 'post3')).toBeUndefined();
        });
    });

The test fails because 'post3' appears in the cached data for both users, not just user1.

roomman avatar Nov 26 '24 14:11 roomman

Yes, that part of the analysis is too loose. I'll try to make some improvements in the next release.

ymc9 avatar Nov 27 '24 22:11 ymc9