-
-
Notifications
You must be signed in to change notification settings - Fork 14
merge dev to main (v3.0.0-beta.32) #508
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
Conversation
Co-authored-by: ymc9 <[email protected]>
WalkthroughMonorepo version bump to 3.0.0-beta.32 across packages, refactors ORM type-level select/include/omit logic with stricter generics, adds cascade delete to template relations, handles many-to-many delegated models, and introduces regression tests for issues 503 and 505. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude finished @ymc9's task —— View job PR Review: Merge dev to main (v3.0.0-beta.32)This PR merges development changes to main for the v3.0.0-beta.32 release. Overall, this is a well-structured release with appropriate bug fixes and comprehensive test coverage. Key Changes Analyzed:✅ Version Bump Consistency
✅ Functional Code Changes - ORM Packagepackages/orm/src/client/crud-types.ts - Significant TypeScript type improvements:
packages/orm/src/client/query-utils.ts - Error handling enhancement:
✅ New Regression TestsIssue #503: Tests complex nested creation with media relations
Issue #505: Tests delegate model relations
✅ Security Assessment
✅ Performance Considerations
Minor Observations:
Recommendations:✅ Approve: This is a clean, well-tested release with appropriate bug fixes and no security concerns. The type system improvements in the ORM package are particularly valuable for developer experience. Overall Assessment: High quality release ready for production. The combination of bug fixes, improved type inference, and comprehensive test coverage makes this a solid beta release. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR merges the dev branch to main, releasing version 3.0.0-beta.32. It includes bug fixes for many-to-many relations on delegate models, TypeScript type improvements, and several quality-of-life enhancements.
Key changes:
- Critical fix for delegate model m2m relations: Resolves issue #505 where join table names were incorrectly determined for many-to-many relations on delegate models by using the
originModelfield - TypeScript type refactoring: Simplifies complex conditional types in
ModelResultto improve type inference and reduce nesting - Path handling improvement: Uses relative paths in generated import statements for better portability
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| package.json + 19 package.json files | Version bump from 3.0.0-beta.31 to 3.0.0-beta.32 across all packages |
| packages/orm/src/client/query-utils.ts | Fixes m2m join table name resolution for delegate models by using originModel field |
| packages/orm/src/client/crud-types.ts | Refactors ModelResult type to simplify conditional logic and improve type inference |
| packages/cli/src/actions/generate.ts | Uses path.relative() for generated import paths instead of absolute paths |
| packages/cli/src/actions/templates.ts | Adds onDelete: Cascade to Post-User relation in starter template |
| tests/regression/package.json | Renames "generate" script to "test:generate" for consistency |
| tests/regression/test/issue-505.test.ts | Adds regression test for m2m relations on delegate models (issue #505) |
| tests/regression/test/issue-503/* | Adds regression test and generated files for nested relation selection (issue #503) |
| tests/e2e/orm/schemas/typing/typecheck.ts | Adds take: 1 to test nested relation selection with filtering |
| packages/ide/vscode/package.json | Version bump for VSCode extension from 3.0.14 to 3.0.15 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/regression/test/issue-505.test.ts (2)
4-5: Consider a more descriptive test name.The test name "verifies the issue" doesn't explain what behavior is being validated. Consider renaming to something like "verifies many-to-many relationships work with delegated models" or "verifies bidirectional relations between delegated Media and Message models" for better test documentation.
Also, minor inconsistency: the suite name uses plural "issues" but refers to a single issue number.
58-60: Consider using more specific assertions for consistency.The assertion compares the entire
photoobject, which works but could be fragile if the returned object has additional fields. For consistency with the second assertion (line 66) and better maintainability, consider usingexpect.objectContainingto check specific fields.Apply this diff for more robust assertions:
- expect(message).toMatchObject({ - media: [photo], - }); + expect(message).toMatchObject({ + media: [expect.objectContaining({ + id: photo.id, + tgFileId: 'file123', + type: 'TelegramPhoto', + })], + });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
package.json(1 hunks)packages/auth-adapters/better-auth/package.json(1 hunks)packages/cli/package.json(1 hunks)packages/cli/src/actions/generate.ts(1 hunks)packages/cli/src/actions/templates.ts(1 hunks)packages/clients/tanstack-query/package.json(1 hunks)packages/common-helpers/package.json(1 hunks)packages/config/eslint-config/package.json(1 hunks)packages/config/typescript-config/package.json(1 hunks)packages/config/vitest-config/package.json(1 hunks)packages/create-zenstack/package.json(1 hunks)packages/ide/vscode/package.json(1 hunks)packages/language/package.json(1 hunks)packages/orm/package.json(1 hunks)packages/orm/src/client/crud-types.ts(2 hunks)packages/orm/src/client/query-utils.ts(1 hunks)packages/plugins/policy/package.json(1 hunks)packages/schema/package.json(1 hunks)packages/sdk/package.json(1 hunks)packages/server/package.json(1 hunks)packages/testtools/package.json(1 hunks)packages/zod/package.json(1 hunks)samples/next.js/package.json(1 hunks)samples/orm/package.json(1 hunks)tests/e2e/orm/schemas/typing/typecheck.ts(1 hunks)tests/e2e/package.json(1 hunks)tests/regression/package.json(1 hunks)tests/regression/test/issue-503/input.ts(1 hunks)tests/regression/test/issue-503/models.ts(1 hunks)tests/regression/test/issue-503/regression.test.ts(1 hunks)tests/regression/test/issue-503/schema.ts(1 hunks)tests/regression/test/issue-503/schema.zmodel(1 hunks)tests/regression/test/issue-505.test.ts(1 hunks)tests/runtimes/bun/package.json(1 hunks)tests/runtimes/edge-runtime/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.zmodel
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.zmodel: Always runzenstack generateafter modifying ZModel schemas
ZModel schema files should define database structure and policies that compile to TypeScript viazenstack generate
Files:
tests/regression/test/issue-503/schema.zmodel
tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests should validate real-world schema compatibility with established projects
Files:
tests/e2e/orm/schemas/typing/typecheck.ts
tests/**/type*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure TypeScript inference and type coverage are validated through type coverage tests
Files:
tests/e2e/orm/schemas/typing/typecheck.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
packages/orm/package.jsontests/regression/test/issue-503/models.tspackages/orm/src/client/query-utils.tspackages/cli/src/actions/generate.tstests/e2e/orm/schemas/typing/typecheck.tspackages/orm/src/client/crud-types.tstests/regression/test/issue-503/input.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Use Kysely as the query builder interface for low-level database queries, avoiding raw SQL when possible
Applied to files:
packages/orm/package.jsontests/regression/test/issue-503/models.tspackages/orm/src/client/query-utils.tspackages/cli/src/actions/generate.tspackages/orm/src/client/crud-types.tstests/regression/test/issue-503/input.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
Applied to files:
packages/orm/package.jsontests/regression/test/issue-505.test.tstests/regression/test/issue-503/regression.test.tspackages/cli/src/actions/generate.tspackages/orm/src/client/crud-types.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to ide/vscode/**/{package.json,version.ts} : The VSCode IDE extension package should maintain a different version from other packages to comply with VSCode Marketplace requirements
Applied to files:
packages/config/vitest-config/package.jsonpackages/ide/vscode/package.json
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
packages/language/package.jsontests/regression/test/issue-503/schema.zmodelpackages/schema/package.jsontests/regression/test/issue-503/models.tspackages/zod/package.jsonpackages/cli/src/actions/generate.tstests/regression/test/issue-503/schema.tstests/regression/test/issue-503/input.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
tests/regression/package.jsontests/regression/test/issue-503/models.tstests/regression/test/issue-505.test.tstests/regression/test/issue-503/regression.test.tstests/regression/test/issue-503/schema.tstests/e2e/package.jsontests/e2e/orm/schemas/typing/typecheck.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Use `pnpm` with workspaces for package management, pinned to version `pnpm10.12.1`
Applied to files:
package.json
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
packages/orm/src/client/query-utils.tstests/regression/test/issue-503/schema.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/**/type*.{ts,tsx} : Ensure TypeScript inference and type coverage are validated through type coverage tests
Applied to files:
tests/regression/test/issue-503/regression.test.tstests/e2e/orm/schemas/typing/typecheck.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Database migrations should use Prisma CLI under the hood via ZenStack commands
Applied to files:
packages/cli/src/actions/generate.ts
🧬 Code graph analysis (3)
tests/regression/test/issue-503/regression.test.ts (2)
packages/testtools/src/client.ts (1)
createTestClient(101-248)tests/regression/test/issue-503/schema.ts (1)
schema(109-109)
tests/regression/test/issue-503/schema.ts (2)
packages/schema/src/schema.ts (1)
SchemaDef(11-19)packages/schema/src/expression-utils.ts (1)
ExpressionUtils(19-123)
packages/orm/src/client/crud-types.ts (1)
packages/schema/src/schema.ts (5)
NonRelationFields(215-219)RelationFields(221-225)RelationFieldType(233-240)ModelFieldIsOptional(242-246)FieldIsArray(266-270)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Agent
- GitHub Check: claude-review
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: build-test (20.x, postgresql)
🔇 Additional comments (37)
tests/e2e/orm/schemas/typing/typecheck.ts (1)
170-170: LGTM! Appropriate test coverage for nested relation limits.The addition of
take: 1correctly validates that TypeScript inference works with limit clauses in nested selects, aligning with the PR's type-level refinements for select/include/omit logic.tests/regression/test/issue-505.test.ts (2)
6-34: LGTM! Schema correctly models delegated many-to-many relationships.The schema properly demonstrates the delegation pattern with
@@delegate(type)and extends, combined with an implicit many-to-many relationship between Media and Message. This effectively tests the issue described in the PR summary regarding many-to-many delegated models.
36-56: LGTM! Test data creation correctly demonstrates the relationship.The test properly creates a TelegramPhoto, then creates a TelegramMessage with a connection to that photo using the correct connect syntax. The include clause appropriately fetches the relation for verification.
packages/sdk/package.json (1)
3-3: LGTM! Coordinated version bump.The version increment from 3.0.0-beta.31 to 3.0.0-beta.32 aligns with the monorepo release strategy.
packages/server/package.json (1)
3-3: LGTM! Coordinated version bump.The version increment from 3.0.0-beta.31 to 3.0.0-beta.32 aligns with the monorepo release strategy.
packages/auth-adapters/better-auth/package.json (1)
3-3: LGTM! Coordinated version bump.The version increment from 3.0.0-beta.31 to 3.0.0-beta.32 aligns with the monorepo release strategy.
packages/clients/tanstack-query/package.json (1)
3-3: LGTM! Coordinated version bump.The version increment from 3.0.0-beta.31 to 3.0.0-beta.32 aligns with the monorepo release strategy.
tests/runtimes/bun/package.json (1)
3-3: LGTM! Coordinated version bump.The version increment from 3.0.0-beta.31 to 3.0.0-beta.32 aligns with the monorepo release strategy.
package.json (1)
3-3: LGTM! Coordinated version bump.The version increment from 3.0.0-beta.31 to 3.0.0-beta.32 aligns with the monorepo release strategy.
packages/testtools/package.json (1)
3-3: LGTM! Coordinated version bump.The version increment from 3.0.0-beta.31 to 3.0.0-beta.32 aligns with the monorepo release strategy.
packages/ide/vscode/package.json (1)
4-4: LGTM! VSCode extension version bump follows separate versioning.The version increment from 3.0.14 to 3.0.15 is correct. The VSCode extension maintains a different version scheme from other packages to comply with VSCode Marketplace requirements. Based on learnings, this is expected behavior.
packages/common-helpers/package.json (1)
3-3: LGTM! Version bump is consistent with the release.The version update from 3.0.0-beta.31 to 3.0.0-beta.32 aligns with the coordinated monorepo release preparation.
packages/zod/package.json (1)
3-3: LGTM! Version bump is consistent with the release.The version update aligns with the monorepo-wide 3.0.0-beta.32 release.
samples/next.js/package.json (1)
3-3: LGTM! Version bump is consistent with the release.The sample application version correctly tracks the monorepo release version.
packages/language/package.json (1)
4-4: LGTM! Version bump is consistent with the release.The version update aligns with the coordinated 3.0.0-beta.32 release.
samples/orm/package.json (1)
3-3: LGTM! Version bump is consistent with the release.The sample version correctly reflects the monorepo-wide 3.0.0-beta.32 release.
packages/plugins/policy/package.json (1)
3-3: LGTM! Version bump is consistent with the release.The version update aligns with the coordinated monorepo release to 3.0.0-beta.32.
packages/config/typescript-config/package.json (1)
3-3: LGTM! Version bump is consistent with the release.The configuration package version correctly tracks the 3.0.0-beta.32 release.
tests/e2e/package.json (1)
3-3: LGTM! Version bump is consistent with the release.The test package version correctly reflects the monorepo-wide 3.0.0-beta.32 release.
packages/create-zenstack/package.json (1)
3-3: LGTM! Version bump is consistent with the monorepo release.The version bump from 3.0.0-beta.31 to 3.0.0-beta.32 aligns with the coordinated release across all packages in the PR.
packages/config/eslint-config/package.json (1)
3-3: LGTM! Version bump is consistent with the monorepo release.The version bump from 3.0.0-beta.31 to 3.0.0-beta.32 aligns with the coordinated release across all packages in the PR.
tests/runtimes/edge-runtime/package.json (1)
3-3: LGTM! Version bump is consistent with the monorepo release.The version bump from 3.0.0-beta.31 to 3.0.0-beta.32 aligns with the coordinated release across all packages in the PR.
packages/config/vitest-config/package.json (1)
4-4: LGTM! Version bump is consistent with the monorepo release.The version bump from 3.0.0-beta.31 to 3.0.0-beta.32 aligns with the coordinated release across all packages in the PR.
packages/cli/package.json (1)
6-6: LGTM! Version bump is consistent with the monorepo release.The version bump from 3.0.0-beta.31 to 3.0.0-beta.32 aligns with the coordinated release across all packages in the PR.
packages/schema/package.json (1)
3-3: LGTM! Version bump is consistent with the monorepo release.The version bump from 3.0.0-beta.31 to 3.0.0-beta.32 aligns with the coordinated release across all packages in the PR.
packages/cli/src/actions/templates.ts (1)
24-24: LGTM! Cascade delete is a sensible default for the starter template.Adding
onDelete: Cascadeensures that when a User is deleted, their Posts are automatically removed. This is a reasonable default for the blog-style starter template and prevents orphaned Post records.Note that this changes the behavior for new projects created from this template—deleting a user will now cascade to their posts.
packages/cli/src/actions/generate.ts (1)
39-39: Path resolution change improves import statement portability.The use of
path.relative('.', outputPath)correctly generates relative import paths that work across different project structures and working directories. The path.relative() method handles path resolution from the current working directory, properly supporting absolute paths and cross-platform compatibility without requiring explicit edge case handling in this code.tests/regression/package.json (1)
3-9: LGTM!Version bump and script consolidation look correct. The new
test:generatescript provides a unified entry point for test generation, and thebuildandtestscripts are properly wired to use it.packages/orm/src/client/query-utils.ts (1)
235-258: LGTM!The handling for m2m relations inherited from delegate bases is correctly implemented. Using
originModelwhen present ensures:
- Join table names are computed based on the actual model owning the relation
- FK ordering uses the correct model for non-self-relations
- ID fields are retrieved from the correct model
The fallback to
modelwhenoriginModelis undefined preserves backward compatibility.packages/orm/src/client/crud-types.ts (2)
141-170: LGTM!The
ModelSelectResultrefactor improves clarity:
- Key filtering logic is now more explicit with separate handling for
_count,Omit, and regular keys- Relation field recursion is unified into a single
ModelResultcall, eliminating prior branching variants
190-218: LGTM!The
ModelResultrefactor enforces stricter type constraints:
selectandincludeare now constrained toextends object, preventing invalid types- The
includerecursion path is simplified by passingI[Key]directly toModelResult, which correctly handles both booleantrue(falling through toDefaultModelResult) and nested query objectspackages/orm/package.json (1)
3-3: LGTM!Version bump to 3.0.0-beta.32 aligns with the monorepo release.
tests/regression/test/issue-503/regression.test.ts (1)
1-31: LGTM!Well-structured regression test that validates nested relation creation and retrieval (InternalChat → Message → Media). The test exercises the select/include type paths that were refactored in this PR, ensuring the nested
mediarelation is correctly included and retrievable.tests/regression/test/issue-503/schema.ts (1)
1-109: LGTM! Generated schema file is correctly structured.The generated schema properly implements the SchemaDef interface with all three models (InternalChat, Message, Media) and their bidirectional relations. The foreign key constraints and optional fields are correctly defined per the source schema.
tests/regression/test/issue-503/schema.zmodel (1)
1-23: LGTM! Valid regression test schema.The ZModel schema correctly defines three models with proper bidirectional relations and foreign key constraints. The optional media relation on Message is appropriately marked with nullable FK.
As per coding guidelines, ensure
zenstack generatehas been run to regenerate the TypeScript artifacts after this schema was added.tests/regression/test/issue-503/models.ts (1)
1-12: LGTM! Correctly generated model type aliases.The file properly exports ModelResult type aliases for all three models, providing type-safe access to model results in tests.
tests/regression/test/issue-503/input.ts (1)
1-70: LGTM! Comprehensive CRUD type definitions.The file correctly generates all necessary type aliases for CRUD operations, query arguments, and payload types for the three models. The generic constraints on GetPayload types are properly defined.
Summary by CodeRabbit
Release Notes
Chores
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.