merge dev to main (v3.0.0-alpha.17) #137
Conversation
* feat: add sql.js dialect * update * update
Co-authored-by: ymc9 <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Caution Review failedThe pull request is closed. WalkthroughThis update introduces a new SQL.js dialect package for Kysely, including its implementation, configuration, and a comprehensive test suite. It also standardizes CLI and documentation conventions, adds a shared Vitest configuration package, updates validation logic for datasources, and bumps versions across multiple packages. Minor code and documentation cleanups are included. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✨ 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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Claude finished @ymc9's task —— View job Pull Request ReviewReviewing this dev to main merge with focus on code quality, potential issues, performance, security, and test coverage. Todo List:
Review SummaryThis PR introduces significant improvements including a new sql.js dialect, unified vitest configuration, and version bumps. The changes are well-structured and follow the project conventions outlined in CLAUDE.md. Key Changes Reviewed✅ New SQL.js Dialect Package (
|
There was a problem hiding this comment.
Pull Request Overview
This PR merges development branch changes to main, updating the project from version 3.0.0-alpha.16 to 3.0.0-alpha.17. The changes focus on consolidating Vitest configuration, improving validation logic, adding new dialect support, and enhancing error handling.
- Centralizes Vitest configuration into a shared package across the workspace
- Simplifies validation logic by removing error collection and datasource URL validation requirements
- Adds new SQL.js dialect package for Kysely integration with comprehensive test coverage
Reviewed Changes
Copilot reviewed 48 out of 49 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/vitest-config/* | New shared Vitest configuration package with base settings |
| packages/dialects/sql.js/* | New Kysely dialect implementation for SQL.js with complete test suite |
| tests/e2e/prisma-consistency/* | Simplified validation testing by removing error message collection |
| packages/language/src/validators/datasource-validator.ts | Relaxed datasource validation to make URL field optional |
| packages/cli/src/index.ts | Enhanced error handling with proper exception catching |
| **/package.json | Version bumps and dependency updates across workspace |
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: 4
🧹 Nitpick comments (14)
packages/vitest-config/package.json (1)
1-10: Consider tightening the published surface even for private packages
Although the package is markedprivate, adding an explicitfileswhitelist future-proofs the package ifprivateis ever flipped and avoids accidentally publishing stray artefacts.{ "name": "@zenstackhq/vitest-config", "type": "module", "version": "3.0.0-alpha.17", "private": true, "license": "MIT", + "files": ["base.config.js"], "exports": { "./base": "./base.config.js" } }packages/vitest-config/base.config.js (1)
3-12: Broaden spec pattern and externalise long timeouts
Limiting to*.test.tsmisses the common*.spec.tsconvention, and the hard-coded 100 s timeout can mask performance regressions.export default defineConfig({ test: { deps: { interopDefault: true, }, - include: ['**/*.test.ts'], - testTimeout: 100000, - hookTimeout: 100000, + include: ['**/*.{test,spec}.ts'], + // Allow CI to override; falls back to 30 s locally + testTimeout: Number(process.env.VITEST_TIMEOUT ?? 30000), + hookTimeout: Number(process.env.VITEST_TIMEOUT ?? 30000), }, });packages/dialects/sql.js/tsup.config.ts (1)
3-13: Add build‐target clarity and keep deps external
Explicitplatform,target, andexternalsettings reduce bundle size and avoid polyfills sneaking in.export default defineConfig({ entry: { index: 'src/index.ts', }, outDir: 'dist', splitting: false, sourcemap: true, clean: true, dts: true, - format: ['cjs', 'esm'], + format: ['cjs', 'esm'], + platform: 'node', + target: 'es2020', + external: ['kysely', 'sql.js'], });README.md (1)
130-141: Avoid jargon like “database db” – clarify wordingThe phrase “before being used to create a database db” is awkward (“db” already abbreviates database).
Suggest re-phrasing to something like “before being used to create a database client” or “before being used to instantiate the database object” for precision and readability.packages/cli/src/actions/generate.ts (1)
52-55: Snippet variable name inconsistent with new docsThe generated usage snippet still declares
const client = new ZenStackClient(...), whereas the README now standardizes onconst db = ....
For consistency (and to prevent copy-paste friction) renameclient→db.-const client = new ZenStackClient(schema, { +const db = new ZenStackClient(schema, {packages/language/package.json (1)
4-4: Remember to update the CHANGELOG alongside the version bumpThe package version is incremented to
3.0.0-alpha.17, but there’s no indication in this PR of a corresponding CHANGELOG entry. Keeping the CHANGELOG in sync with published versions helps consumers track changes.packages/dialects/sql.js/tsconfig.json (1)
1-7: Exclude build output to prevent accidental type-checking of emitted filesAlthough
includedoesn’t matchdist/**, future glob changes or IDE defaults could cause TSC to pick up compiled JS/DTs indist, leading to duplicate-identifier errors.{ "extends": "@zenstackhq/typescript-config/base.json", "compilerOptions": { "outDir": "dist" + }, + "exclude": ["dist"], - }, - "include": ["src/**/*", "test/**/*"] + "include": ["src/**/*", "test/**/*"] }tests/e2e/package.json (1)
13-14: devDependency versions rely on the workspace tagUsing
"workspace:*"means you depend on whatever version the workspace currently resolves to.
If you intend E2E tests to be reproducible outside the mono-repo (e.g., in CI downloading onlye2epackage), consider pinning versions or publishing the shared config package first.TODO.md (2)
13-16: Fix Markdown list indentation (MD007)The new sub-items are indented with 4 spaces instead of 2 / 4 as required by
markdownlint. This is already flagged by CI tools.- - [ ] plugin mechanism - - [ ] built-in plugins - - [ ] ts - - [ ] prisma + - [ ] plugin mechanism + - [ ] built-in plugins + - [ ] ts + - [ ] prismaApply the same 2-space indentation rule to nested lists elsewhere (see Line 80).
80-80: Consistent nesting level
- [ ] Self relationshould align with its sibling items:- - [ ] Self relation + - [ ] Self relationThis prevents broken bullet rendering.
packages/dialects/sql.js/src/types.ts (1)
1-5: Optional: broaden config shape for future settingsThe interface currently exposes only the
Databaseinstance. Future options (e.g.,busyTimeout, custom pragma flags) would require a breaking change.-export interface SqlJsDialectConfig { - sqlJs: Database; -} +export interface SqlJsDialectConfig { + /** + * An initialised sql.js database instance. + */ + sqlJs: Database; + /** + * Future-proof extension point. + */ + readonly options?: Record<string, unknown>; +}Not mandatory, but adding an optional bag now avoids a major version bump later.
packages/dialects/sql.js/README.md (1)
1-1: Fix bare URL formatting.The GitHub URL should be properly formatted as a Markdown link to follow best practices.
-Forked from https://github.com/betarixm/kysely-sql-js +Forked from [kysely-sql-js](https://github.com/betarixm/kysely-sql-js)packages/dialects/sql.js/test/getting-started/person-repository.ts (2)
9-33: Well-implemented dynamic query building.The function correctly handles Kysely's immutable query building pattern and includes proper null handling for the
last_namefield. The inline comment on Line 13 is helpful for developers new to Kysely.Consider that the current truthy checks might not work correctly for boolean fields or numeric 0 values if they exist in the Person type.
35-37: LGTM!Simple and correct update implementation. For production use, consider returning the updated record or the number of affected rows to verify the update succeeded.
📜 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 (48)
README.md(15 hunks)TODO.md(2 hunks)package.json(1 hunks)packages/cli/package.json(2 hunks)packages/cli/src/actions/action-utils.ts(1 hunks)packages/cli/src/actions/generate.ts(1 hunks)packages/cli/src/index.ts(2 hunks)packages/cli/vitest.config.ts(1 hunks)packages/common-helpers/package.json(1 hunks)packages/create-zenstack/package.json(1 hunks)packages/dialects/sql.js/README.md(1 hunks)packages/dialects/sql.js/eslint.config.js(1 hunks)packages/dialects/sql.js/package.json(1 hunks)packages/dialects/sql.js/src/connection.ts(1 hunks)packages/dialects/sql.js/src/dialect.ts(1 hunks)packages/dialects/sql.js/src/driver.ts(1 hunks)packages/dialects/sql.js/src/index.ts(1 hunks)packages/dialects/sql.js/src/types.ts(1 hunks)packages/dialects/sql.js/test/getting-started/database.ts(1 hunks)packages/dialects/sql.js/test/getting-started/person-repository.test.ts(1 hunks)packages/dialects/sql.js/test/getting-started/person-repository.ts(1 hunks)packages/dialects/sql.js/test/getting-started/types.ts(1 hunks)packages/dialects/sql.js/tsconfig.json(1 hunks)packages/dialects/sql.js/tsup.config.ts(1 hunks)packages/dialects/sql.js/vitest.config.ts(1 hunks)packages/eslint-config/package.json(1 hunks)packages/ide/vscode/package.json(1 hunks)packages/language/package.json(2 hunks)packages/language/src/index.ts(1 hunks)packages/language/src/validators/datasource-validator.ts(1 hunks)packages/language/vitest.config.ts(1 hunks)packages/runtime/package.json(2 hunks)packages/runtime/vitest.config.ts(1 hunks)packages/sdk/package.json(1 hunks)packages/sdk/src/ts-schema-generator.ts(1 hunks)packages/tanstack-query/package.json(1 hunks)packages/testtools/package.json(1 hunks)packages/typescript-config/package.json(1 hunks)packages/vitest-config/base.config.js(1 hunks)packages/vitest-config/package.json(1 hunks)packages/zod/package.json(1 hunks)pnpm-workspace.yaml(0 hunks)samples/blog/package.json(1 hunks)tests/e2e/package.json(1 hunks)tests/e2e/prisma-consistency/datasource.test.ts(1 hunks)tests/e2e/prisma-consistency/relations-one-to-many.test.ts(1 hunks)tests/e2e/prisma-consistency/test-utils.ts(0 hunks)tests/e2e/vitest.config.ts(1 hunks)
💤 Files with no reviewable changes (2)
- pnpm-workspace.yaml
- tests/e2e/prisma-consistency/test-utils.ts
🧰 Additional context used
📓 Path-based instructions (2)
{packages,samples,tests}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/dialects/sql.js/eslint.config.jspackages/dialects/sql.js/test/getting-started/types.tspackages/dialects/sql.js/test/getting-started/person-repository.test.tstests/e2e/prisma-consistency/datasource.test.tspackages/dialects/sql.js/package.jsonpackages/language/src/index.tspackages/dialects/sql.js/src/types.tspackages/cli/src/actions/generate.tspackages/ide/vscode/package.jsonpackages/common-helpers/package.jsonpackages/vitest-config/base.config.jspackages/runtime/package.jsonpackages/zod/package.jsonpackages/cli/package.jsontests/e2e/package.jsonpackages/vitest-config/package.jsonpackages/cli/vitest.config.tspackages/language/vitest.config.tspackages/tanstack-query/package.jsonpackages/dialects/sql.js/src/index.tspackages/cli/src/index.tspackages/dialects/sql.js/tsconfig.jsonpackages/dialects/sql.js/tsup.config.tspackages/eslint-config/package.jsonpackages/dialects/sql.js/test/getting-started/person-repository.tspackages/dialects/sql.js/src/connection.tspackages/dialects/sql.js/src/dialect.tspackages/dialects/sql.js/vitest.config.tstests/e2e/vitest.config.tspackages/typescript-config/package.jsonpackages/language/src/validators/datasource-validator.tssamples/blog/package.jsonpackages/language/package.jsonpackages/testtools/package.jsonpackages/runtime/vitest.config.tspackages/dialects/sql.js/README.mdpackages/create-zenstack/package.jsonpackages/sdk/src/ts-schema-generator.tstests/e2e/prisma-consistency/relations-one-to-many.test.tspackages/cli/src/actions/action-utils.tspackages/dialects/sql.js/test/getting-started/database.tspackages/sdk/package.jsonpackages/dialects/sql.js/src/driver.ts
tests/e2e/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
E2E tests are in
tests/e2e/directory
Files:
tests/e2e/prisma-consistency/datasource.test.tstests/e2e/package.jsontests/e2e/vitest.config.tstests/e2e/prisma-consistency/relations-one-to-many.test.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.149Z
Learning: Kysely-Based ORM: V3 uses Kysely as query builder instead of Prisma runtime dependency
📚 Learning: `zenstack generate` compiles zmodel to typescript schema (`schema.ts`)...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.149Z
Learning: `zenstack generate` compiles ZModel to TypeScript schema (`schema.ts`)
Applied to files:
packages/dialects/sql.js/test/getting-started/types.tstests/e2e/prisma-consistency/datasource.test.tspackages/dialects/sql.js/package.jsonpackages/cli/src/actions/generate.tspackages/ide/vscode/package.jsonpackages/zod/package.jsonpackages/cli/package.jsonpackages/vitest-config/package.jsonpackages/tanstack-query/package.jsonTODO.mdpackages/cli/src/index.tspackages/dialects/sql.js/tsconfig.jsonpackages/dialects/sql.js/tsup.config.tsREADME.mdpackages/typescript-config/package.jsonpackage.jsonpackages/language/package.jsonpackages/create-zenstack/package.jsontests/e2e/prisma-consistency/relations-one-to-many.test.tspackages/sdk/package.json
📚 Learning: kysely-based orm: v3 uses kysely as query builder instead of prisma runtime dependency...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.149Z
Learning: Kysely-Based ORM: V3 uses Kysely as query builder instead of Prisma runtime dependency
Applied to files:
packages/dialects/sql.js/test/getting-started/types.tspackages/dialects/sql.js/package.jsonREADME.mdpackages/dialects/sql.js/src/connection.tspackages/dialects/sql.js/README.mdpackages/dialects/sql.js/test/getting-started/database.tspackages/dialects/sql.js/src/driver.ts
📚 Learning: applies to tests/e2e/** : e2e tests are in `tests/e2e/` directory...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.148Z
Learning: Applies to tests/e2e/** : E2E tests are in `tests/e2e/` directory
Applied to files:
packages/dialects/sql.js/test/getting-started/person-repository.test.tstests/e2e/prisma-consistency/datasource.test.tspackages/cli/package.jsontests/e2e/package.jsontests/e2e/vitest.config.tspackages/language/package.jsontests/e2e/prisma-consistency/relations-one-to-many.test.ts
📚 Learning: no runtime dependency on @prisma/client...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.149Z
Learning: No runtime dependency on @prisma/client
Applied to files:
tests/e2e/prisma-consistency/datasource.test.tspackages/cli/src/actions/generate.tspackages/runtime/package.jsonpackages/cli/package.jsonREADME.mdtests/e2e/prisma-consistency/relations-one-to-many.test.ts
📚 Learning: e2e tests validate real-world schema compatibility (cal.com, formbricks, trigger.dev)...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.149Z
Learning: E2E tests validate real-world schema compatibility (cal.com, formbricks, trigger.dev)
Applied to files:
tests/e2e/prisma-consistency/datasource.test.tstests/e2e/package.jsonTODO.mdtests/e2e/prisma-consistency/relations-one-to-many.test.ts
📚 Learning: type coverage tests ensure typescript inference works correctly...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.149Z
Learning: Type coverage tests ensure TypeScript inference works correctly
Applied to files:
tests/e2e/prisma-consistency/datasource.test.tspackages/runtime/package.jsonpackages/dialects/sql.js/tsconfig.jsontests/e2e/prisma-consistency/relations-one-to-many.test.ts
📚 Learning: database migrations still use prisma cli under the hood...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.148Z
Learning: Database migrations still use Prisma CLI under the hood
Applied to files:
tests/e2e/prisma-consistency/datasource.test.tsTODO.mdREADME.md
📚 Learning: schema is used to instantiate `zenstackclient` with type-safe crud operations...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.149Z
Learning: Schema is used to instantiate `ZenStackClient` with type-safe CRUD operations
Applied to files:
packages/cli/src/actions/generate.tsREADME.md
📚 Learning: schema-first approach with zmodel dsl extension of prisma schema language...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.149Z
Learning: Schema-first approach with ZModel DSL extension of Prisma schema language
Applied to files:
packages/cli/src/actions/generate.tsTODO.mdREADME.mdpackages/language/package.json
📚 Learning: applies to **/schema.zmodel : always run `zenstack generate` after modifying zmodel schemas...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.148Z
Learning: Applies to **/schema.zmodel : Always run `zenstack generate` after modifying ZModel schemas
Applied to files:
packages/cli/src/actions/generate.tsREADME.md
📚 Learning: monorepo structure: uses pnpm workspaces with turbo for build orchestration...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.149Z
Learning: Monorepo Structure: Uses pnpm workspaces with Turbo for build orchestration
Applied to files:
packages/runtime/package.jsonpackages/cli/package.jsonpackages/language/package.json
📚 Learning: language-first design: zmodel dsl compiles to typescript, not runtime code generation...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.149Z
Learning: Language-First Design: ZModel DSL compiles to TypeScript, not runtime code generation
Applied to files:
TODO.mdpackages/cli/src/index.tsREADME.mdpackages/language/package.json
📚 Learning: zmodel schema (`schema.zmodel`) defines database structure and policies...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.149Z
Learning: ZModel schema (`schema.zmodel`) defines database structure and policies
Applied to files:
TODO.mdREADME.mdpackages/language/package.json
📚 Learning: client provides both high-level orm api and low-level kysely query builder...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.149Z
Learning: Client provides both high-level ORM API and low-level Kysely query builder
Applied to files:
README.mdpackages/dialects/sql.js/README.mdpackages/dialects/sql.js/test/getting-started/database.ts
📚 Learning: kysely query builder as escape hatch instead of raw sql...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.149Z
Learning: Kysely query builder as escape hatch instead of raw SQL
Applied to files:
README.mdpackages/dialects/sql.js/README.md
📚 Learning: plugin architecture: runtime plugins for query interception and entity mutation hooks...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.149Z
Learning: Plugin Architecture: Runtime plugins for query interception and entity mutation hooks
Applied to files:
README.md
📚 Learning: plugin system allows interception at orm, kysely, and entity mutation levels...
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.148Z
Learning: Plugin system allows interception at ORM, Kysely, and entity mutation levels
Applied to files:
README.md
🧬 Code Graph Analysis (8)
packages/dialects/sql.js/test/getting-started/person-repository.test.ts (1)
packages/dialects/sql.js/test/getting-started/database.ts (1)
db(10-12)
packages/dialects/sql.js/src/types.ts (1)
packages/dialects/sql.js/test/getting-started/types.ts (1)
Database(3-6)
packages/cli/src/index.ts (1)
packages/cli/src/cli-error.ts (1)
CliError(4-4)
packages/dialects/sql.js/test/getting-started/person-repository.ts (2)
packages/dialects/sql.js/test/getting-started/database.ts (1)
db(10-12)packages/dialects/sql.js/test/getting-started/types.ts (3)
Person(40-40)PersonUpdate(42-42)NewPerson(41-41)
packages/language/src/validators/datasource-validator.ts (2)
packages/language/src/utils.ts (1)
getStringLiteral(72-74)packages/language/src/generated/ast.ts (1)
isInvocationExpr(542-544)
packages/sdk/src/ts-schema-generator.ts (2)
packages/language/src/generated/ast.ts (4)
Model(559-563)Model(565-565)isDataSource(417-419)isLiteralExpr(133-135)packages/common-helpers/src/tiny-invariant.ts (1)
invariant(4-14)
packages/cli/src/actions/action-utils.ts (1)
packages/cli/src/cli-error.ts (1)
CliError(4-4)
packages/dialects/sql.js/test/getting-started/database.ts (2)
packages/dialects/sql.js/test/getting-started/types.ts (1)
Database(3-6)packages/dialects/sql.js/src/dialect.ts (1)
SqlJsDialect(12-26)
🪛 markdownlint-cli2 (0.17.2)
TODO.md
13-13: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
80-80: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
packages/dialects/sql.js/README.md
1-1: Bare URL used
(MD034, no-bare-urls)
🪛 LanguageTool
README.md
[style] ~136-~136: Consider shortening or rephrasing this to strengthen your wording.
Context: ...run the "generate" command whenever you make changes to the ZModel schema. ## Creating ZenStac...
(MAKE_CHANGES)
🪛 Biome (2.1.2)
packages/dialects/sql.js/src/connection.ts
[error] 27-29: This generator function doesn't contain yield.
(lint/correctness/useYield)
⏰ 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). (2)
- GitHub Check: build-test (20.x)
- GitHub Check: claude-review
🔇 Additional comments (60)
packages/sdk/package.json (1)
3-3: Version bump looks correct and consistent with the mono-repo release.
No additional adjustments needed.packages/eslint-config/package.json (1)
3-3: Package version updated in line with the coordinated release – LGTM.packages/testtools/package.json (1)
3-3: Test-tools package follows the alpha.17 release tag – looks good.packages/common-helpers/package.json (1)
3-3: Common-helpers version bump aligns with the rest of the packages – no issues.packages/ide/vscode/package.json (1)
4-4: VSCode extension version moved to alpha.17 as expected – all good.samples/blog/package.json (1)
3-3: Version bump looks goodPackage version advanced to
3.0.0-alpha.17, matching the workspace release cycle.
No other metadata touched — zero risk. ✅package.json (1)
3-3: Root version updated consistentlyThe monorepo root now reflects
3.0.0-alpha.17, keeping it in sync with package versions.
No further action required.packages/zod/package.json (1)
3-3: Library version bump aligns with release cadence
@zenstackhq/zodmoves to3.0.0-alpha.17.
Exports, peer-deps, and scripts remain unchanged — safe change.packages/tanstack-query/package.json (1)
3-3: Version increment only
@zenstackhq/tanstack-queryupdated to3.0.0-alpha.17; no collateral changes.
LGTM.packages/create-zenstack/package.json (1)
3-3: Version bump acknowledged
create-zenstacknow at3.0.0-alpha.17. Everything else intact.
Ship it.packages/typescript-config/package.json (1)
3-3: Version bump aligns with workspace release cadence
Package version updated to3.0.0-alpha.17; no other metadata changes. Looks good and keeps this package consistent with the rest of the monorepo.packages/runtime/package.json (2)
3-3: Runtime package version bump acknowledged
The increment to3.0.0-alpha.17is consistent with the workspace-wide release.
99-100: Dev-only dependency correctly added
Including@zenstackhq/vitest-configunderdevDependenciesscopes the new shared Vitest config to test/build time only and avoids polluting the runtime footprint.packages/cli/package.json (2)
6-7: Version bump looks fine
3.0.0-alpha.17correctly reflects a prerelease increment and aligns with other workspace packages.
No further action needed.
52-53: Ensure workspace dependency is published
@zenstackhq/vitest-configis referenced viaworkspace:*.
Confirm that the package is part of the workspace root and will be published so external consumers resolving the CLI outside the monorepo won’t get an unmet peer error.packages/language/vitest.config.ts (1)
1-4: Centralised Vitest config import LGTMSwitching to
@zenstackhq/vitest-config/basesimplifies maintenance.
Confirm@zenstackhq/vitest-configis listed in this package’sdevDependencies(it should match the CLI change).tests/e2e/vitest.config.ts (1)
1-4: Consistent base config import acknowledgedImport path updated to the shared package — looks good.
packages/language/package.json (1)
65-65: 👍 Centralised Vitest config dependency looks correctAdding
@zenstackhq/vitest-configas a workspace-scoped devDependency is consistent with the repo-wide move to a shared Vitest base. No issues spotted here.tests/e2e/prisma-consistency/relations-one-to-many.test.ts (1)
1-8: Import tidy-up LGTMRemoval of the unused
expectimport and re-ordering aligns with the updated helpers. No functional impact.tests/e2e/prisma-consistency/datasource.test.ts (1)
1-2: Import list correctly prunedUnused imports were dropped, keeping the test file clean. Looks good.
packages/cli/vitest.config.ts (1)
1-4: Switched to shared Vitest base – good moveImporting the base config from
@zenstackhq/vitest-configunifies test setup across packages. Ensure theclipackage’spackage.jsonalready lists this devDependency (it appears to be added elsewhere in the PR).tests/e2e/package.json (1)
5-5: ESM validation passed for tests/e2eA search for CommonJS
require(in all.js/.tsfiles undertests/e2e/returned no matches, confirming that the"type": "module"setting intests/e2e/package.jsonwon’t break your E2E tests.packages/runtime/vitest.config.ts (1)
1-3: Adoption of centralised Vitest base config looks goodImporting the shared config from
@zenstackhq/vitest-configeliminates path gymnastics and keeps packages in sync.
LGTM.packages/dialects/sql.js/src/index.ts (1)
1-4: Barrel export is succinct and clearClean re-exports make consumer imports ergonomic.
packages/dialects/sql.js/eslint.config.js (1)
1-4: LGTM! Standard ESLint configuration setup.The configuration follows the established pattern used across the codebase, importing the base ESLint config with proper JSDoc typing for IDE support.
packages/language/src/validators/datasource-validator.ts (1)
44-55: Confirm URL field optionality change
ThevalidateUrlmethod now returns early when nourlfield is found (lines 44–47), effectively making theurlfield optional. Previously, a missingurlwould have been flagged as an error.• packages/language/src/validators/datasource-validator.ts:
– validateUrl (lines 44–47)Please verify that skipping URL validation when the field is absent is an intentional behavioral change.
packages/cli/src/actions/action-utils.ts (1)
42-42: Improved error handling with clear messaging.Good improvement using
CliErrorwith a descriptive message that references the detailed errors printed above. This enhances the user experience by providing clear, actionable feedback.packages/dialects/sql.js/vitest.config.ts (1)
1-4: LGTM! Proper use of centralized Vitest configuration.The configuration correctly uses the centralized base config with the standard merge pattern. This promotes consistency across packages and simplifies maintenance.
packages/dialects/sql.js/test/getting-started/database.ts (3)
1-7: LGTM! Clean import structure.The imports are well-organized with proper separation of types and implementation imports.
10-12: LGTM! Proper database configuration for testing.The Kysely instance is correctly configured with proper TypeScript generics and an in-memory SQL.js database, which is ideal for testing scenarios.
8-8: Ensure ES module & TS config support top-level awaitWe’ve confirmed in packages/dialects/sql.js/package.json that
"type": "module". The local tsconfig.json extends@zenstackhq/typescript-config/base.jsonbut doesn’t overridemodule/target.Please verify in your base config (node_modules/@zenstackhq/typescript-config/base.json) that:
compilerOptions.moduleis set to"ES2022"/"ESNext"compilerOptions.targetis set to at least"ES2022"This ensures top-level await works as expected.
packages/language/src/index.ts (1)
189-195: LGTM! Improved datasource validation logic.The enhanced validation correctly ensures exactly one datasource declaration exists by checking for both zero and multiple datasources. The error messages are clear and consistently formatted.
packages/dialects/sql.js/test/getting-started/person-repository.test.ts (3)
1-5: LGTM! Appropriate test imports.All necessary dependencies are imported correctly for the test suite.
7-36: LGTM! Well-structured test lifecycle.The test setup properly creates the table schema, seeds data before each test, and cleans up appropriately. The table schema with constraints and default values is realistic for testing purposes.
38-94: LGTM! Comprehensive CRUD test coverage.The test suite thoroughly covers all repository operations with appropriate assertions and realistic test data. The tests properly validate expected behavior for create, read, update, and delete operations.
packages/dialects/sql.js/package.json (4)
1-17: LGTM! Proper package metadata and configuration.The package follows naming conventions, has a clear description, and is properly configured as an ES module with appropriate build scripts.
18-29: LGTM! Proper dual package exports configuration.The exports field correctly provides both ESM and CommonJS entry points with corresponding type definitions, ensuring broad compatibility.
30-37: LGTM! Appropriate development dependencies.The dev dependencies include necessary type definitions and shared workspace configurations for consistent tooling across the project.
38-42: Approve: Peer dependencies and workspace catalog mapping validated
- packages/dialects/sql.js/package.json correctly declares
•"sql.js": "^1.13.0"
•"kysely": "catalog:"- pnpm-workspace.yaml defines a
catalog.kysely: ^0.27.6, matching the intended workspace configurationNo further changes needed.
packages/sdk/src/ts-schema-generator.ts (1)
562-570: Simplification of getDataSourceProvider confirmed safeNo code in the SDK or elsewhere relies on the previously‐removed
urlorenvproperties. All call sites—both ints-schema-generator.tsand in the language validator—only access the returnedtype(or use their own validator’s string return), so this change poses no breaking impact.packages/cli/src/index.ts (3)
3-3: LGTM: Import addition for enhanced error handling.The addition of
CommanderErrorimport supports the improved error handling strategy.
5-5: LGTM: Import addition for custom CLI errors.The
CliErrorimport enables proper handling of custom CLI errors as referenced in the relevant code snippets.
130-142: Excellent error handling implementation.The async conversion and comprehensive error handling strategy properly addresses different error scenarios:
CliError: Custom CLI errors with user-friendly red outputCommanderError: Built-in Commander errors that are already reported- Unexpected errors: Generic fallback with full error details
The use of appropriate exit codes (1 for errors,
err.exitCodefor Commander errors) follows CLI conventions.packages/dialects/sql.js/src/dialect.ts (3)
9-11: Good documentation of usage constraints.The comment clearly indicates this dialect is for testing purposes only, which is important for preventing production misuse.
12-17: LGTM: Clean dialect class structure.The class properly implements the Dialect interface with appropriate config dependency injection.
19-25: LGTM: Proper factory method implementations.The factory methods correctly instantiate SQLite components, which is appropriate for SQL.js since it's SQLite-based. The arrow function syntax is consistent and clean.
packages/dialects/sql.js/src/connection.ts (2)
6-11: LGTM: Clean connection wrapper.The class properly wraps the SQL.js Database instance with appropriate constructor injection.
13-24: LGTM: Correct query execution and result mapping.The
executeQueryimplementation correctly:
- Executes SQL with parameters using SQL.js API
- Retrieves affected row count
- Maps SQL.js column/value format to Kysely's expected object format
- Returns proper QueryResult structure
packages/dialects/sql.js/src/driver.ts (4)
8-13: LGTM: Clean driver class structure.The class properly implements the Driver interface with appropriate config dependency injection.
15-17: LGTM: Proper connection acquisition.The method correctly creates a new SqlJsConnection instance using the configured SQL.js database.
19-29: LGTM: Standard transaction management.The transaction methods correctly use standard SQL commands:
BEGINfor starting transactionsCOMMITfor committing changesROLLBACKfor reverting changesUsing
CompiledQuery.raw()is the appropriate way to execute these commands.
31-37: LGTM: Proper resource management.The
destroymethod properly closes the SQL.js instance, and the no-op implementations forinitandreleaseConnectionare appropriate for SQL.js which doesn't require connection pooling.packages/dialects/sql.js/test/getting-started/types.ts (4)
1-6: LGTM: Proper Kysely type imports and database interface.The imports include all necessary Kysely type utilities, and the Database interface correctly defines the table structure.
8-32: Excellent type definitions with comprehensive documentation.The PersonTable interface demonstrates proper usage of Kysely type utilities:
Generated<number>for auto-incrementing ID- Union types for constrained values (gender)
- Nullable types with
| null(not optional properties)ColumnTypefor operation-specific typing (created_at)The documentation comments provide valuable guidance on best practices.
34-42: LGTM: Proper type aliases following Kysely patterns.The type aliases using
Selectable,Insertable, andUpdateablewrappers follow Kysely best practices and provide the correct types for different database operations.
44-53: LGTM: Consistent type definitions for PetTable.The PetTable interface and associated type aliases follow the same patterns as PersonTable, maintaining consistency across the schema.
packages/dialects/sql.js/test/getting-started/person-repository.ts (4)
1-3: LGTM!Clean import structure with proper type-only imports and clear separation of concerns.
5-7: LGTM!Clean implementation using Kysely's fluent API. The use of
executeTakeFirst()appropriately returns undefined when no match is found.
39-41: LGTM!Correct implementation that properly returns the inserted record. The use of
executeTakeFirstOrThrow()is appropriate since we expect exactly one insertion.
46-48: LGTM!Well-implemented delete function that returns the deleted record. The use of
executeTakeFirst()appropriately handles cases where no matching record exists.
|
Claude finished @ymc9's task —— View job Pull Request ReviewReviewing this dev to main merge with focus on code quality, potential issues, performance, security, and test coverage. Todo List:
Review SummaryThis PR introduces significant improvements including a new sql.js dialect, unified vitest configuration, and version bumps. The changes are well-structured and follow the project conventions outlined in CLAUDE.md. Key Changes Reviewed✅ New SQL.js Dialect Package (
|
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores