merge dev to main (v3.0.0-alpha.25) #174
Conversation
* fix: error when selecting relation that contains computed fields * update
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
* feat: add "migrate resolve" command to CLI * Update packages/cli/src/index.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * add test * update test --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
WalkthroughIntroduces a new CLI subcommand migrate resolve with validation and tests, updates runtime field reference handling with an inlineComputedField control, removes SQL context comments, revises computed-fields tests, updates build scripts to include TypeScript type-checking, adjusts multiple tsconfig settings (remove outDir/add baseUrl/rootDir), bumps versions, and tweaks README Postgres install instructions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Claude finished @ymc9's task —— View job Pull Request Review - dev to mainReviewing this PR that merges dev branch to main with 4 commits and 40 files changed. Review Tasks:
Review SummaryThis PR introduces several important changes including a new ✅ Positive Findings1. New
2. Version Consistency
3. Test Coverage
4. Code Quality
|
There was a problem hiding this comment.
Pull Request Overview
This PR merges the dev branch into main for version 3.0.0-alpha.25, focusing on package version bumps, TypeScript configuration standardization, and improvements to computed fields and CLI migration functionality.
- Version bump across all packages from 3.0.0-alpha.24 to 3.0.0-alpha.25
- Standardization of TypeScript configurations to use
baseUrlinstead ofoutDir - Enhanced computed fields functionality with better handling of relation queries and inlining control
- Addition of new CLI migration
resolvecommand for handling migration conflicts
Reviewed Changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| package.json | Root package version bump |
| tests/e2e/package.json | Version bump for e2e tests |
| samples/blog/package.json | Version bump for blog sample |
| packages/*/package.json | Version bumps and build script improvements across all packages |
| packages/*/tsconfig.json | TypeScript config standardization with baseUrl |
| packages/runtime/tsconfig.build.json | New build-specific TypeScript config |
| packages/runtime/test/client-api/computed-fields.test.ts | Enhanced test coverage for computed fields |
| packages/runtime/src/client/query-utils.ts | Added inlineComputedField parameter support |
| packages/runtime/src/client/crud/operations/base.ts | Disabled context comments in queries |
| packages/runtime/src/client/crud/dialects/*.ts | Updated to use new inlineComputedField parameter |
| packages/language/src/validators/typedef-validator.ts | Type definition improvement |
| packages/cli/src/*.ts | Added new migrate resolve command |
| README.md | Removed pg-connection-string dependency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or 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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/create-zenstack/tsconfig.json (1)
4-4: Same baseUrl considerations apply here.Ensure you don’t rely on absolute imports unless your bundler resolves them. If desired, add "paths": { "src/": ["src/"] } and verify tsup/esbuild picks it up.
You can reuse the import scan, adjusting the glob:
#!/bin/bash rg -nP -C1 --type=ts -g 'packages/create-zenstack/**' '\bfrom\s+["'\''`](src/[^"'\''`]+)["'\''`]|import\(["'\''`](src/[^"'\''`]+)["'\''`]\)'packages/zod/tsconfig.json (1)
4-4: baseUrl added — verify import patterns and bundler behavior.Echoing the earlier note: baseUrl alone won’t change runtime resolution. Confirm no 'src/...' imports or add paths + bundler support if you want them.
Scan this package similarly:
#!/bin/bash rg -nP -C1 --type=ts -g 'packages/zod/**' '\bfrom\s+["'\''`](src/[^"'\''`]+)["'\''`]|import\(["'\''`](src/[^"'\''`]+)["'\''`]\)'
🧹 Nitpick comments (19)
packages/common-helpers/package.json (1)
7-7: Nice: add tsc --noEmit before bundling.Good safeguard to fail builds on type errors. Optional: expose a reusable typecheck script and call it from build, which plays nicer with Turbo’s caching and CI logs.
Apply this diff to add a dedicated typecheck script and reuse it:
"scripts": { - "build": "tsc --noEmit && tsup-node", + "build": "pnpm typecheck && tsup-node", + "typecheck": "tsc --noEmit", "watch": "tsup-node --watch", "lint": "eslint src --ext ts", "pack": "pnpm pack" },samples/blog/package.json (1)
6-11: Optional: expose new CLI drift-resolve in sample scriptsGiven the new migrate resolve subcommand in the CLI, consider adding a convenience script so users can resolve migration drift from the sample.
Apply this diff:
"scripts": { "generate": "zenstack generate", "db:migrate": "zenstack migrate dev", + "db:resolve": "zenstack migrate resolve", "build": "pnpm generate && tsc --noEmit", "dev": "tsx main.ts" },README.md (1)
112-114: Good simplification: pg-connection-string is unnecessary for Pool connectionString.pg parses DATABASE_URL natively, so removing pg-connection-string is correct. Consider adding a short note showing a sample DATABASE_URL (including SSL flags if applicable) and reminding users to bring in dotenv (or similar) locally.
packages/cli/package.json (1)
25-25: Pre-build type-check added — consider type-checking in watch mode too."tsc --noEmit && tsup-node" strengthens CI/builds. For local dev, watch doesn’t type-check, so errors surface only at build/test time. If helpful, add a typecheck watcher in parallel to tsup.
Example (platform-dependent ampersand usage; adjust to your tooling):
- "watch": "tsup-node --watch", + "watch": "tsc --noEmit --watch & tsup-node --watch",If you prefer cross-platform, wire in a dev runner (e.g., npm-run-all or concurrently) and run both in parallel.
packages/sdk/package.json (2)
7-8: Pre-type-check before bundling: good move.Adding
tsc --noEmitbeforetsup-nodewill catch type issues earlier. Consider mirroring this in a watch flow if you rely on watch builds for local dev (optional).
40-46: Move TypeScript to devDependencies.TypeScript is a build-time tool; keeping it in runtime dependencies bloats downstream installs.
"dependencies": { "@zenstackhq/language": "workspace:*", "@zenstackhq/common-helpers": "workspace:*", "langium": "catalog:", "ts-pattern": "catalog:", - "typescript": "catalog:" + // moved to devDependencies }, "devDependencies": { "@zenstackhq/eslint-config": "workspace:*", "@zenstackhq/typescript-config": "workspace:*", "decimal.js": "^10.4.3", - "kysely": "catalog:" + "kysely": "catalog:", + "typescript": "catalog:" }packages/runtime/src/client/crud/operations/base.ts (3)
484-491: Remove stale commented-out error handlingThese commented try/catch blocks add noise and drift. Please delete them or reintroduce them properly.
- // let createdEntity: any; - // try { - // createdEntity = await this.executeQueryTakeFirst(kysely, query, 'create'); - // } catch (err) { - // const { sql, parameters } = query.compile(); - // throw new QueryError(`Error during create: ${err}, sql: ${sql}, parameters: ${parameters}`); - // }
1167-1175: Remove stale commented-out error handlingSame here; prefer deleting commented code to keep the file clean.
- // try { - // updatedEntity = await this.executeQueryTakeFirst(kysely, query, 'update'); - // } catch (err) { - // const { sql, parameters } = query.compile(); - // throw new QueryError( - // `Error during update: ${err}, sql: ${sql}, parameters: ${parameters}` - // ); - // }
1306-1309: SQL context comments disabled — confirm no downstream reliance & consider feature-flaggingmakeContextComment currently returns an empty SQL fragment. I checked the repo: CONTEXT_COMMENT_PREFIX is defined but only referenced in the commented-out line in this file — there are no other internal consumers of that prefix.
Findings:
- packages/runtime/src/client/constants.ts — CONTEXT_COMMENT_PREFIX = '-- $$context:' (only definition).
- packages/runtime/src/client/crud/operations/base.ts
- makeContextComment definition (no-op) at ~line 1306.
- Multiple call sites that append the fragment via .modifyEnd(this.makeContextComment(...)) (e.g. lines 139, 214, 410, 476, 891, 1159, 1404, 1666, 1682, 1704, 1822, 1842, 1921, 1943, 2114).
- Search for CONTEXT_COMMENT_PREFIX found only the constant and the commented-out usage — no other code/tests parse or depend on the comment text.
Recommendation (keep default no-op; emit sanitized comment when debug is enabled):
If you want to preserve observability/tracing, re-enable a minimal, sanitized comment behind a client option (debug). Example patch:
private makeContextComment(_context: { model: GetModels; operation: CRUD }) {
return sql``;// return sql.raw(`${CONTEXT_COMMENT_PREFIX}${JSON.stringify(context)}`);}
- private makeContextComment(context: { model: GetModels; operation: CRUD }) {
// Default no-op to avoid leaking metadata into SQL. Emit sanitized comment when debug is on.if (this.options?.debug) {const safe = { model: context.model, operation: context.operation };return sql.raw(`${CONTEXT_COMMENT_PREFIX}${safeJSONStringify(safe)}`);}return sql``;- }
Notes:
- safeJSONStringify is already available in this file's imports — prefer it over JSON.stringify to avoid injection/syntax issues.
- this.options?.debug is already referenced elsewhere in this class, so adding this check is consistent with existing behavior.
- I cannot verify external/downstream consumers outside the repo; please confirm there are no external systems relying on embedded SQL comments before removing them entirely.
packages/cli/src/actions/migrate.ts (2)
119-124: Quote user-supplied flags to avoid shell injection and spacing issuesCurrently, applied/rolledBack values are concatenated directly into a shell string. Quote them to be safe and to support names with spaces.
Apply this diff:
const cmd = [ 'prisma migrate resolve', ` --schema "${prismaSchemaFile}"`, - options.applied ? ` --applied ${options.applied}` : '', - options.rolledBack ? ` --rolled-back ${options.rolledBack}` : '', + options.applied ? ` --applied "${options.applied}"` : '', + options.rolledBack ? ` --rolled-back "${options.rolledBack}"` : '', ].join('');
3-3: Deduplicate handleSubProcessError: import shared helper instead of redefiningThis function already exists in action-utils. Keeping a single source avoids divergence.
Apply this diff:
-import { generateTempPrismaSchema, getSchemaFile } from './action-utils'; +import { generateTempPrismaSchema, getSchemaFile, handleSubProcessError } from './action-utils'; @@ -function handleSubProcessError(err: unknown) { - if (err instanceof Error && 'status' in err && typeof err.status === 'number') { - process.exit(err.status); - } else { - process.exit(1); - } -} +// use handleSubProcessError from './action-utils'Also applies to: 132-138
packages/runtime/src/client/crud/dialects/sqlite.ts (1)
173-175: Avoid inlining computed fields for nested relation JSON is the right callPassing inlineComputedField=false prevents referencing base-table columns inside the subquery context. One minor improvement: explicitly use the subquery alias for clarity and to prevent ambiguity if the selection expands later.
Apply this diff:
- .map(([field]) => [sql.lit(field), this.fieldRef(relationModel, field, eb, undefined, false)]) + .map(([field]) => [ + sql.lit(field), + this.fieldRef(relationModel, field, eb, subQueryName, false), + ]) @@ - return [ - sql.lit(field), - this.fieldRef(relationModel, field, eb, undefined, false) as ArgsType, - ]; + return [ + sql.lit(field), + this.fieldRef(relationModel, field, eb, subQueryName, false) as ArgsType, + ];Also applies to: 202-205
packages/cli/test/migrate.test.ts (2)
42-66: Harden the test: assert migration discovery and the intended deploy failureTwo small robustness tweaks:
- Assert the migration folder scan actually finds the “_init” migration before proceeding.
- Instead of swallowing the deploy error, assert it throws to prove the failure path is exercised.
Apply this diff:
const migrationRecords = fs.readdirSync(path.join(workDir, 'zenstack/migrations')); - const migration = migrationRecords.find((f) => f.endsWith('_init')); + const migration = migrationRecords.find((f) => f.endsWith('_init')); + expect(migration).toBeDefined(); // force a migration failure fs.writeFileSync(path.join(workDir, 'zenstack/migrations', migration!, 'migration.sql'), 'invalid content'); // redeploy the migration, which will fail fs.rmSync(path.join(workDir, 'zenstack/dev.db'), { force: true }); - try { - runCli('migrate deploy', workDir); - } catch { - // noop - } + expect(() => runCli('migrate deploy', workDir)).toThrow(); // --rolled-back runCli(`migrate resolve --rolled-back ${migration}`, workDir); // --applied runCli(`migrate resolve --applied ${migration}`, workDir);
68-71: Optional: assert error message for missing flags to make the failure more specificAsserting on the message (or a substring/regex) helps detect regressions where a different error is thrown.
For example:
- expect(() => runCli('migrate resolve', workDir)).toThrow(); + expect(() => runCli('migrate resolve', workDir)).toThrow(/(--applied|--rolled-back)/);packages/runtime/test/client-api/computed-fields.test.ts (3)
6-14: Avoid cross-provider DB name collisionsWhen tests run with multiple providers, using the same dbName can cause flakiness (especially for PostgreSQL where create/drop happens per test). Prefer namespacing by provider.
Apply this diff to define a provider-specific DB name:
describe.each([{ provider: 'sqlite' as const }, { provider: 'postgresql' as const }])( 'Computed fields tests', ({ provider }) => { let db: any; + const dbName = `${TEST_DB}-${provider}`;Then replace occurrences of
dbName: TEST_DBin this block todbName(see additional changes below).Additional changes (outside the selected range):
- Replace
dbName: TEST_DBwithdbNamein options at lines 26, 116, 158, 187, and 236.
1-1: Strengthen typing for computedFields lambdasUsing any hides type errors. You can keep tests lightweight while still benefiting from Kysely’s ExpressionBuilder types.
Apply these changes:
-import { afterEach, describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it } from 'vitest'; +import type { ExpressionBuilder } from 'kysely';- upperName: (eb: any) => eb.fn('upper', ['name']), + upperName: (eb: ExpressionBuilder<any, any>) => eb.fn('upper', ['name']),- upperName: (eb: any) => eb.lit(null), + upperName: (eb: ExpressionBuilder<any, any>) => eb.lit(null),- postCount: (eb: any) => + postCount: (eb: ExpressionBuilder<any, any>) => eb .selectFrom('Post') .whereRef('Post.authorId', '=', 'User.id') .select(() => eb.fn.countAll().as('count')),Also applies to: 29-30, 161-162, 239-244
217-257: Relation-derived computed field test is solidVerifies a subquery-based computed field reading from a relation and ensures the value flows through nested selection. Consider also adding a filter assertion (e.g., where: { postCount: 1 }) in a follow-up to cover predicate translation for computed fields.
packages/runtime/src/client/query-utils.ts (1)
170-181: Tighten the computed function type and add a brief doc for clarity
- Use a precise function type instead of Function.
- Optional: document inlineComputedField to clarify behavior for future maintainers.
Apply this diff:
- let computer: Function | undefined; + // Function that returns an expression for the computed field + let computer: + | ((eb: ExpressionBuilder<any, any>) => ExpressionWrapper<any, any, unknown>) + | undefined;Optional doc (outside selected range, above inlineComputedField parameter in the signature):
/** * @param inlineComputedField * - true: inline the computed expression via options.computedFields * - false: reference the field as a column (assumes it has been selected/aliased upstream) */packages/runtime/src/client/crud/dialects/postgresql.ts (1)
229-231: Correctly avoid inlining computed fields when projecting relation JSONPassing undefined, false to fieldRef ensures we reference the preselected alias instead of inlining the computed expression again. The cast on Object.entries reduces TS friction for payload.select.
If you want to avoid any usage, consider:
- ...Object.entries<any>(payload.select) + ...Object.entries(payload.select as Record<string, unknown>)This keeps type unsafety localized without resorting to any.
Also applies to: 235-236, 252-253
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (40)
README.md(1 hunks)package.json(1 hunks)packages/cli/package.json(2 hunks)packages/cli/src/actions/migrate.ts(4 hunks)packages/cli/src/index.ts(1 hunks)packages/cli/test/migrate.test.ts(1 hunks)packages/cli/tsconfig.json(1 hunks)packages/common-helpers/package.json(1 hunks)packages/common-helpers/tsconfig.json(1 hunks)packages/create-zenstack/package.json(1 hunks)packages/create-zenstack/tsconfig.json(1 hunks)packages/dialects/sql.js/package.json(1 hunks)packages/dialects/sql.js/tsconfig.json(1 hunks)packages/eslint-config/package.json(1 hunks)packages/ide/vscode/package.json(1 hunks)packages/ide/vscode/tsconfig.json(1 hunks)packages/language/package.json(2 hunks)packages/language/src/validators/typedef-validator.ts(2 hunks)packages/language/tsconfig.json(1 hunks)packages/runtime/package.json(1 hunks)packages/runtime/src/client/crud/dialects/base.ts(1 hunks)packages/runtime/src/client/crud/dialects/postgresql.ts(2 hunks)packages/runtime/src/client/crud/dialects/sqlite.ts(2 hunks)packages/runtime/src/client/crud/operations/base.ts(2 hunks)packages/runtime/src/client/query-utils.ts(1 hunks)packages/runtime/test/client-api/computed-fields.test.ts(3 hunks)packages/runtime/tsconfig.build.json(1 hunks)packages/runtime/tsconfig.json(1 hunks)packages/sdk/package.json(1 hunks)packages/sdk/tsconfig.json(1 hunks)packages/tanstack-query/package.json(1 hunks)packages/tanstack-query/tsconfig.json(1 hunks)packages/testtools/package.json(1 hunks)packages/testtools/tsconfig.json(1 hunks)packages/typescript-config/package.json(1 hunks)packages/vitest-config/package.json(1 hunks)packages/zod/package.json(1 hunks)packages/zod/tsconfig.json(1 hunks)samples/blog/package.json(1 hunks)tests/e2e/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{packages,samples,tests}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/eslint-config/package.jsonpackages/ide/vscode/package.jsonpackages/vitest-config/package.jsonpackages/common-helpers/package.jsonpackages/sdk/tsconfig.jsonpackages/runtime/tsconfig.build.jsonpackages/cli/tsconfig.jsonpackages/create-zenstack/tsconfig.jsonpackages/cli/package.jsonsamples/blog/package.jsonpackages/testtools/package.jsonpackages/zod/package.jsonpackages/zod/tsconfig.jsonpackages/typescript-config/package.jsonpackages/dialects/sql.js/package.jsonpackages/sdk/package.jsonpackages/dialects/sql.js/tsconfig.jsonpackages/language/tsconfig.jsonpackages/cli/test/migrate.test.tspackages/common-helpers/tsconfig.jsonpackages/runtime/src/client/crud/dialects/base.tspackages/tanstack-query/package.jsonpackages/cli/src/index.tspackages/create-zenstack/package.jsonpackages/testtools/tsconfig.jsontests/e2e/package.jsonpackages/language/package.jsonpackages/cli/src/actions/migrate.tspackages/language/src/validators/typedef-validator.tspackages/tanstack-query/tsconfig.jsonpackages/runtime/src/client/crud/operations/base.tspackages/runtime/src/client/query-utils.tspackages/runtime/tsconfig.jsonpackages/runtime/src/client/crud/dialects/sqlite.tspackages/ide/vscode/tsconfig.jsonpackages/runtime/package.jsonpackages/runtime/src/client/crud/dialects/postgresql.tspackages/runtime/test/client-api/computed-fields.test.ts
tests/e2e/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
E2E tests are in
tests/e2e/directory
Files:
tests/e2e/package.json
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: Database migrations still use Prisma CLI under the hood
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: `zenstack generate` compiles ZModel to TypeScript schema (`schema.ts`)
Applied to files:
packages/cli/package.jsonpackages/zod/package.jsonpackages/tanstack-query/package.jsonpackages/create-zenstack/package.jsonpackages/language/package.json
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: Database migrations still use Prisma CLI under the hood
Applied to files:
packages/cli/src/index.tspackages/cli/src/actions/migrate.ts
🧬 Code Graph Analysis (8)
packages/cli/test/migrate.test.ts (1)
packages/cli/test/utils.ts (2)
createProject(12-18)runCli(20-23)
packages/runtime/src/client/crud/dialects/base.ts (1)
packages/runtime/src/client/query-utils.ts (1)
buildFieldRef(157-183)
packages/cli/src/index.ts (1)
packages/runtime/src/client/crud/operations/base.ts (1)
options(92-94)
packages/cli/src/actions/migrate.ts (3)
packages/cli/src/cli-error.ts (1)
CliError(4-4)packages/cli/src/utils/exec-utils.ts (1)
execPackage(20-26)packages/cli/src/actions/action-utils.ts (1)
handleSubProcessError(50-56)
packages/language/src/validators/typedef-validator.ts (1)
packages/language/src/generated/ast.ts (2)
DataField(333-340)DataField(342-342)
packages/runtime/src/client/crud/operations/base.ts (2)
packages/sdk/src/schema/schema.ts (1)
GetModels(108-108)packages/runtime/src/client/contract.ts (1)
CRUD(214-214)
packages/runtime/src/client/query-utils.ts (2)
packages/runtime/src/client/crud/operations/base.ts (2)
requireField(119-121)schema(88-90)samples/blog/zenstack/schema.ts (1)
schema(9-228)
packages/runtime/test/client-api/computed-fields.test.ts (1)
packages/runtime/test/utils.ts (1)
createTestClient(78-195)
⏰ 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 (39)
packages/language/src/validators/typedef-validator.ts (1)
24-26: Approve — validateField passes DataFieldAttribute and validator accepts itVerified: validateAttributeApplication accepts AttributeApplication which includes DataFieldAttribute, and DataField.attributes is Array<DataFieldAttribute — the call in validateField is correct.
Files checked:
- packages/language/src/validators/typedef-validator.ts — validateField calls validateAttributeApplication(attr, accept).
- packages/language/src/validators/attribute-application-validator.ts — type AttributeApplication = DataModelAttribute | DataFieldAttribute | InternalAttribute and export function validateAttributeApplication(...).
- packages/language/src/generated/ast.ts — DataField.attributes: Array<DataFieldAttribute and DataFieldAttribute interface present.
package.json (1)
3-3: Approve: version bump to 3.0.0-alpha.25 — no leftovers foundRan the provided scan: no "3.0.0-alpha.24" occurrences found; 17 package.json files contain "3.0.0-alpha.25".
- No files require changes based on this check.
Updated snippet:
"version": "3.0.0-alpha.25",packages/vitest-config/package.json (1)
4-4: LGTM: version bump only.No functional/package export changes here.
packages/common-helpers/package.json (1)
3-3: LGTM: package version bumped to 3.0.0-alpha.25.Aligned with the workspace release.
tests/e2e/package.json (1)
3-3: LGTM: e2e package version bump.No other behavioral changes in test runner config.
packages/ide/vscode/package.json (1)
4-4: LGTM: VSCode extension version bump.Build/publish scripts remain consistent with pre-release flow.
packages/typescript-config/package.json (1)
3-3: Version bump looks goodPackage version aligned to alpha.25. No other concerns in this file.
packages/runtime/tsconfig.build.json (1)
1-7: New type-check-only build config: LGTM (verified)Confirmed the runtime build script uses the new config:
- packages/runtime/package.json (line ~7): "build": "tsc --project tsconfig.build.json --noEmit && tsup-node && pnpm test:generate"
samples/blog/package.json (1)
3-3: Sample version bump alignedVersion updated to alpha.25; scripts remain compatible with the repo’s new type-check-before-bundle pattern.
packages/ide/vscode/tsconfig.json (1)
4-4: baseUrl "." is OK — tsup bundles workspace deps; no change requiredBriefly: the non-relative imports in the VSCode extension are package-scoped (e.g. @zenstackhq/language, langium, vscode-languageserver) and tsup is configured to bundle workspace dependencies, so resolution does not depend on tsconfig baseUrl.
Files inspected:
- packages/ide/vscode/tsconfig.json — "baseUrl": "."
- packages/ide/vscode/src/language-server/main.ts — imports:
- import { createZModelLanguageServices } from '@zenstackhq/language';
- import { startLanguageServer } from 'langium/lsp';
- import { NodeFileSystem } from 'langium/node';
- import { createConnection, ProposedFeatures } from 'vscode-languageserver/node.js';
- packages/ide/vscode/tsup.config.ts — bundler config includes:
- noExternal: [/^(?!vscode$)/]
- external: ['vscode']
Conclusion: no action needed.
packages/eslint-config/package.json (1)
3-3: Version bump LGTMScoped, non-functional change. No issues.
packages/zod/package.json (2)
3-3: Version bump LGTMConsistent with the repo-wide release bump.
9-9: Confirmed: tsup emits declaration files for packages/zod (dts: true)tsup config in packages/zod/tsup.config.ts sets dts: true, so the build script "tsc --noEmit && tsup-node" will produce .d.ts/.d.cts — no change required.
Files checked:
- packages/zod/package.json — scripts.build: tsc --noEmit && tsup-node
- packages/zod/tsup.config.ts — contains
dts: true(match at line 11)- packages/zod/package.json — no package-level
tsupfield- packages/zod/tsconfig.json — no
compilerOptions.declaration(tsc is intentionally --noEmit)The suggested inline change (
tsup-node --dts) is unnecessary given the existing tsup config.packages/testtools/tsconfig.json (1)
4-4: baseUrl="." — OK; no local absolute imports found in packages/testtoolsQuick summary: packages/testtools/tsconfig.json sets compilerOptions.baseUrl = "." and has no paths mappings. I scanned for non-relative imports and found only external packages and Node builtins — no local absolute imports (e.g., "src/..." or similar) that would be affected.
Non-relative imports found:
- packages/testtools/tsup.config.ts: import { defineConfig } from 'tsup';
- packages/testtools/eslint.config.js: import config from '@zenstackhq/eslint-config/base.js';
- packages/testtools/src/schema.ts:
- import { TsSchemaGenerator } from '@zenstackhq/sdk';
- import type { SchemaDef } from '@zenstackhq/sdk/schema';
- import { glob } from 'glob';
- import { execSync } from 'node:child_process';
- import fs from 'node:fs';
- import path from 'node:path';
- import { match } from 'ts-pattern';
- packages/testtools/src/project.ts:
- import fs from 'node:fs';
- import path from 'node:path';
- import tmp from 'tmp';
Recommendation (optional): if you intend to use absolute imports for local code, add explicit "paths" mappings for clarity; otherwise no changes required here.
packages/sdk/tsconfig.json (1)
4-4: Confirmed: baseUrl="." in packages/sdk/tsconfig.json — no local absolute imports found, no action required.Checked packages/sdk/tsconfig.json (compilerOptions.baseUrl = ".") and scanned package sources for non-relative imports that look like local absolute paths — none were found.
- packages/sdk/tsconfig.json — compilerOptions.baseUrl: "."
- Scan result — No suspicious non-relative local imports in packages/sdk
packages/language/package.json (2)
4-4: Version bump LGTMMatches the monorepo update cadence.
13-13: No action needed — tsup already emits declarationstsup is configured to produce declaration files and the build order includes generation + type-checking before bundling:
- packages/language/package.json — build: "pnpm langium:generate && tsc --noEmit && tsup-node"
- packages/language/tsup.config.ts — dts: true, format: ['esm','cjs'], outDir: 'dist' (so tsup will emit .d.ts and the CommonJS .d.cts)
- packages/language/tsconfig.json — includes "src/**/*.ts" (generated Langium files under src/ are covered), so tsc --noEmit will type-check the generated output
Remove the suggested change (adding --dts) — redundant because tsup.config.ts already enables dts.
packages/cli/tsconfig.json (1)
4-4: No 'src/...' absolute imports found in packages/cli — baseUrl "." is safe as-isQuick check results:
- packages/cli/tsconfig.json contains "baseUrl": "."
- No source files import from "src/..." (search returned only tsconfig include and tsup.config.ts index: 'src/index.ts' — a build entry, not a runtime import).
- Non-relative imports found are package or node builtins (e.g., 'colors', 'commander', 'vitest/config').
Conclusion: No action required for this PR. If you do intend to use absolute imports from the repo root (e.g., import from "src/..."), add an explicit "paths" mapping (e.g., "src/": ["src/"]) and ensure your bundler (tsup/esbuild) is configured to respect tsconfig paths.
packages/cli/package.json (1)
6-6: Version bump looks good and aligned with the repo-wide update.No issues spotted with the version increment.
packages/runtime/tsconfig.json (1)
4-6: No action required — build uses tsconfig.build.json that excludes testsConfirmed: packages/runtime/tsconfig.build.json has "include": ["src/**/*"], and the package.json build script runs
tsc --project tsconfig.build.json --noEmit, so tests in packages/runtime/tsconfig.json are not type‑checked during the build.Files checked:
- packages/runtime/tsconfig.build.json — include: ["src/**/*"]
- packages/runtime/package.json — "build": "tsc --project tsconfig.build.json --noEmit && tsup-node && pnpm test:generate"
You can keep the test entries in packages/runtime/tsconfig.json for IDE/test-time checking; no build change required.
packages/common-helpers/tsconfig.json (1)
4-5: No action needed — baseUrl-only is safe for packages/common-helpersScanned the package for non-relative/internal absolute imports. Found only external/scoped packages:
- packages/common-helpers/tsup.config.ts — import { defineConfig } from 'tsup';
- packages/common-helpers/eslint.config.js — import config from '@zenstackhq/eslint-config/base.js';
No internal absolute imports (e.g., "src/...") were detected, so leaving "baseUrl": "." in this package's tsconfig is fine. If you later add project-root absolute imports, add appropriate "paths" and ensure the bundler resolves them.
packages/create-zenstack/package.json (2)
7-8: Type-check before bundling: looks good.The build pipeline becomes safer with
tsc --noEmitin front. No further concerns here.
14-19: Confirmed: bin/cli is included in the published tarballRan
npm pack --json --dry-runin packages/create-zenstack — the generated file list includesbin/cli(andpackage.json), so the CLI will be published as expected.
- Verified location: packages/create-zenstack/package.json (bin → "create-zenstack": "bin/cli")
packages/tanstack-query/package.json (1)
9-10: LGTM on adding pre-bundle type-check.This makes failures surface earlier without affecting the bundle output.
packages/tanstack-query/tsconfig.json (1)
4-4: No local non-relative imports found — no tsup path tweaks neededI scanned packages/tanstack-query for bare/non-relative local imports (e.g., "src/...") and found none. The only non-relative import encountered is the external 'tsup' import in packages/tanstack-query/tsup.config.ts. That tsup config uses an entry file 'src/react.ts' (a file path for the bundler), and tsup is present and configured (dts:true, formats: cjs/esm).
Files checked:
- packages/tanstack-query/* (source & test files) — no bare local imports found
- packages/tanstack-query/tsup.config.ts — entry: react: 'src/react.ts'
Conclusion: the added "baseUrl": "." in tsconfig.json does not currently require extra tsup/esbuild path resolution. If you later introduce baseUrl-dependent imports, configure tsup/esbuild to resolve tsconfig paths (e.g., tsconfig-paths or esbuild aliases) or use relative imports.
packages/language/tsconfig.json (1)
4-4: baseUrl OK — no action requiredQuick summary: I checked packages/language/tsconfig.json, the package's tsup config, and scanned packages/language/src for non-relative local imports that would be affected by baseUrl.
Findings:
- packages/language/tsconfig.json contains:
"baseUrl": "."- packages/language/tsup.config.ts exists and uses tsup (no custom path aliases; entries: index, ast, utils).
- Scanned packages/language/src — found 35 non-relative imports, but they are external modules or workspace package imports (e.g., 'langium', 'node:fs', '@zenstackhq/common-helpers'). No local bare/absolute imports like "src/…" or package-local bare specifiers that baseUrl would change.
Conclusion: The introduced baseUrl does not create immediate issues for this package given current imports and tsup setup. No changes required.
packages/testtools/package.json (2)
3-3: Version bump looks goodNo issues with the alpha bump.
7-7: tsup already generates .d.ts — no change requiredtsup is configured to emit declarations (packages/testtools/tsup.config.ts contains
dts: trueandoutDir: 'dist'), so the current build script ("tsc --noEmit && tsup-node") will produce the declaration files into ./dist.Files checked:
- packages/testtools/tsup.config.ts —
dts: true,outDir: 'dist'- packages/testtools/package.json —
build: "tsc --noEmit && tsup-node"packages/runtime/src/client/crud/operations/base.ts (1)
26-26: Import of NUMERIC_FIELD_TYPES is appropriateUsed solely by isNumericField; no issues spotted.
packages/dialects/sql.js/package.json (2)
3-3: Version bump looks goodNo issues with the alpha bump.
7-7: Declarations already emitted — no change requiredTsup is configured to emit declarations (packages/dialects/sql.js/tsup.config.ts contains
dts: true), so tsup will generate .d.ts files and the build script can remain:
- packages/dialects/sql.js/tsup.config.ts — dts: true
- packages/dialects/sql.js/package.json — "build": "tsc --noEmit && tsup-node"
Do not apply the suggested diff adding
--dts.packages/runtime/package.json (2)
3-3: Version bump looks goodAligns with the repo-wide alpha.25 update mentioned in the PR summary.
7-7: Pre-bundle type-check step: OK — tsconfig.build.json present and includes sourcesQuick check: packages/runtime/tsconfig.build.json exists, extends "@zenstackhq/typescript-config/base.json", sets "rootDir": "." and includes "src/**/*" — the tsc --noEmit gate is valid.
- packages/runtime/package.json (build script): "build": "tsc --project tsconfig.build.json --noEmit && tsup-node && pnpm test:generate"
- packages/runtime/tsconfig.build.json: extends "@zenstackhq/typescript-config/base.json", "rootDir": ".", "include": ["src/**/*"]
packages/cli/src/actions/migrate.ts (2)
56-58: Wiring the 'resolve' subcommand into the dispatcher is correctThis integrates cleanly with the existing migrate flow and temp schema handling.
113-116: Good input validation for resolveFailing fast when neither --applied nor --rolled-back is provided improves UX and prevents confusing Prisma errors.
packages/cli/src/index.ts (1)
98-99: Minor doc polish is fineCapitalization tweak reads better. No functional change.
packages/runtime/src/client/crud/dialects/base.ts (1)
996-1004: fieldRef extension is consistent and backwards-compatibleAdding inlineComputedField with a default true and threading it to buildFieldRef aligns with the updated callers and maintains existing behavior.
packages/runtime/test/client-api/computed-fields.test.ts (1)
15-103: Great coverage for computed scalar field behavior across CRUD pathsCovers create/select/filter/orderBy/aggregate/groupBy, and negative take semantics. This gives good confidence in the new inlineComputedField handling.
packages/runtime/src/client/query-utils.ts (1)
157-165: Add param to control inlining computed fields: good callDefaulting to inline and allowing an opt-out solves double-evaluation and aliasing issues when selecting nested relation payloads.
| migrateCommand | ||
| .command('resolve') | ||
| .addOption(schemaOption) | ||
| .addOption(migrationsOption) | ||
| .addOption(new Option('--applied <migration>', 'record a specific migration as applied')) | ||
| .addOption(new Option('--rolled-back <migration>', 'record a specific migration as rolled back')) | ||
| .description('Resolve issues with database migrations in deployment databases') | ||
| .action((options) => migrateAction('resolve', options)); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Resolve subcommand wiring LGTM; confirm whether both flags can be used together
The action accepts either or both of --applied and --rolled-back. Prisma typically allows multiple resolve operations in one call, but please confirm this is intended and supported to avoid surprising UX; otherwise, consider marking the options as conflicting in Commander.
🌐 Web query:
Does Prisma CLI support using both --applied and --rolled-back simultaneously with `prisma migrate resolve`?
💡 Result:
Short answer: No — you cannot use both at the same time. prisma migrate resolve requires you to specify either --applied or --rolled-back (they are mutually exclusive). (prisma.io)
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locating files mentioning 'migrateCommand'..."
rg -n --hidden --no-ignore -S "migrateCommand" || true
echo
echo "Showing surrounding context for packages/cli/src/index.ts (if present):"
if [ -f packages/cli/src/index.ts ]; then
echo "----- packages/cli/src/index.ts -----"
nl -ba packages/cli/src/index.ts | sed -n '1,240p'
else
echo "packages/cli/src/index.ts not found"
fiLength of output: 987
Make --applied and --rolled-back mutually exclusive (fix required)
Prisma's CLI does not allow using both flags at once for migrate resolve — the command requires either --applied or --rolled-back, not both (prisma docs). Prevent both being selected in our CLI to avoid runtime errors.
- Location: packages/cli/src/index.ts — migrate resolve command (around line ~101).
- Action: enforce mutual exclusivity — either declare the options as conflicting via Commander or add a guard in the action handler that errors if both are passed.
Suggested minimal change:
migrateCommand
.command('resolve')
.addOption(schemaOption)
.addOption(migrationsOption)
.addOption(new Option('--applied <migration>', 'record a specific migration as applied'))
.addOption(new Option('--rolled-back <migration>', 'record a specific migration as rolled back'))
.description('Resolve issues with database migrations in deployment databases')
.action((options) => {
if (options.applied && options.rolledBack) {
throw new Error('Cannot use --applied and --rolled-back together. Choose one.');
}
migrateAction('resolve', options);
});
🤖 Prompt for AI Agents
In packages/cli/src/index.ts around lines 101 to 109, the migrate resolve
subcommand currently allows both --applied and --rolled-back to be passed which
Prisma forbids; enforce mutual exclusivity by either marking the two options as
conflicting on the Commander Option objects (use Option.prototype.conflicts with
the other option name) or add a small guard in the .action handler that checks
if options.applied and options.rolledBack are both set and throws a clear error
(e.g., "Cannot use --applied and --rolled-back together. Choose one.") before
calling migrateAction.
| "baseUrl": "." | ||
| }, | ||
| "include": ["src/**/*", "test/**/*"] | ||
| "include": ["src/**/*"] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
baseUrl addition is fine; consider explicitly excluding build and tests
The narrowed include to src only is good for package builds. As a small hardening, you can explicitly exclude typical folders to avoid accidental type-checking if files land there in the future.
Apply this diff:
{
"extends": "@zenstackhq/typescript-config/base.json",
"compilerOptions": {
"baseUrl": "."
},
- "include": ["src/**/*"]
+ "include": ["src/**/*"],
+ "exclude": ["dist", "build", "coverage", "node_modules", "test", "tests"]
}Also ensure your test runner uses its own tsconfig if test files relied on the package tsconfig previously.
Run to confirm tests have an explicit tsconfig or separate config:
🏁 Script executed:
#!/bin/bash
# Look for a test-specific tsconfig under this package or repo-level config used by tests.
fd -a -i --type f '^tsconfig\.(test|spec)\.json$' | sed 's/^/found: /'
rg -n -C2 -g '!**/node_modules/**' -P '(vitest|jest).*config' Length of output: 6318
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- packages/dialects/sql.js/tsconfig.json ----"
sed -n '1,200p' packages/dialects/sql.js/tsconfig.json || echo "missing"
echo
echo "---- packages/dialects/sql.js/vitest.config.ts ----"
sed -n '1,200p' packages/dialects/sql.js/vitest.config.ts || echo "missing"
echo
echo "---- packages/vitest-config/base.config.js ----"
sed -n '1,200p' packages/vitest-config/base.config.js || echo "missing"
echo
echo "---- search for tsconfig.* in that package ----"
rg -n --hidden -g '!**/node_modules/**' 'tsconfig' packages/dialects/sql.js -S || true
echo
echo "---- all tsconfig.test/spec files in repo ----"
fd -a -i --type f '^tsconfig\.(test|spec)\.json$' | sed 's/^/found: /' || trueLength of output: 1534
baseUrl addition is fine; add explicit excludes and confirm test tsconfig
Short check: packages/dialects/sql.js/tsconfig.json currently limits "include" to "src//*", and the package's Vitest config merges the workspace base (which includes '/*.test.ts'). There is no package-level tsconfig.test.json (only found packages/runtime/tsconfig.test.json), so tests in this package will still pick up the package tsconfig unless you add a test-specific tsconfig.
Files to update / inspect:
- packages/dialects/sql.js/tsconfig.json — add explicit excludes.
- packages/dialects/sql.js/vitest.config.ts — currently merges base; add a package tsconfig.test.json or configure Vitest to use a separate tsconfig if you want tests to use a different TS setup.
Apply this diff:
{
"extends": "@zenstackhq/typescript-config/base.json",
"compilerOptions": {
"baseUrl": "."
},
- "include": ["src/**/*"]
+ "include": ["src/**/*"],
+ "exclude": ["dist", "build", "coverage", "node_modules", "test", "tests"]
}Note: excluding "test" / "tests" won't remove test files that live under src (src/**/*.test.ts). If you need that, create a package-level tsconfig.test.json or point Vitest to a different tsconfig.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "baseUrl": "." | |
| }, | |
| "include": ["src/**/*", "test/**/*"] | |
| "include": ["src/**/*"] | |
| { | |
| "extends": "@zenstackhq/typescript-config/base.json", | |
| "compilerOptions": { | |
| "baseUrl": "." | |
| }, | |
| "include": ["src/**/*"], | |
| "exclude": ["dist", "build", "coverage", "node_modules", "test", "tests"] | |
| } |
🤖 Prompt for AI Agents
In packages/dialects/sql.js/tsconfig.json around lines 4 to 6 the config only
sets "baseUrl" and "include": ["src/**/*"] which can let workspace-wide test
globs leak in; add an explicit "exclude" array (e.g. exclude node_modules, dist,
and any top-level test directories like "test" or "tests") to the package
tsconfig, and either add a package-level tsconfig.test.json (mirroring the
workspace test settings) or update packages/dialects/sql.js/vitest.config.ts to
point Vitest to a separate tsconfig so tests under src/**/*.test.ts are handled
by the intended test TS config rather than the package tsconfig.
| @@ -1,5 +1,5 @@ | |||
| import type { ValidationAcceptor } from 'langium'; | |||
| import type { TypeDef, TypeDefField } from '../generated/ast'; | |||
| import type { DataField, TypeDef } from '../generated/ast'; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Import switch to DataField is correct and consistent with generated AST
Matches the generated AST where DataField can be contained by TypeDef. Good alignment with the model changes.
Run this to confirm there are no straggling TypeDefField references and that TypeDef.fields is typed to DataField:
🏁 Script executed:
#!/bin/bash
set -e
# 1) Ensure no residual TypeDefField usages remain
rg -nP --type=ts -C2 '\bTypeDefField\b' || echo "OK: No TypeDefField references found."
# 2) Inspect TypeDef.fields type in the generated AST
# Expectation: fields: Array<DataField>
rg -nPU --type=ts -C3 '(?s)^\s*export\s+interface\s+TypeDef\b.*?^\s*fields\s*:\s*Array<[^>]+>' packages/language/src/generated/ast.ts || \
rg -nPU --type=ts -C3 '(?s)^\s*export\s+interface\s+TypeDef\b.*?^\s*fields\s*:\s*Array<[^>]+>' Length of output: 1240
Import to DataField is correct — one leftover 'TypeDefField' reference must be fixed
TypeDef.fields is already typed to Array in the generated AST, but there is at least one remaining reference to "TypeDefField" that should be updated.
Files to fix:
- packages/language/src/generated/ast.ts — TypeDef.fields: Array (lines ~725–733) — OK.
- packages/language/src/validators/attribute-application-validator.ts:422 —
case 'TypeDefField'— update this to the new name (e.g., 'DataField') or adjust the switch logic accordingly.
🤖 Prompt for AI Agents
In packages/language/src/validators/attribute-application-validator.ts around
line 422 there's a switch/case still matching the legacy string 'TypeDefField'
even though the AST now uses DataField; change the case label to 'DataField' (or
add a fall-through to handle both 'TypeDefField' and 'DataField' if backward
compatibility is required), and ensure any related type references or imports
align with the generated AST (the typedef-validator.ts import at line 2 already
imports DataField and TypeDef correctly).
Summary by CodeRabbit