merge dev to main (v3.0.0-alpha.16) #132
Conversation
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
WalkthroughThis change transitions all code, documentation, and templates from configuring the ZenStack client with a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–25 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
|
Claude finished @ymc9's task —— View job PR Review: Merge dev to mainReview Tasks
✅ Code Review SummaryThis PR merges development changes that primarily focus on simplifying the Kysely dialect configuration and bumping the version to 3.0.0-alpha.16. The changes represent a significant improvement in the API design. 🔍 Key Changes Analysis1. Runtime API Simplification ⭐Location: What Changed:
Code Quality: ✅ Excellent
// Before (removed complexity)
dialectConfig: DialectConfig<Schema['provider']>
private getKyselyDialect() { /* complex provider matching */ }
// After (cleaner)
dialect: Dialect2. Test Infrastructure Updates ✅Location: What Changed:
Code Quality: ✅ Good
3. Version Consistency ✅Location: All What Changed:
🚀 Performance Implications✅ Positive Impact:
🔒 Security Assessment✅ No Security Concerns:
🐛 Potential Issues & RecommendationsMinor Issues Found:
Suggestions:
🧪 Test Coverage Assessment✅ Comprehensive:
📊 Overall Assessment
✅ Recommendation: APPROVEThis PR represents a high-quality architectural improvement that:
The breaking change is justified as it significantly improves the developer experience and aligns better with Kysely's design principles. |
There was a problem hiding this comment.
Pull Request Overview
This PR merges development branch changes to main, primarily involving a version bump from 3.0.0-alpha.15 to 3.0.0-alpha.16 and a significant API change to replace dialectConfig with dialect for database configuration in the ZenStack runtime.
- Version bump across all packages to 3.0.0-alpha.16
- Migration from
dialectConfigtodialectparameter for database configuration - Updated imports and usage patterns to use Kysely dialect classes directly
Reviewed Changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| package.json | Root package version bump |
| packages/*/package.json | Version bumps across all packages |
| packages/runtime/src/client/options.ts | API change: replaced dialectConfig with dialect parameter |
| packages/runtime/src/client/client-impl.ts | Removed dialect creation logic, now uses provided dialect directly |
| packages/runtime/package.json | Moved kysely from dependencies to peerDependencies |
| samples/blog/* | Updated to use new dialect API with SqliteDialect |
| test files | Updated test setup to use new dialect configuration |
| documentation | Updated examples to reflect new API usage |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/testtools/package.json (1)
3-3: Keep changelog or release notes alignedSince
@zenstackhq/testtoolsnow targets3.0.0-alpha.16, ensure the package’s changelog (if any) includes an entry for this release so consumers understand the change—even if it’s only dependency alignment.packages/cli/src/actions/generate.ts (1)
52-55: Documentation updated to reflect new dialect API.The change from
dialectConfigtodialectcorrectly reflects the new client initialization pattern. The example aligns with the broader refactor moving to explicit Kysely dialect instances.Consider providing a more concrete example to better guide users:
const client = new ZenStackClient(schema, { - dialect: { ... } + dialect: new SqliteDialect({ database: db }) });This would give users a clearer understanding of the expected format.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
README.md(2 hunks)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/common-helpers/package.json(1 hunks)packages/create-zenstack/package.json(1 hunks)packages/eslint-config/package.json(1 hunks)packages/ide/vscode/package.json(1 hunks)packages/language/package.json(1 hunks)packages/runtime/package.json(2 hunks)packages/runtime/src/client/client-impl.ts(2 hunks)packages/runtime/src/client/options.ts(2 hunks)packages/runtime/test/client-api/computed-fields.test.ts(2 hunks)packages/runtime/test/client-api/default-values.test.ts(2 hunks)packages/runtime/test/client-api/name-mapping.test.ts(3 hunks)packages/runtime/test/plugin/kysely-on-query.test.ts(2 hunks)packages/runtime/test/plugin/mutation-hooks.test.ts(2 hunks)packages/runtime/test/plugin/query-lifecycle.test.ts(2 hunks)packages/runtime/test/query-builder/query-builder.test.ts(2 hunks)packages/runtime/test/schemas/delegate/typecheck.ts(1 hunks)packages/runtime/test/schemas/typing/typecheck.ts(1 hunks)packages/runtime/test/utils.ts(4 hunks)packages/sdk/package.json(1 hunks)packages/tanstack-query/package.json(1 hunks)packages/testtools/package.json(1 hunks)packages/typescript-config/package.json(1 hunks)packages/zod/package.json(1 hunks)samples/blog/README.md(1 hunks)samples/blog/main.ts(1 hunks)samples/blog/package.json(2 hunks)tests/e2e/package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/runtime/test/schemas/typing/typecheck.ts (2)
packages/runtime/src/client/client-impl.ts (1)
ZenStackClient(40-46)packages/runtime/test/schemas/delegate/schema.ts (1)
schema(9-464)
packages/runtime/test/schemas/delegate/typecheck.ts (2)
packages/runtime/src/client/client-impl.ts (1)
ZenStackClient(40-46)packages/runtime/test/schemas/delegate/schema.ts (1)
schema(9-464)
samples/blog/main.ts (1)
samples/blog/zenstack/schema.ts (1)
schema(9-228)
packages/runtime/test/utils.ts (2)
packages/runtime/src/client/options.ts (1)
ClientOptions(24-64)packages/sdk/src/schema/schema.ts (1)
SchemaDef(10-18)
⏰ 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). (3)
- GitHub Check: claude-review
- GitHub Check: build-test (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (47)
tests/e2e/package.json (1)
3-3: Confirm lock-file regeneration after version bumpThe package’s version has been raised to
3.0.0-alpha.16. Make sure you committed the updatedpnpm-lock.yaml(or equivalent) so CI installs the expected set of dependencies.packages/language/package.json (1)
4-4: Version bump looks good – double-check peer/workspace rangesNothing else changed in this manifest. Verify that downstream packages referencing
@zenstackhq/languageviaworkspace:*don’t pin an explicit3.0.0-alpha.15, otherwise the monorepo install may become inconsistent.package.json (1)
3-3: Root version updated – ensure release tooling picks this upThe root package now reports
3.0.0-alpha.16. Confirm that any release scripts (e.g.,turbo, CI workflows) rely on this value or the individual package versions as intended, so publishing tags remain in sync.packages/typescript-config/package.json (1)
3-3: LGTM – no additional action requiredThe version increment is consistent with the rest of the workspace.
packages/cli/package.json (1)
6-6: Version bump aligns with the workspace release – LGTM
The update keeps the CLI package in sync with the coordinatedalpha.16release across the monorepo.packages/sdk/package.json (1)
3-3: Version bump confirmed – no further action needed
@zenstackhq/sdknow matches the new prerelease tag.packages/eslint-config/package.json (1)
3-3: Consistent version update – looks good
Configuration package reflects the collectivealpha.16bump.packages/zod/package.json (1)
3-3: Package version increment verified
The@zenstackhq/zodpackage is correctly updated to3.0.0-alpha.16.packages/common-helpers/package.json (1)
3-3: Version bump matches the rest of the release
@zenstackhq/common-helpersnow points toalpha.16; no other changes spotted.packages/tanstack-query/package.json (1)
3-3: LGTM! Version bump is consistent with coordinated release.The version increment from
3.0.0-alpha.15to3.0.0-alpha.16aligns with the broader ecosystem changes for the dialect configuration refactor.packages/create-zenstack/package.json (1)
3-3: LGTM! Version bump maintains consistency across packages.The version increment to
3.0.0-alpha.16is appropriate for maintaining version alignment across the ZenStack ecosystem during this dialect configuration refactor.packages/ide/vscode/package.json (1)
4-4: LGTM! VSCode extension version stays aligned with runtime changes.The version increment to
3.0.0-alpha.16ensures the VSCode extension version remains consistent with the underlying ZenStack runtime changes.samples/blog/package.json (2)
3-3: Version bump aligns with coordinated release.The version increment to
3.0.0-alpha.16maintains consistency across the sample projects and core packages.
17-18: Kysely dependency added for new dialect pattern.The addition of
kyselydependency is necessary for the transition fromdialectConfigobjects to explicit Kysely dialect instances (e.g.,SqliteDialect). This change supports the broader API refactor described in the PR.packages/runtime/package.json (2)
3-3: LGTM - Version bump aligns with breaking API change.The version increment to
3.0.0-alpha.16is appropriate for the breaking change fromdialectConfigtodialectproperty.
79-79: LGTM - Moving kysely to peerDependencies is the right architectural decision.Moving
kyselyfromdependenciestopeerDependenciesmakes sense since users now provide their own dialect instances, avoiding potential version conflicts and reducing bundle size.packages/runtime/test/plugin/kysely-on-query.test.ts (2)
2-2: LGTM - Correct import addition for SqliteDialect.The import statement properly adds
SqliteDialectfrom the kysely package to support the new dialect configuration pattern.
12-12: LGTM - Proper migration to explicit dialect instance.The client initialization correctly migrates from
dialectConfigobject to the newdialectproperty using an explicitSqliteDialectinstance.packages/runtime/test/client-api/computed-fields.test.ts (1)
58-58: LGTM - Appropriate use of mock dialect for type checking tests.The migration from
dialectConfigtodialect: {} as anyis appropriate for these type-focused tests where actual database functionality isn't required, only type validation.Also applies to: 125-125
packages/runtime/test/query-builder/query-builder.test.ts (2)
3-3: LGTM - Correct import addition for SqliteDialect.The import statement properly adds
SqliteDialectfrom the kysely package to support the new dialect configuration pattern.
13-13: LGTM - Proper migration to explicit dialect instance.The client initialization correctly migrates from
dialectConfigobject to the newdialectproperty using an explicitSqliteDialectinstance.packages/runtime/test/plugin/query-lifecycle.test.ts (2)
2-2: LGTM - Correct import addition for SqliteDialect.The import statement properly adds
SqliteDialectfrom the kysely package to support the new dialect configuration pattern.
12-12: LGTM - Proper migration to explicit dialect instance.The client initialization correctly migrates from
dialectConfigobject to the newdialectproperty using an explicitSqliteDialectinstance.packages/runtime/test/plugin/mutation-hooks.test.ts (2)
2-2: LGTM! Import update aligns with dialect refactor.The addition of
SqliteDialectimport fromkyselyis consistent with the broader refactor to use explicit dialect instances.
12-12: LGTM! Client configuration updated correctly.The change from
dialectConfigtodialectproperty with explicitSqliteDialectinstantiation is consistent with the broader API refactor and maintains the same functionality.packages/runtime/test/schemas/typing/typecheck.ts (2)
2-2: LGTM! Import addition aligns with dialect refactor.The addition of
SqliteDialectimport fromkyselyis consistent with the broader API changes.
7-8: LGTM! Client initialization updated correctly.The change from
dialectConfigtodialectproperty with explicitSqliteDialectinstantiation aligns with the broader refactor while maintaining the same database functionality.packages/runtime/test/client-api/default-values.test.ts (2)
3-3: LGTM! Import addition supports dialect refactor.The addition of
SqliteDialectimport fromkyselyis consistent with the migration to explicit dialect instances.
72-72: LGTM! Client configuration migrated correctly.The change from
dialectConfigtodialectproperty with explicitSqliteDialectinstantiation maintains test functionality while aligning with the new API pattern.packages/runtime/test/schemas/delegate/typecheck.ts (2)
2-2: LGTM! Import addition supports API migration.The addition of
SqliteDialectimport fromkyselyis consistent with the broader dialect refactor.
7-7: LGTM! Client initialization updated appropriately.The change from
dialectConfigtodialectproperty with explicitSqliteDialectinstantiation aligns with the API refactor while maintaining the same database functionality.packages/runtime/test/client-api/name-mapping.test.ts (3)
2-2: LGTM! Import addition supports dialect refactor.The addition of
SqliteDialectimport fromkyselyis consistent with the migration to explicit dialect instances.
63-63: LGTM! First test client configuration updated correctly.The change from
dialectConfigtodialectproperty with explicitSqliteDialectinstantiation maintains test functionality while aligning with the API refactor.
95-95: LGTM! Second test client configuration updated correctly.The change from
dialectConfigtodialectproperty with explicitSqliteDialectinstantiation is consistent with the broader refactor and maintains test functionality.samples/blog/main.ts (1)
3-3: LGTM! Dialect configuration updated correctly.The import of
SqliteDialectand the transition fromdialectConfigtodialectproperty with explicit dialect instantiation follows the new API pattern correctly.Also applies to: 8-8
samples/blog/README.md (1)
27-27: Documentation correctly updated for new dialect API.The example code properly demonstrates the new pattern of importing
SqliteDialectand using it as an explicit dialect instance rather than a configuration object.Also applies to: 30-30
packages/cli/src/actions/templates.ts (1)
31-32: CLI template correctly updated for new dialect configuration.The template now properly imports
SqliteDialectand uses explicit dialect instantiation. The multi-line formatting for the dialect configuration also improves readability.Also applies to: 36-38
README.md (2)
148-148: SQLite example correctly updated for new dialect API.The SQLite example properly demonstrates importing
SqliteDialectand using it as an explicit dialect instance.Also applies to: 151-151
160-160: PostgreSQL example correctly updated for new dialect API.The PostgreSQL example properly demonstrates importing
PostgresDialectand using it with the connection pool configuration.Also applies to: 165-167
packages/runtime/src/client/options.ts (1)
1-3: Core API successfully simplified to use explicit dialect instances.The change from
dialectConfigtodialect: Dialectremoves internal dialect construction complexity and makes the API more explicit. This is a breaking change that requires users to instantiate Kysely dialect classes directly, but it provides better type safety and clarity.Also applies to: 26-28
packages/runtime/test/utils.ts (5)
6-6: LGTM! Proper import of Kysely dialect classes.The import of
PostgresDialectandSqliteDialectfrom kysely is correctly added to support the new explicit dialect configuration pattern.
24-24: LGTM! Correct migration from dialectConfig to dialect instance.The change from a configuration object to an explicit
SqliteDialectinstance with an in-memory SQLite database is appropriate for test utilities.
50-55: LGTM! Proper PostgreSQL dialect configuration.The PostgreSQL dialect is correctly instantiated with a connection pool using the test configuration. The multi-line formatting enhances readability.
61-61: LGTM! Type definition correctly updated.The
CreateTestClientOptionstype has been properly updated to omitdialectinstead ofdialectConfig, maintaining consistency with the new client options structure.
170-179: LGTM! Consistent dialect instantiation in createTestClient.Both PostgreSQL and SQLite dialect instances are correctly created with appropriate configurations. The provider-specific logic ensures the right dialect is used for each database type.
packages/runtime/src/client/client-impl.ts (2)
2-2: LGTM! Import optimization for QueryExecutor type.The import has been optimized to only include the
QueryExecutortype, which aligns with the simplified client implementation that no longer handles dialect construction internally.
87-99: LGTM! Simplified client construction with external dialect instance.The client implementation has been significantly simplified by directly using the provided
dialectinstance from options instead of constructing dialects internally. This change:
- Removes complex dialect-specific construction logic
- Makes the client more focused on its core responsibilities
- Delegates dialect management to the caller
- Maintains proper initialization of driver, compiler, and adapter components
The approach is cleaner and more maintainable.
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores