merge dev to main (v2.9.3) #1886
Conversation
…relationships have null values (#1884) Co-authored-by: Simon Zimmerman <[email protected]>
📝 WalkthroughWalkthroughThe changes in this pull request include updates to the Changes
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: 1
🧹 Outside diff range and nitpick comments (15)
SECURITY.md (1)
11-11: Consider enhancing the security contact informationWhile the email contact method is clear, consider these enhancements:
- Use markdown link syntax instead of bare URL
- Add PGP key information for encrypted communication
-Please send an email to [email protected]. We'll actively watch, verify, and fix them with high priority. +Please send an email to [[email protected]](mailto:[email protected]). We'll actively watch, verify, and fix them with high priority.🧰 Tools
🪛 LanguageTool
[style] ~11-~11: Consider using “email”.
Context: ...| ## Reporting a Vulnerability Please send an email to [email protected]. We'll actively wa...(SEND_AN_EMAIL)
🪛 Markdownlint (0.35.0)
11-11: null
Bare URL used(MD034, no-bare-urls)
packages/ide/jetbrains/CHANGELOG.md (2)
Line range hint
1-8: Update version number for releaseSince this PR is merging v2.9.3 to main, the "Unreleased" section should be updated to "2.9.3" to reflect the current release version.
Apply this diff:
# Changelog -## [Unreleased] +## 2.9.3 ### Fixed - Proper semantic highlighting and formatting for type declarations.
5-7: Improve changelog entry formattingThe formatting of the changelog entry could be improved by removing unnecessary blank lines between the section header and the bullet point.
Apply this diff:
### Fixed - - Proper semantic highlighting and formatting for type declarations. +- Proper semantic highlighting and formatting for type declarations.packages/plugins/tanstack-query/tests/test-model-meta.ts (1)
77-96: Add uniqueConstraints configuration for consistencyThe category model implementation looks good overall, but it's missing the
uniqueConstraintsconfiguration that's present in other models.Consider adding the uniqueConstraints configuration for consistency:
}, + uniqueConstraints: { id: { name: 'id', fields: ['id'] } }, },packages/schema/src/language-server/zmodel-formatter.ts (1)
107-107: Consider improving error handling in the fallback caseWhile the type handling is correct, the fallback case could be more informative for debugging purposes.
Consider this improvement:
- } else { - // we shouldn't get here - length = 1; + } else { + console.warn(`Unexpected field type encountered: ${JSON.stringify(field.type)}`); + length = 1; // fallback to minimum lengthAlso applies to: 114-119
packages/runtime/src/cross/mutator.ts (1)
164-166: LGTM with optimization suggestion: Consider lazy cloningThe changes correctly prevent mutation during iteration by cloning the data upfront. However, consider deferring the clone operation until an update is actually needed to optimize performance.
Here's a potential optimization:
- // Clone resultData to prevent mutations affecting the loop - const currentData = { ...resultData }; - - // iterate over each field and apply mutation to nested data models - for (const [key, value] of Object.entries(currentData)) { + // iterate over each field and apply mutation to nested data models + for (const [key, value] of Object.entries(resultData)) { const fieldInfo = modelFields[key]; if (!fieldInfo?.isDataModel) { continue; } const r = await doApplyMutation( fieldInfo.type, value, mutationModel, mutationOp, mutationArgs, modelMeta, logging ); if (r && typeof r === 'object') { - resultData = { ...resultData, [key]: r }; + // Clone only when we need to update + if (!updated) { + resultData = { ...resultData }; + updated = true; + } + resultData[key] = r; - updated = true; } }Also applies to: 168-168, 184-187
packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx (9)
14-14: Consider using environment variables forBASE_URL.The
BASE_URLis hardcoded as'http://localhost'. For better flexibility across different environments, consider using an environment variable or configuration file to set theBASE_URL.
30-32: Simplify themakeUrlfunction by handling query parameters more robustly.Currently, the function constructs the URL manually and appends query parameters using string concatenation. Consider using the
URLandURLSearchParamsAPIs for more robust URL construction and to properly handle complex query parameters.Apply this refactor:
- function makeUrl(model: string, operation: string, args?: unknown) { - let r = `${BASE_URL}/api/model/${model}/${operation}`; - if (args) { - r += `?q=${encodeURIComponent(JSON.stringify(args))}`; - } - return r; - } + function makeUrl(model: string, operation: string, args?: unknown) { + const url = new URL(`${BASE_URL}/api/model/${model}/${operation}`); + if (args) { + url.searchParams.append('q', JSON.stringify(args)); + } + return url.toString(); + }
390-390: Fix typo in the comment on line 390.The comment has a typo: 'pupulate' should be 'populate'.
504-504: Fix typo in the comment on line 504.The word 'relatonship' should be 'relationship'.
548-549: Clarify console log messages in mock replies.The console messages like 'Mutating data' or 'Not mutating data' might be misleading. Ensure that the log messages accurately reflect whether data mutation is simulated in the mock replies to improve test readability.
Also applies to: 644-645, 832-833
355-356: Avoid using theanytype for test data structures.Using
anyreduces type safety and can hide potential issues. Define interfaces or types foruserData,categoryData, andpostDatato leverage TypeScript's type checking.For example:
interface User { id: string; name: string; posts: Post[]; } interface Post { id: string; title: string; ownerId: string; categoryId?: string | null; category?: Category | null; } interface Category { id: string; name: string; posts: Post[]; } // Then use these types: const userData: User[] = [{ id: '1', name: 'user1', posts: [] }];Also applies to: 391-392, 598-603
501-591: Correct the typo in the test case comment and ensure consistency.In the test starting at line 501, the comment has a typo: 'relatonship' should be 'relationship'. Also, ensure that the test case descriptions accurately reflect the tested scenarios.
Line range hint
820-821: Ensure consistent cleanup of mock interceptors after tests.Consider adding
nock.cleanAll()in theafterEachhook to ensure that mock interceptors do not interfere with other tests, maintaining test isolation.
622-629: Align object properties in code blocks for better readability.In the test queries, the nested
includeobjects can be formatted to align properties, enhancing readability and making the structure clearer.For example:
include: { posts: { include: { category: true, }, }, },Also applies to: 672-675
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (13)
package.jsonis excluded by!**/*.jsonpackages/ide/jetbrains/package.jsonis excluded by!**/*.jsonpackages/language/package.jsonis excluded by!**/*.jsonpackages/misc/redwood/package.jsonis excluded by!**/*.jsonpackages/plugins/openapi/package.jsonis excluded by!**/*.jsonpackages/plugins/swr/package.jsonis excluded by!**/*.jsonpackages/plugins/tanstack-query/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/package.jsonis excluded by!**/*.jsonpackages/runtime/package.jsonis excluded by!**/*.jsonpackages/schema/package.jsonis excluded by!**/*.jsonpackages/sdk/package.jsonis excluded by!**/*.jsonpackages/server/package.jsonis excluded by!**/*.jsonpackages/testtools/package.jsonis excluded by!**/*.json
📒 Files selected for processing (8)
SECURITY.md(1 hunks)packages/ide/jetbrains/CHANGELOG.md(1 hunks)packages/ide/jetbrains/build.gradle.kts(1 hunks)packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx(3 hunks)packages/plugins/tanstack-query/tests/test-model-meta.ts(1 hunks)packages/runtime/src/cross/mutator.ts(3 hunks)packages/schema/src/language-server/zmodel-formatter.ts(2 hunks)packages/schema/src/language-server/zmodel-semantic.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/ide/jetbrains/build.gradle.kts
🧰 Additional context used
🪛 LanguageTool
SECURITY.md
[style] ~11-~11: Consider using “email”.
Context: ...| ## Reporting a Vulnerability Please send an email to [email protected]. We'll actively wa...
(SEND_AN_EMAIL)
🪛 Markdownlint (0.35.0)
SECURITY.md
11-11: null
Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (9)
SECURITY.md (1)
7-7: Verify impact of minimum supported version change
The increase in minimum supported version from 0.4.0 to 1.0.0 is significant.
✅ Verification successful
Version support change to 1.0.0+ is appropriate
The version update in SECURITY.md is justified and safe to proceed:
- No active issues reference version 0.4.0
- Pre-1.0.0 versions only appear in CHANGELOG.md as historical records
- The project is now at v2.9.3, making 1.0.0 a reasonable minimum supported version
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any references to pre-1.0.0 version support in docs and issues
# Check documentation files for pre-1.0.0 version references
rg -i "0\.[0-9]+\.[0-9]+" --type md
# Check open issues mentioning pre-1.0.0 versions
gh issue list --search "0.4.0 in:body" --json number,title,body
Length of output: 242
packages/ide/jetbrains/CHANGELOG.md (1)
Line range hint 1-8: Verify the completeness of v2.9.3 changes
Let's verify if there are any additional changes that should be documented for v2.9.3 by checking the git history.
packages/schema/src/language-server/zmodel-semantic.ts (2)
21-22: LGTM: New type definition imports
The new imports are properly integrated and follow the existing import pattern.
41-41: Verify semantic token types for new type definitions
The integration of type definition checks follows the existing pattern correctly. However, let's verify that these new types don't require any special highlighting rules different from their peer declarations.
Also applies to: 49-49
✅ Verification successful
Semantic token types are correctly assigned for type definitions
Based on the search results, the semantic token assignments are consistent with the existing patterns:
- Type definitions (
isTypeDef) useSemanticTokenTypes.type, matching other type-level declarations like DataSource, Generator, Plugin, and Enum - Type definition fields (
isTypeDefField) useSemanticTokenTypes.variable, consistent with other field-level declarations
The implementation follows the established semantic highlighting patterns in the codebase, and no special highlighting rules are needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any special highlighting rules for type definitions
# in related configuration files or documentation
# Look for any type-related token configurations
rg -l "type.*token" --type ts
# Look for any documentation or examples of type definition highlighting
fd -e md -e txt . | xargs rg -l "type.*highlight"
# Check for any test cases related to type definition highlighting
fd "test|spec" -e ts | xargs rg -l "type.*highlight"
Length of output: 138
Script:
#!/bin/bash
# Let's try a different approach to verify semantic token handling
# Check semantic token type definitions and usage
rg -A 5 "SemanticTokenTypes" --type ts
# Look for type-related token mappings
rg -A 5 "type['\"].*:" --type ts
# Check test files specifically for type definition cases
fd "test|spec" -e ts | xargs rg "isTypeDef|isTypeDefField"
# Look for any existing semantic highlighting patterns
rg "registerProvider.*semantic" --type ts
Length of output: 140894
Script:
#!/bin/bash
# Let's check how type definitions are handled in semantic highlighting tests and implementations
# Look for semantic token type assignments for type definitions
rg -A 2 "SemanticTokenTypes.*type" packages/schema/src/language-server/zmodel-semantic.ts
# Check for any test cases involving type definitions
rg -A 2 "isTypeDef|isTypeDefField" packages/schema/src/
# Look for any semantic token type mappings in language configuration
rg -A 2 "tokenTypes|semanticTokens" packages/schema/src/
Length of output: 14955
packages/plugins/tanstack-query/tests/test-model-meta.ts (2)
57-96: Verify test coverage for the new relationships
The model changes look well-structured. Let's ensure they are properly covered by tests.
✅ Verification successful
Test coverage for category relationships is comprehensive
The test file includes thorough coverage of the category-post relationships, including:
- Initialization of category data with empty posts array
- Creating and connecting posts to categories
- Verification of optimistic updates in the cache
- Handling of optional (null) category relationships
- Testing both directions of the relationship (category->posts and post->category)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for tests using the new category relationships
rg -A 5 "category.*post|post.*category" --type ts ./packages/plugins/tanstack-query/tests/
Length of output: 5513
57-73: Consider updating deleteCascade configuration for the new relationship
The category-post relationship implementation looks good, with proper foreign key mapping and backlinking. However, the deleteCascade configuration at the bottom of the file hasn't been updated to handle the new relationship between categories and posts.
Let's check if there are any cascade delete tests that might be affected:
Consider updating the deleteCascade configuration to specify the behavior when a category is deleted:
deleteCascade: {
user: ['Post'],
+ category: ['Post'],
},packages/schema/src/language-server/zmodel-formatter.ts (1)
29-30: LGTM: Clean extension of formatting support to TypeDef nodes
The condition has been properly extended to handle both DataModel and TypeDef nodes while maintaining consistent formatting logic.
packages/runtime/src/cross/mutator.ts (1)
Line range hint 154-163: LGTM: Enhanced type safety in array item handling
The additional type check typeof r === 'object' is a good defensive programming practice that prevents potential issues with non-object values during array updates.
packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx (1)
350-499: Good job on the 'optimistic create updating deeply nested query' test case.
This test case effectively covers the scenario of creating a post connected to both a user and a category with optimistic updates. It ensures the cache is properly updated in deeply nested queries.
No description provided.