-
-
Notifications
You must be signed in to change notification settings - Fork 12
merge dev to main (v3.0.0-beta.27) #460
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
* fix(orm): correcting timezone for Date returned in nested queries * Update packages/orm/src/client/crud/dialects/postgresql.ts Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
* The standard way to access request query parameters is through ctx.query, not request.searchParams. * request.searchParams can be utilized but generally elysia utilizes ctx.query, unfortunately, they are not synchronized but ctx.query is generally where you would derive additional fields vs. request.searchParams.
* fix(server): nuxt handler should import `setResponseStatus` from `h3` * chore(server): import H3Event as a type
Co-authored-by: ymc9 <[email protected]>
* chore(cli): suppress prisma update message * update
* test(bun): basic e2e tests * update CI * update * update * update
* test: add edge-runtime e2e * update * add cleanup * update * simplify typing
WalkthroughBumps many package versions to 3.0.0-beta.27, adds Bun and edge-runtime test runtimes and CI Bun setup, upgrades Vitest to v4, converts root package to ESM, introduces type/generator/validator changes, adjusts server adapter query handling, and adds multiple test/schema generation artifacts. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
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.27)I've completed a comprehensive review of this PR covering code quality, potential issues, performance, security, and test coverage. Review Tasks:
Review SummaryThis is a well-structured PR merging dev to main for the v3.0.0-beta.27 release. The changes demonstrate good engineering practices with comprehensive testing, proper version management, and thoughtful improvements. ✅ Strengths1. Comprehensive Runtime Testing
2. Robust Version Management
3. Language Validation Improvements
4. Database Compatibility Enhancements
5. Server Adapter Refinements
|
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 for the v3.0.0-beta.27 release. The release includes significant improvements to runtime compatibility, testing infrastructure, configuration management, and several bug fixes. Key highlights include adding support for edge-runtime and Bun environments, upgrading to Vitest 4.0.14, improving PostgreSQL date handling, and enhancing the developer experience with better CLI output management.
Key Changes:
- Added comprehensive test suites for edge-runtime and Bun JavaScript runtimes with dedicated configurations
- Upgraded Vitest from 3.2.4 to 4.0.14 and updated test configuration patterns from single-level (
packages/*) to recursive (packages/**) - Fixed PostgreSQL date handling for jsonb_build_object by adding 'Z' suffix to UTC timestamps
- Improved CLI experience by suppressing Prisma update messages and adding generator declaration warnings
Reviewed changes
Copilot reviewed 57 out of 59 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Updated project patterns to recursive matching for better monorepo support |
| turbo.json | Disabled test caching and added TEST_DB_PROVIDER environment variable passthrough |
| tests/runtimes/edge-runtime/* | New edge-runtime e2e test suite with PostgreSQL support |
| tests/runtimes/bun/* | New Bun runtime e2e test suite with sqlite/PostgreSQL support |
| tests/regression/tsconfig.json | Added 'node' types for better TypeScript compatibility |
| packages/testtools/src/client.ts | Exported TEST_PG_CONFIG and TEST_PG_URL for reuse in tests |
| packages/server/src/adapter/nuxt/handler.ts | Refactored imports to use h3's setResponseStatus directly |
| packages/server/src/adapter/elysia/handler.ts | Optimized to use Elysia's parsed query object instead of parsing URL |
| packages/orm/src/client/crud/dialects/postgresql.ts | Fixed date parsing for PostgreSQL jsonb_build_object output |
| packages/language/src/validator.ts | Added warning for unused generator declarations |
| packages/language/test/*.test.ts | Added tests for 'this' keyword resolution and relation optionality validation |
| packages/cli/src/utils/exec-utils.ts | Added PRISMA_HIDE_UPDATE_MESSAGE to suppress update notifications |
| scripts/bump-version.ts | Added ESM support with __dirname fallback using import.meta.url |
| package.json | Added "type": "module" and changed test script to use turbo |
| pnpm-workspace.yaml | Reorganized catalog entries alphabetically |
| pnpm-lock.yaml | Updated dependencies including vitest 4.0.14 and edge-runtime packages |
| .vscode/* | Updated to use vitest.explorer extension and increased maximumConfigs |
| .github/workflows/build-test.yml | Added Bun setup for CI/CD pipeline |
| tests/runtimes/edge-runtime/.DS_Store | macOS system file that should not be committed |
| Multiple package.json files | Version bumped from 3.0.0-beta.26 to 3.0.0-beta.27 |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
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: 2
🧹 Nitpick comments (8)
packages/testtools/src/client.ts (1)
28-36: LGTM! Clean refactoring for reusability.Exporting
TEST_PG_CONFIGandTEST_PG_URLcentralizes PostgreSQL configuration and enables reuse across test files, which aligns well with the PR's goal of supporting Bun and edge-runtime tests.Optional: Consider validating port parsing.
Line 30's
parseIntcould returnNaNifTEST_PG_PORTcontains invalid data. While this is unlikely in a controlled test environment, adding validation would make the code more defensive:- port: process.env['TEST_PG_PORT'] ? parseInt(process.env['TEST_PG_PORT']) : 5432, + port: process.env['TEST_PG_PORT'] ? parseInt(process.env['TEST_PG_PORT']) || 5432 : 5432,packages/cli/src/utils/exec-utils.ts (1)
46-52: Env wrapping for Prisma CLI looks good; only minor optional tweaksThe
_optionswrapper cleanly ensuresPRISMA_HIDE_UPDATE_MESSAGEis injected for both the resolved Prisma path and the npx/bunx fallback, while preserving any caller‑supplied env and lettingexecSyncstill merge withprocess.env. No correctness issues from what I can see.Two optional nits you might consider (non‑blocking):
- If you ever want to let callers override
PRISMA_HIDE_UPDATE_MESSAGE, you could set it only when it’s not already present instead of hard‑overriding:env: { ...options?.env, ...(options?.env?.PRISMA_HIDE_UPDATE_MESSAGE == null && { PRISMA_HIDE_UPDATE_MESSAGE: '1', }), }- In
execSync, you could avoid passingenvat all when there are no overrides (instead ofenv: undefined) for slightly clearer intent, though Node will treatundefinedas “use parent env” anyway.Please double‑check that you have tests (or a manual check) covering both branches of
execPrisma(resolved Prisma path vs npx/bunx fallback) to confirmPRISMA_HIDE_UPDATE_MESSAGEis present in each case.Also applies to: 56-56, 60-60
tests/runtimes/bun/tsconfig.json (1)
1-8: Bun tsconfig looks good; consider stricternoImplicitAnyConfig wiring (extend base + add
bun-types) looks consistent with the Bun runtime package. If the shared base is strict, overridingnoImplicitAnytofalseweakens type safety for these tests; consider keeping it strict unless Bun typings force this relaxation..github/workflows/build-test.yml (1)
54-58: Pin Bun version instead of usinglatestInstalling Bun via
oven-sh/setup-bun@v2is good, butbun-version: latestmakes CI non-deterministic and can introduce surprise breakage when Bun releases. Prefer pinning to a specific version (or at least a major range like'1.x') and updating intentionally.tests/runtimes/edge-runtime/scripts/generate.ts (1)
1-20: Modernizeglobimport and remove unnecessary async/awaitThe overall flow (find
*.zmodel, runnpx zen generatein each dir) looks fine. Two improvements:
- Use the ESM named export
globSyncinstead ofglob.sync():-import { glob } from 'glob'; +import { globSync } from 'glob'; - const zmodelFiles = [...glob.sync(path.resolve(dir, '../schemas/*.zmodel'))]; + const zmodelFiles = globSync(path.resolve(dir, '../schemas/*.zmodel'));
- Both
mainandgenerateare markedasyncbut only call synchronous APIs. Remove the unnecessary wrappers:-async function main() { +function main() { @@ - await generate(file); + generate(file); @@ -async function generate(schemaPath: string) { +function generate(schemaPath: string) {tests/runtimes/bun/scripts/generate.ts (1)
1-20: Align Bun generate script with modernglobv11 API and remove unnecessary async/awaitLogic is sound and mirrors the edge-runtime generator; however, the current code doesn't follow glob v11's recommended ESM patterns:
- Use the named export
globSyncinstead of the deprecatedglob.syncalias:-import { glob } from 'glob'; +import { globSync } from 'glob'; - const zmodelFiles = [...glob.sync(path.resolve(dir, '../schemas/*.zmodel'))]; + const zmodelFiles = globSync(path.resolve(dir, '../schemas/*.zmodel'));
- Remove unnecessary
async/awaitsince all operations are synchronous (execSyncis synchronous):-async function main() { +function main() { @@ - await generate(file); + generate(file); @@ -async function generate(schemaPath: string) { +function generate(schemaPath: string) {Note: These same improvements apply to other generate scripts in the codebase (edge-runtime, regression, e2e, tanstack-query) that currently use the deprecated
glob.syncpattern.tests/runtimes/edge-runtime/edge-runtime-e2e.test.ts (1)
10-14: Consider stronger typing for_db.Using
anytype for_dbloses type safety. Consider using the actual client type or at minimum a type that exposes$disconnect().- let _db: any; + let _db: ZenStackClient<typeof schema> | undefined; afterEach(async () => { await _db?.$disconnect(); });tests/runtimes/bun/bun-e2e.test.ts (1)
90-116: Consider adding error handling for database setup.The
createClientfunction performs several async operations that could fail (database creation, connection). If any step fails, resources may be left in an inconsistent state.async function createClient(provider: 'sqlite' | 'postgresql', dbName: string) { const _schema = clone(schema); let dialect: Dialect; if (provider === 'sqlite') { (_schema as any).provider.type = 'sqlite'; dialect = new BunSqliteDialect({ database: new Database(':memory:'), }); } else { (_schema as any).provider.type = 'postgresql'; const pgClient = new Client({ connectionString: TEST_PG_URL, }); - await pgClient.connect(); - await pgClient.query(`DROP DATABASE IF EXISTS "${dbName}"`); - await pgClient.query(`CREATE DATABASE "${dbName}"`); - await pgClient.end(); + try { + await pgClient.connect(); + await pgClient.query(`DROP DATABASE IF EXISTS "${dbName}"`); + await pgClient.query(`CREATE DATABASE "${dbName}"`); + } finally { + await pgClient.end(); + } dialect = new PostgresDialect({ pool: new Pool({ connectionString: `${TEST_PG_URL}/${dbName}`, }), }); } const db = new ZenStackClient(_schema, { dialect }); await db.$pushSchema(); return db; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltests/runtimes/edge-runtime/.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (57)
.github/workflows/build-test.yml(1 hunks).vscode/extensions.json(1 hunks).vscode/settings.json(1 hunks)package.json(2 hunks)packages/auth-adapters/better-auth/package.json(1 hunks)packages/cli/package.json(1 hunks)packages/cli/src/utils/exec-utils.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/language/src/validator.ts(3 hunks)packages/language/test/attribute-application.test.ts(1 hunks)packages/language/test/this-resolution.test.ts(1 hunks)packages/orm/package.json(1 hunks)packages/orm/src/client/crud/dialects/postgresql.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/server/src/adapter/elysia/handler.ts(1 hunks)packages/server/src/adapter/nuxt/handler.ts(1 hunks)packages/testtools/package.json(1 hunks)packages/testtools/src/client.ts(2 hunks)packages/zod/package.json(1 hunks)pnpm-workspace.yaml(1 hunks)samples/next.js/package.json(1 hunks)samples/orm/package.json(1 hunks)scripts/bump-version.ts(3 hunks)tests/e2e/package.json(1 hunks)tests/regression/package.json(2 hunks)tests/regression/test/issue-204/input.ts(2 hunks)tests/regression/test/issue-422/input.ts(4 hunks)tests/regression/tsconfig.json(1 hunks)tests/runtimes/bun/bun-e2e.test.ts(1 hunks)tests/runtimes/bun/package.json(1 hunks)tests/runtimes/bun/schemas/input.ts(1 hunks)tests/runtimes/bun/schemas/models.ts(1 hunks)tests/runtimes/bun/schemas/schema.ts(1 hunks)tests/runtimes/bun/schemas/schema.zmodel(1 hunks)tests/runtimes/bun/scripts/generate.ts(1 hunks)tests/runtimes/bun/tsconfig.json(1 hunks)tests/runtimes/edge-runtime/edge-runtime-e2e.test.ts(1 hunks)tests/runtimes/edge-runtime/package.json(1 hunks)tests/runtimes/edge-runtime/schemas/input.ts(1 hunks)tests/runtimes/edge-runtime/schemas/models.ts(1 hunks)tests/runtimes/edge-runtime/schemas/schema.ts(1 hunks)tests/runtimes/edge-runtime/schemas/schema.zmodel(1 hunks)tests/runtimes/edge-runtime/scripts/generate.ts(1 hunks)tests/runtimes/edge-runtime/tsconfig.json(1 hunks)tests/runtimes/edge-runtime/vitest.config.ts(1 hunks)turbo.json(1 hunks)vitest.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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/runtimes/bun/schemas/schema.zmodeltests/runtimes/edge-runtime/schemas/schema.zmodel
🧠 Learnings (14)
📚 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/src/validator.tstests/runtimes/edge-runtime/tsconfig.jsonpackages/schema/package.jsontests/runtimes/bun/schemas/models.tstests/runtimes/bun/scripts/generate.tstests/runtimes/edge-runtime/scripts/generate.tspackages/zod/package.jsontests/regression/package.jsontests/runtimes/bun/schemas/schema.zmodeltests/regression/tsconfig.jsontests/runtimes/bun/tsconfig.jsontests/runtimes/edge-runtime/schemas/models.tstests/runtimes/edge-runtime/schemas/schema.zmodeltests/runtimes/edge-runtime/schemas/schema.tstests/runtimes/bun/schemas/schema.tspackages/language/package.jsontests/runtimes/bun/schemas/input.tstests/runtimes/edge-runtime/schemas/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} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
packages/orm/package.jsontests/runtimes/bun/schemas/models.tstests/runtimes/edge-runtime/edge-runtime-e2e.test.tspackages/server/src/adapter/nuxt/handler.tstests/regression/package.jsontests/regression/test/issue-204/input.tstests/runtimes/bun/bun-e2e.test.tstests/runtimes/edge-runtime/schemas/models.tstests/regression/test/issue-422/input.tstests/runtimes/edge-runtime/schemas/schema.tstests/runtimes/bun/schemas/schema.tspnpm-workspace.yamltests/runtimes/bun/schemas/input.tstests/runtimes/edge-runtime/schemas/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/runtimes/bun/schemas/models.tspackages/server/src/adapter/nuxt/handler.tstests/regression/test/issue-204/input.tstests/runtimes/edge-runtime/schemas/models.tstests/regression/test/issue-422/input.tstests/runtimes/edge-runtime/schemas/schema.tstests/runtimes/bun/schemas/schema.tspnpm-workspace.yamltests/runtimes/bun/schemas/input.tstests/runtimes/edge-runtime/schemas/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.jsonpackages/testtools/src/client.tstests/runtimes/edge-runtime/edge-runtime-e2e.test.tstests/regression/package.jsontests/regression/test/issue-204/input.tstests/runtimes/bun/bun-e2e.test.tstests/regression/test/issue-422/input.tstests/runtimes/bun/schemas/input.tstests/runtimes/edge-runtime/schemas/input.ts
📚 Learning: 2025-10-21T16:04:56.292Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/crud/dialects/base-dialect.ts:745-747
Timestamp: 2025-10-21T16:04:56.292Z
Learning: In packages/runtime/src/client/crud/dialects/base-dialect.ts, it's intentional that buildCursorFilter applies default ordering (via makeDefaultOrderBy fallback) while buildOrderBy does not. This ensures cursor-based pagination always has stable ordering for correctness, while regular queries remain unordered unless explicitly specified. This design is to be consistent with Prisma's pagination requirements.
Applied to files:
packages/orm/src/client/crud/dialects/postgresql.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/runtimes/edge-runtime/tsconfig.jsontests/runtimes/bun/package.jsontests/runtimes/bun/schemas/models.tstests/runtimes/edge-runtime/edge-runtime-e2e.test.tstests/runtimes/edge-runtime/scripts/generate.tstests/runtimes/edge-runtime/package.jsonvitest.config.tspackages/language/test/this-resolution.test.tstests/runtimes/bun/bun-e2e.test.tstests/regression/tsconfig.jsontests/runtimes/bun/tsconfig.jsontests/runtimes/edge-runtime/schemas/models.tstests/runtimes/edge-runtime/schemas/schema.tstests/runtimes/bun/schemas/schema.tspackages/language/test/attribute-application.test.tstests/runtimes/bun/schemas/input.tstests/runtimes/edge-runtime/schemas/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/**/type*.{ts,tsx} : Ensure TypeScript inference and type coverage are validated through type coverage tests
Applied to files:
tests/runtimes/edge-runtime/tsconfig.jsontests/runtimes/bun/schemas/models.tstests/runtimes/bun/scripts/generate.tsvitest.config.tspackages/language/test/this-resolution.test.tstests/regression/tsconfig.jsontests/runtimes/bun/tsconfig.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 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/sdk/package.jsonpackages/ide/vscode/package.jsonscripts/bump-version.ts.vscode/extensions.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 : Always run `zenstack generate` after modifying ZModel schemas
Applied to files:
tests/runtimes/bun/scripts/generate.tstests/runtimes/edge-runtime/scripts/generate.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 Turbo for build orchestration and run `pnpm build`, `pnpm watch`, `pnpm lint`, and `pnpm test` for development tasks
Applied to files:
turbo.json.github/workflows/build-test.ymlpackage.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: Database migrations should use Prisma CLI under the hood via ZenStack commands
Applied to files:
packages/cli/src/utils/exec-utils.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/cli/**/*.test.{ts,tsx} : CLI package tests should focus on action-specific tests for each command
Applied to files:
tests/regression/tsconfig.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: Use `pnpm` with workspaces for package management, pinned to version `pnpm10.12.1`
Applied to files:
scripts/bump-version.tspnpm-workspace.yamlpackage.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:
tests/runtimes/edge-runtime/schemas/schema.tstests/runtimes/bun/schemas/schema.ts
🧬 Code graph analysis (9)
packages/language/src/validator.ts (1)
packages/language/src/generated/ast.ts (2)
GeneratorDecl(507-512)GeneratorDecl(514-514)
tests/runtimes/bun/schemas/models.ts (1)
tests/runtimes/edge-runtime/schemas/models.ts (2)
User(10-10)Post(11-11)
packages/testtools/src/client.ts (2)
packages/orm/src/client/crud/dialects/postgresql.ts (1)
provider(33-35)packages/orm/src/client/crud/dialects/sqlite.ts (1)
provider(27-29)
tests/runtimes/bun/scripts/generate.ts (1)
packages/cli/src/utils/exec-utils.ts (1)
execSync(7-16)
tests/runtimes/edge-runtime/edge-runtime-e2e.test.ts (4)
packages/plugins/policy/src/plugin.ts (1)
PolicyPlugin(6-29)packages/testtools/src/client.ts (1)
TEST_PG_URL(35-35)packages/orm/src/dialects/postgres.ts (1)
PostgresDialect(1-1)tests/runtimes/edge-runtime/schemas/schema.ts (1)
schema(103-103)
tests/runtimes/edge-runtime/scripts/generate.ts (1)
packages/cli/src/utils/exec-utils.ts (1)
execSync(7-16)
tests/runtimes/edge-runtime/schemas/models.ts (1)
tests/runtimes/bun/schemas/models.ts (2)
User(10-10)Post(11-11)
tests/runtimes/bun/schemas/schema.ts (2)
packages/schema/src/expression-utils.ts (1)
ExpressionUtils(19-123)packages/schema/src/schema.ts (1)
SchemaDef(11-19)
tests/runtimes/bun/schemas/input.ts (1)
tests/runtimes/edge-runtime/schemas/input.ts (40)
UserFindManyArgs(11-11)UserFindUniqueArgs(12-12)UserFindFirstArgs(13-13)UserCreateArgs(14-14)UserCreateManyArgs(15-15)UserCreateManyAndReturnArgs(16-16)UserUpdateArgs(17-17)UserUpdateManyArgs(18-18)UserUpdateManyAndReturnArgs(19-19)UserUpsertArgs(20-20)UserDeleteArgs(21-21)UserDeleteManyArgs(22-22)UserCountArgs(23-23)UserAggregateArgs(24-24)UserGroupByArgs(25-25)UserWhereInput(26-26)UserSelect(27-27)UserInclude(28-28)UserOmit(29-29)UserGetPayload(30-30)PostFindManyArgs(31-31)PostFindUniqueArgs(32-32)PostFindFirstArgs(33-33)PostCreateArgs(34-34)PostCreateManyArgs(35-35)PostCreateManyAndReturnArgs(36-36)PostUpdateArgs(37-37)PostUpdateManyArgs(38-38)PostUpdateManyAndReturnArgs(39-39)PostUpsertArgs(40-40)PostDeleteArgs(41-41)PostDeleteManyArgs(42-42)PostCountArgs(43-43)PostAggregateArgs(44-44)PostGroupByArgs(45-45)PostWhereInput(46-46)PostSelect(47-47)PostInclude(48-48)PostOmit(49-49)PostGetPayload(50-50)
⏰ 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 (54)
tests/regression/test/issue-204/input.ts (1)
9-9: LGTM!The addition of
ClientOptionsimport and the newOptionsgeneric parameter toFooGetPayloadfollows a consistent pattern. The default value$ClientOptions<$Schema>ensures backward compatibility while enabling optional client option customization.Also applies to: 30-30
tests/regression/test/issue-422/input.ts (1)
9-9: LGTM!The
ClientOptionsimport and theOptionsgeneric parameter are consistently applied to all three model payload types (SessionGetPayload,UserGetPayload,ProfileGetPayload). The pattern mirrors the changes inissue-204/input.ts, maintaining consistency across the generated type system.Also applies to: 30-30, 50-50, 70-70
packages/testtools/src/client.ts (1)
109-109: LGTM! Cleaner URL construction.Using the exported
TEST_PG_URLconstant simplifies the code and eliminates duplication.packages/server/src/adapter/nuxt/handler.ts (1)
3-11: Import consolidation from h3 is correct and safe.The refactoring consolidates imports into a single h3 import and adds explicit type imports. All exports are confirmed: h3 v1.15.4+ properly exports
setResponseStatus,H3Event, andEventHandlerRequest. The function signature used at line 28 (setResponseStatus(event, 500)) and the type usage at line 21 (H3Event<EventHandlerRequest>) match h3's documented API. No compatibility issues.packages/server/src/adapter/elysia/handler.ts (1)
28-28: Good simplification by using Elysia's built-in query parsing.Using
ctx.querydirectly is cleaner than manually reconstructing query parameters from URL searchParams. This refactoring reduces code complexity and relies on Elysia's idiomatic pattern for accessing query parameters.However, please verify that
ctx.queryprovides the same data structure as the previousObject.fromEntries(url.searchParams)approach, especially for edge cases:
- Multiple values for the same query parameter (arrays)
- URL encoding/decoding handling
- Empty or missing query parameters
- Null/undefined value handling
Ensure test coverage exists for these query parameter scenarios to confirm functional equivalence.
packages/orm/package.json (1)
3-3: Version bump to beta.27 is consistent across the monorepo.Routine version update from 3.0.0-beta.26 to 3.0.0-beta.27. No dependency changes or API modifications.
packages/clients/tanstack-query/package.json (1)
3-3: Version bump to beta.27.Routine version update. No dependency or functional changes.
packages/auth-adapters/better-auth/package.json (1)
3-3: Version bump to beta.27.Routine version update. No changes to dependencies or exports.
packages/create-zenstack/package.json (1)
3-3: Version bump to beta.27.Routine version update with no functional changes.
packages/server/package.json (1)
3-3: Version bump to beta.27.Routine version update. All framework exports and dependencies remain stable.
packages/zod/package.json (1)
3-3: Version bump to beta.27.Routine version update for private package. No functional changes.
packages/cli/package.json (1)
6-6: Version bump to beta.27.Routine version update. CLI bin exports and dependencies remain stable.
tests/regression/tsconfig.json (1)
5-5: Node.js type definitions are properly configured for expanded test runtime support.The addition of "node" to the types array is correct and necessary.
@types/nodeis already declared in devDependencies, and explicitly including it in the types compiler option ensures TypeScript properly resolves Node.js APIs (fs, process, path, etc.) when tests run in the Bun environment. The base config does not specify types, so this explicit inclusion in the regression test config is the right approach.packages/plugins/policy/package.json (1)
3-3: Version bump to 3.0.0-beta.27 — LGTM.Routine version increment with no changes to dependencies or exports.
packages/common-helpers/package.json (1)
3-3: Version bump to 3.0.0-beta.27 — LGTM.Routine version increment with no changes to dependencies or exports.
tests/e2e/package.json (1)
3-3: Version bump to 3.0.0-beta.27 — LGTM.Routine version increment with no changes to test scripts or dependencies.
packages/config/eslint-config/package.json (1)
3-3: Version bump to 3.0.0-beta.27 — LGTM.Config package version increment.
packages/language/package.json (1)
4-4: Version bump to 3.0.0-beta.27 — LGTM.Routine version increment with no changes to exports, scripts, or dependencies.
packages/config/typescript-config/package.json (1)
3-3: Version bump to 3.0.0-beta.27 — LGTM.Config package version increment.
packages/schema/package.json (1)
3-3: Version bump to 3.0.0-beta.27 — LGTM.Routine version increment with no changes to exports, scripts, or dependencies.
samples/orm/package.json (1)
3-3: Version bump to 3.0.0-beta.27 — LGTM.Sample project version increment with no changes to scripts or dependencies.
packages/config/vitest-config/package.json (1)
4-4: Version bump aligns with release objectives. This is a straightforward version increment for the beta.27 release with no functional changes.packages/testtools/package.json (1)
3-3: Version bump is consistent with the release. No functional changes introduced.packages/sdk/package.json (1)
3-3: Version bump is consistent with the release. No functional changes introduced.samples/next.js/package.json (1)
3-3: Version bump is consistent with the release. No functional changes introduced.packages/ide/vscode/package.json (1)
4-4: VSCode extension version maintained separately per Marketplace compliance.The VSCode extension correctly maintains independent versioning (3.0.14) separate from the monorepo beta version, aligning with VSCode Marketplace requirements.
tests/regression/package.json (1)
3-3: Version bump and test infrastructure improvements are well-scoped.The addition of
@zenstackhq/vitest-configaligns the regression tests with the Vitest 4 upgrade and shared test configuration, while@types/nodeprovides proper TypeScript support for Node.js APIs in the test environment. Both additions are appropriate for an ESM-based test package.Also applies to: 20-21
.vscode/extensions.json (1)
6-6: Extension recommendation updated to official Vitest explorer for Vitest 4 compatibility.Switching from the community
kingwl.vscode-vitest-runnerto the officialvitest.explorerensures better compatibility with Vitest 4 and its expanded test discovery capabilities. This recommendation improves the out-of-box developer experience without breaking existing setups (extensions.json is advisory only)..vscode/settings.json (1)
1-3: VSCode Vitest config cap aligns with expanded test discovery.This setting caps the number of Vitest configurations the VSCode extension tracks. With 11 vitest.config.ts files across packages/** and tests/** (root, 7 in packages, 3 in tests), a limit of 20 provides sufficient headroom while preventing performance issues.
packages/language/test/attribute-application.test.ts (1)
24-47: Nice negative test for relation/FK optionality mismatchThe schema and expected error pattern accurately capture the inconsistent
bar/barIdoptionality case and give good regression coverage for this validation rule. No changes needed here.scripts/bump-version.ts (1)
5-10: ESM/CJS-safe dirname shim looks correctUsing
_dirnamewithtypeof __dirname !== 'undefined' ? __dirname : path.dirname(fileURLToPath(import.meta.url))is a solid way to keep this script working in both CommonJS and ESM mode, and the updatedpath.resolve(_dirname, '../…')calls still resolve to the repo root files as before. Worth just re-running the script once under your intended runner (e.g.tsx/Node) to confirm everything behaves as expected.Also applies to: 29-29, 45-45, 49-49
packages/language/test/this-resolution.test.ts (1)
1-71: Good coverage forthisresolution semanticsBoth the failing and passing schemas nicely pin down that
thisshould always resolve to the containing model (Ahere), even through nested relation filters. The regex on the error message and the positiveloadSchemaassertion look correct; this should guard against regressions in resolver behavior.pnpm-workspace.yaml (1)
6-30: Catalog entries align with workspace usage; verify versionsThe catalog additions/updates (React/Next/Prisma/pg/zod/TypeScript/etc.) look structurally correct and consistent with the
"catalog:"references in package manifests. Please just make sure these versions match the ones you actually intend to target and are available in the registry, since mismatches here can be painful to debug.packages/language/src/validator.ts (1)
9-14: GeneratorDecl validation is correctly integratedImporting
GeneratorDecl, registering it inValidationChecks, and implementingcheckGeneratoras a warning is consistent with the existing validator pattern. The message clearly tells users thatgeneratoris unsupported without hard-failing their schema, which seems like the right balance.Also applies to: 33-43, 61-63
package.json (1)
3-7: Root metadata, ESM switch, and test runner upgrade look coherentThe version bump,
"type": "module"addition, move toturbo run test, and Vitest 4 upgrade are internally consistent with the rest of the PR (ESM-safe tooling, turbo orchestration). Please just verify thatpnpm test/CI runs pick up all projects correctly under turbo and that there are no lingering CommonJS-only scripts at the root that would be affected by the ESM default.Also applies to: 11-11, 35-35
tests/runtimes/edge-runtime/schemas/input.ts (1)
1-50: Edge-runtime model arg/payload aliases look consistent with ORM typesThe User/Post arg aliases and
GetPayloadgenerics correctly wrap the shared$Schemaand@zenstackhq/ormhelpers, mirroring the pattern used in other runtimes (e.g., Bun). The addedOptions extends $ClientOptions<$Schema>parameter forGetPayloadis wired through toSimplifiedModelResultas expected. No issues from a typing standpoint.tests/runtimes/edge-runtime/tsconfig.json (1)
1-6: Minimal TS config for edge-runtime tests is appropriateExtending the shared base config and setting
noEmit: trueis a clean setup for a test-only runtime package; compilation is typecheck-only and defers all common settings to the central config.tests/runtimes/edge-runtime/package.json (1)
1-26: Edge-runtime test package wiring looks consistentThe package metadata, build pipeline (
test:generate+test:typecheck), and dependency set look coherent with the rest of the workspace and the new edge-runtime test setup.vitest.config.ts (1)
5-5: Broader Vitest project globs look appropriateExpanding
projectsto['packages/**', 'tests/**']should ensure nested runtimes (e.g., Bun and edge-runtime) are picked up without manual registration; this aligns well with the new directory layout.tests/runtimes/bun/package.json (1)
1-27: Bun runtime test package is wired sensiblyScripts (
test:generate,test:typecheck,test: bun test) and dependencies (workspace ZenStack packages, Kysely,bun-types) fit the intended Bun E2E runtime. This should integrate cleanly with the new Bun setup step in CI.tests/runtimes/edge-runtime/vitest.config.ts (1)
1-11: Edge-runtime Vitest config matches the shared patternMerging the shared base config and overriding
test.environmentto'edge-runtime'is a clean way to specialize this runtime without duplicating settings.turbo.json (1)
17-22: Verify the change from^buildtobuildin test dependencies.The
dependsOnvalue changed from"^build"(dependencies' builds) to"build"(own package's build only). This means tests will wait for the current package's build but not for its dependencies' builds. If this is intentional for performance optimization, it assumes dependencies are already built via a prior pipeline step.The additions of
"cache": falseandglobalPassThroughEnv: ["TEST_DB_PROVIDER"]are appropriate for ensuring fresh test execution and environment variable passthrough for runtime-specific tests.tests/runtimes/edge-runtime/schemas/models.ts (1)
1-11: LGTM!Auto-generated model type definitions following the established pattern. The type aliases correctly expose
UserandPostusing the$ModelResultgeneric type with the schema.tests/runtimes/bun/schemas/models.ts (1)
1-11: LGTM!Auto-generated model type definitions consistent with the edge-runtime counterpart. The structure correctly follows the established pattern for generated schema files.
tests/runtimes/bun/schemas/schema.ts (1)
1-104: LGTM!Well-structured auto-generated schema definition. The schema correctly defines:
- SQLite provider for Bun runtime
- User and Post models with proper field definitions, relations, and attributes
- Access control policies using
ExpressionUtilsfor expression building- Branded type pattern for nominal typing
The
as const satisfies SchemaDefpattern ensures type safety while preserving literal types.tests/runtimes/edge-runtime/edge-runtime-e2e.test.ts (3)
46-81: LGTM!The policy enforcement test correctly validates:
- Anonymous users (no auth) see 0 posts due to the
@@denyrule- User 1 (the author) sees both posts
- User 2 sees only 1 published post
This aligns with the access control policies defined in the schema.
16-44: LGTM!The CRUD test comprehensively validates create, read, update, and delete operations with proper assertions. Good test isolation via fresh database creation.
84-101: Pool cleanup is properly handled through Kysely's destroy() method.The
$disconnect()method correctly callsawait this.kysely.destroy(), which properly closes the underlying Pool. Kysely'sPostgresDialectmanages the pool lifecycle, and whendestroy()is invoked, the pool connections are closed as expected. The test code at lines 84-101 is correctly structured—the Pool is instantiated and passed toPostgresDialect, and cleanup occurs automatically when$disconnect()is called in the afterEach hook.tests/runtimes/bun/bun-e2e.test.ts (2)
22-50: LGTM!The CRUD test comprehensively validates create, read (findUnique/findFirst), update, delete, and count operations with appropriate assertions.
52-87: LGTM!The policy enforcement test correctly validates:
- Anonymous users see 0 posts (due to
@@deny('all', auth() == null))- User 1 (author) sees 2 posts (all their own posts)
- User 2 sees 1 post (only the published one via
@@allow('read', published))This aligns well with the schema policies defined in
schema.zmodel.tests/runtimes/edge-runtime/schemas/schema.zmodel (1)
1-31: LGTM!The schema is well-structured for edge-runtime E2E testing with appropriate:
- PostgreSQL datasource for edge compatibility
- Policy plugin integration
- Access policies that enable testing of auth contexts (anonymous, owner, other authenticated users)
- Cascade delete for clean test teardown
tests/runtimes/bun/schemas/schema.zmodel (1)
1-31: LGTM!The schema correctly mirrors the edge-runtime schema with SQLite as the default provider. The Bun E2E tests dynamically override the provider type based on
TEST_DB_PROVIDER, making this a suitable default.tests/runtimes/edge-runtime/schemas/schema.ts (1)
1-104: LGTM!Auto-generated schema file correctly represents the ZModel definition with proper field types, relations, defaults, and access policy expressions.
tests/runtimes/bun/schemas/input.ts (1)
1-50: LGTM!Auto-generated type definitions file providing strongly-typed query and mutation argument types for User and Post models. The structure is consistent with the edge-runtime counterpart.
* fix(orm): more robust timezone adjustment for postgres * update
|
Claude finished @ymc9's task —— View job PR Review: merge dev to main (v3.0.0-beta.27)I've completed a comprehensive review of this PR covering code quality, potential issues, performance, security, and test coverage. Review Tasks:
Review SummaryThis is a well-structured PR merging dev to main for the v3.0.0-beta.27 release. The changes demonstrate good engineering practices with comprehensive testing, proper version management, and thoughtful improvements. ✅ Strengths1. Comprehensive Runtime Testing
2. Robust Version Management
3. Language Validation Improvements
4. Database Compatibility Enhancements
5. Server Adapter Refinements
|
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 (1)
packages/orm/src/client/crud/dialects/postgresql.ts (1)
30-31: Consider making the schema a static property.The
isoDateSchemadoesn't depend on instance state and will be identical across all instances. Making it static avoids recreating the Zod schema for eachPostgresCrudDialectinstance.export class PostgresCrudDialect<Schema extends SchemaDef> extends BaseCrudDialect<Schema> { - private isoDateSchema = z.iso.datetime({ local: true, offset: true }); + private static readonly isoDateSchema = z.iso.datetime({ local: true, offset: true });Then update the usage on line 116:
- if (this.isoDateSchema.safeParse(value).success) { + if (PostgresCrudDialect.isoDateSchema.safeParse(value).success) {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/orm/src/client/crud/dialects/postgresql.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-12-02T03:11:40.136Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 460
File: packages/orm/src/client/crud/dialects/postgresql.ts:109-112
Timestamp: 2025-12-02T03:11:40.136Z
Learning: In the PostgreSQL dialect for ZenStack ORM, DateTime fields are mapped to PostgreSQL's 'timestamp' type (without time zone), not 'timestamptz'. This means jsonb_build_object serializes them as ISO 8601 strings without timezone suffixes, requiring explicit 'Z' appending for UTC interpretation.
Applied to files:
packages/orm/src/client/crud/dialects/postgresql.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} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
packages/orm/src/client/crud/dialects/postgresql.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/src/client/crud/dialects/postgresql.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 **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
packages/orm/src/client/crud/dialects/postgresql.ts
📚 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/crud/dialects/postgresql.ts
⏰ 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: build-test (20.x, sqlite)
- GitHub Check: build-test (20.x, postgresql)
- GitHub Check: claude-review
🔇 Additional comments (2)
packages/orm/src/client/crud/dialects/postgresql.ts (2)
12-12: LGTM!The Zod import is correct and aligns with Zod 4's standalone ISO datetime schema support.
110-121: LGTM!The implementation is well-structured:
- Uses
safeParsefor non-throwing validation- Correctly handles both offset and non-offset ISO datetime strings
- Gracefully falls back to returning the original string for non-ISO formats
- Comment now accurately reflects the
timestamptype mapping (based on learnings from this PR)
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.