feat(hooks): optimistic update for nested relations #1842
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces modifications across multiple files, primarily enhancing the testing framework for optimistic updates and nested queries in the Tanstack Query library. New test cases have been added to cover various scenarios related to optimistic updates, while existing tests have been renamed for clarity. Additionally, the Changes
Assessment against linked issues
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (4)
packages/plugins/tanstack-query/tests/test-model-meta.ts (1)
48-55: Well-structured relationship definition!The explicit
foreignKeyMappingaddition clearly documents the relationship betweenPostandUsermodels, which is crucial for implementing optimistic updates across models.This pattern of explicit foreign key mapping enhances maintainability and makes the relationship between models unambiguous. Consider adopting this pattern consistently across all model relationships in the codebase to facilitate optimistic updates and improve code clarity.
packages/runtime/src/cross/mutator.ts (2)
37-39: Simplify type checking forqueryDataThe condition can be simplified because arrays are also objects in JavaScript. Checking for
typeof queryData !== 'object'is sufficient.Apply this diff to simplify the condition:
-if (!queryData || (typeof queryData !== 'object' && !Array.isArray(queryData))) { +if (!queryData || typeof queryData !== 'object') { return undefined; }
429-445: Handle missing foreign key mappings inassignForeignKeyFieldsIf
field.foreignKeyMappingis undefined, the function exits silently. Consider handling this case to prevent potential issues.Apply this diff to add a warning when foreign key mapping is missing:
if (!field.foreignKeyMapping) { + if (logging) { + console.warn(`No foreign key mapping for field ${field.name}`); + } return; }packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx (1)
Line range hint
1093-1093: Correct mutation data field inactcallAt line 1093, the mutation data uses
{ name: 'post2' }, but since we're updating aPost, the field should likely betitleinstead ofname.Apply this diff to correct the field name:
- mutationResult.current.mutate({ where: { id: '1' }, data: { name: 'post2' } }); + mutationResult.current.mutate({ where: { id: '1' }, data: { title: 'post2' } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx(5 hunks)packages/plugins/tanstack-query/tests/test-model-meta.ts(1 hunks)packages/runtime/src/cross/mutator.ts(9 hunks)
🔇 Additional comments (5)
packages/runtime/src/cross/mutator.ts (1)
275-277:
Ensure consistent ID field types in idFieldsMatch
Using strict equality (===) may cause mismatches if ID fields have differing types (e.g., number vs. string). Ensure ID fields are of consistent types or adjust the comparison.
Run the following script to check for inconsistent ID field types:
packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx (4)
626-629: Ensure correct HTTP method for mutations
In the useModelMutation call at lines 626-629, the HTTP method is set to 'PUT' for updating a 'Post'. Ensure that 'PUT' is the correct method for your API. If the API expects 'PATCH' for partial updates, adjust accordingly.
Confirm the expected HTTP method with the backend API specifications.
Line range hint 1268-1268: Ensure consistent handling of non-ZenStack queries
At line 1268, while testing optimistic updates mixed with non-ZenStack queries, verify that the non-ZenStack queries are not affected by the optimistic updates. This is important to maintain isolation between different query types.
Review the query keys and caching strategies to ensure they don't interfere with each other.
389-389: Avoid nesting creates when performing mutations
At line 389, the mutation includes nested creation of posts within a user creation. While Prisma supports nested mutations, it's important to ensure that the backend API handles this correctly. Verify that the backend supports nested creates in this context.
Run the following script to confirm that the backend API endpoint accepts nested creates:
#!/bin/bash
# Description: Check if the 'User' model supports nested 'create' operations with 'posts'.
# Test: Look for 'create' operations in the model definitions that include 'posts'.
rg 'model User' -A 20 | rg 'posts\s*:\s*{\s*create'344-344: Verify the optimistic update reflects in the cache
At line 344, ensure that the optimistic post added to the user's posts array has all necessary fields and that $optimistic is properly set. This is crucial for the cache to represent the optimistic state accurately.
Run the following script to check the cache data structure:
✅ Verification successful
The optimistic update is correctly reflected in the cache
Based on the test file content, the verification at line 344 is part of a comprehensive test case that checks optimistic updates when creating posts. The test confirms that:
- The optimistic post is correctly added to the cache with all required fields
- The
$optimisticflag is properly set totrue - The post contains the expected fields:
id,title, andownerId - The values match the expected data:
title: 'post1'andownerId: '1'
The test assertion is accurate and verifies the proper implementation of optimistic updates in the cache.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the optimistic post in the cache has all required fields.
# Test: Retrieve the cache data for the User query and inspect the posts array.
expected_fields=('id' 'title' 'ownerId' '\$optimistic')
rg --json 'getQueryKey.*User.*findMany' | jq '.data' | grep -E "$(IFS='|'; echo "${expected_fields[*]}")"
Length of output: 344
Script:
#!/bin/bash
# Let's analyze the test file to understand the optimistic update implementation
rg -A 10 "\$optimistic.*true" packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx
# Also check the test setup and assertions around optimistic updates
rg -B 5 -A 5 "toMatchObject.*\$optimistic" packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx
# Look for cache-related utility functions
ast-grep --pattern 'function $_(queryClient$_) {
$$$
}'
Length of output: 258
Script:
#!/bin/bash
# Let's try a different approach to find the test context
cat packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx
# Also look for any optimistic update related patterns
rg -l "optimistic" packages/plugins/tanstack-query/tests/
# Search for cache-related test utilities
rg -l "queryClient" packages/plugins/tanstack-query/tests/
Length of output: 47825
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx (1)
313-318: Mock response should return created dataThe mock response for the Post creation returns
null, which doesn't match typical API behavior. Consider returning the created post data to better simulate real-world scenarios and ensure proper handling of the actual response.nock(makeUrl('Post', 'create')) .post(/.*/) .reply(200, () => { console.log('Not mutating data'); - return { data: null }; + return { data: { id: expect.any(String), title: 'post1', ownerId: '1' } }; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx(7 hunks)
🔇 Additional comments (3)
packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx (3)
593-648: LGTM! Well-structured test for nested relation updates
The test case thoroughly validates the optimistic update behavior when modifying both scalar fields (title) and relationships (owner) of a nested model. It ensures that the cache is properly updated with the optimistic changes.
Line range hint 284-1285: Excellent test coverage for optimistic updates with nested relations
The test suite comprehensively covers:
- Direct and nested mutations
- All CRUD operations (create, read, update, delete)
- Various relationship scenarios
- Edge cases with null responses
This aligns perfectly with the PR objective of implementing optimistic updates for related models.
996-999: Verify upsert operation structure
The upsert operation's structure should be reviewed to ensure it follows Prisma's specifications. Consider adding comments to clarify the expected behavior of each field in the upsert operation.
✅ Verification successful
Let me search for more specific upsert operations in test files to better understand the context.
Let me try a different search pattern focusing on the test file in question.
Let me try one final search with a broader pattern to understand the test context.
Based on the review of both the original code snippet and the search results from the codebase, I can now provide the final response:
The upsert operation structure is correct and follows Prisma's specifications
The upsert operation in the test follows the standard Prisma upsert structure with the required where, create, and update fields. The structure matches other upsert operations found in the codebase and is being used appropriately in the context of testing optimistic updates for nested relations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the upsert operation structure in the codebase
# Test: Search for similar upsert operations to confirm the structure
rg -A 5 'upsert.*where.*create.*update'
Length of output: 4286
Script:
#!/bin/bash
# Search for upsert operations specifically in test files under packages/plugins
rg -A 5 'upsert.*where.*create.*update' packages/plugins/
Length of output: 59
Script:
#!/bin/bash
# Search for upsert operations in the specific test file
cat "packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx" | grep -A 10 -B 10 "upsert.*where.*create.*update"
Length of output: 118
Script:
#!/bin/bash
# Get the surrounding context of the upsert operation in the test file
cat "packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx" | grep -A 15 -B 15 "optimistic upsert - update updating nested query"
Length of output: 1185
fixes #1840