merge dev to main (v3.0.0-alpha.30) #198
Conversation
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
* chore: minor fixes to crud types * update
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughVersion bumps across packages; CLI Prisma plugin now accepts a Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 PR Review: Merge dev to mainI've completed a comprehensive review of this PR. Here are my findings: Review Tasks:
SummaryThis PR appears to be a release preparation merge from dev to main with minimal risk. The changes are primarily version bumps and minor fixes. Key Changes Analysis✅ Version ConsistencyAll packages correctly updated to ✅ CRUD Types Improvements (
|
There was a problem hiding this comment.
Pull Request Overview
This PR merges the dev branch to main, primarily updating version numbers across the monorepo from 3.0.0-alpha.29 to 3.0.0-alpha.30. It also includes dependency updates, test enhancements with new enum support, and improvements to the Prisma plugin output handling.
- Version bump across all packages from alpha.29 to alpha.30
- Updated dependencies including Prisma (downgrade to ^6.10.0) and TypeScript (upgrade to ^5.8.0)
- Enhanced test schemas with new Status enum and array field support
Reviewed Changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Root package version update to alpha.30 |
| pnpm-workspace.yaml | Dependency version updates for Prisma and TypeScript |
| packages/*/package.json | Version bumps across all packages to alpha.30 |
| packages/runtime/src/client/crud-types.ts | Type system improvements for array fields and select inputs |
| packages/runtime/test/schemas/typing/* | Test schema enhancements with Status enum and PostgreSQL migration |
| packages/cli/src/plugins/prisma.ts | Improved Prisma plugin output path handling |
| packages/cli/test/plugins/prisma-plugin.test.ts | Updated test for new Prisma output behavior |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/runtime/src/client/crud-types.ts (2)
249-255: Make ArrayFilter schema-aware so enum arrays use enum keys (not string)Introduce an ArrayScalar helper that resolves enum element types to keyof GetEnum<...> and use it throughout ArrayFilter. This preserves strict typing for enum[] fields (e.g., Status.ACTIVE).
+type ArrayScalar<Schema extends SchemaDef, T extends string> = + T extends GetEnums<Schema> ? keyof GetEnum<Schema, T> : MapBaseType<T>; - -type ArrayFilter<T extends string> = { - equals?: MapBaseType<T>[]; - has?: MapBaseType<T>; - hasEvery?: MapBaseType<T>[]; - hasSome?: MapBaseType<T>[]; - isEmpty?: boolean; -}; +type ArrayFilter<Schema extends SchemaDef, T extends string> = { + equals?: ArrayScalar<Schema, T>[]; + has?: ArrayScalar<Schema, T>; + hasEvery?: ArrayScalar<Schema, T>[]; + hasSome?: ArrayScalar<Schema, T>[]; + isEmpty?: boolean; +};
126-134: Exclude false/undefined keys from SelectCountResultWith SelectCount now allowing boolean, nested shapes like { select: { posts: false } } should not produce a posts number in the result type. Refine the mapping to drop keys set to false/undefined.
type SelectCountResult<Schema extends SchemaDef, Model extends GetModels<Schema>, C> = C extends true ? { // count all to-many relation fields [Key in RelationFields<Schema, Model> as FieldIsArray<Schema, Model, Key> extends true ? Key : never]: number; } : C extends { select: infer S } - ? { [Key in keyof S]: number } + ? { [Key in keyof S as S[Key] extends false | undefined ? never : Key]: number } : never;
🧹 Nitpick comments (8)
samples/blog/package.json (1)
3-3: Version bump OK; consider marking the sample as private to avoid accidental publishThe version bump is fine. Small safety improvement: mark the sample package as private so it’s never accidentally published.
Apply outside the selected range:
{ "name": "sample-blog", + "private": true, "version": "3.0.0-alpha.30",packages/tanstack-query/package.json (1)
3-3: Version bump approved; note on peer dependency.Package remains private; only version updated. Peer dependency stays at @tanstack/react-query ^5.0.0—no action needed unless you plan to support v6 soon.
If you want, add a short non-empty "description" to improve package metadata:
- "description": "", + "description": "ZenStack helpers for @tanstack/react-query integration",packages/sdk/package.json (1)
3-3: Version bump looks goodNo API/runtime impact here. Confirm the rest of the workspace is aligned to 3.0.0-alpha.30 and that a changeset/release note exists for the publish.
packages/zod/package.json (1)
3-3: Safe metadata-only bumpNothing else changed. Ensure the workspace publish plan includes this private package if applicable.
packages/runtime/test/schemas/typing/schema.zmodel (1)
11-16: New enum Status is consistent; consider documenting intended orderingEnum definition looks correct and matches downstream exports. If enum value order matters for any serialization/UI mapping, add a brief comment to lock expectations.
packages/cli/src/plugins/prisma.ts (1)
17-17: Prefer async fs.promises.writeFile in async generate()You’re already in an async context; switching to the non-blocking fs.promises API keeps execution consistent and avoids blocking the event loop.
Points to address:
• File: packages/cli/src/plugins/prisma.ts
At the end of the asyncgeneratemethod, replace the synchronous write:- fs.writeFileSync(outFile, prismaSchema); + await fs.promises.writeFile(outFile, prismaSchema);• Imports
No changes needed—fs.promisesis available via the existing
import fs from 'node:fs';Follow-up test ideas (optional but recommended):
- Omit
pluginOptionsentirely → should write todefaultOutputPath/schema.prismaand create the directory.- Provide
pluginOptions.outputas a relative or absolute path → verifyoutFileresolves correctly and directory is created.- (SchemaFile is always defined by the CLI runner) — no need to guard against undefined here.
packages/cli/test/plugins/prisma-plugin.test.ts (1)
46-59: Test aligned to file-based Prisma schema output: LGTMThe path resolution from schemaFile dir to ../prisma/schema.prisma is correct, and the existence check targets the computed destination inside workDir.
Two optional improvements:
- Add an assertion that the generated file content starts with a datasource block to guard against empty writes.
- Consider a regression test for legacy directory-style plugin output (if still supported) or assert a clear error if it’s intentionally dropped.
If backward compatibility is desired, I can add a test variant using output = '../prisma' and assert that the generator either writes schema.prisma inside that dir or fails with a helpful message. Want me to push that?
packages/runtime/test/schemas/typing/typecheck.ts (1)
4-4: Enum[] filter usage added: LGTMImporting Status and exercising status: { has: Status.ACTIVE } validates the array-filter path. Once ArrayFilter becomes schema-aware, we can also add a negative test to ensure non-enum strings are rejected.
Optional: The schema’s provider is PostgreSQL but the dialect here is SQLite. It’s fine for typing, but if you later exercise provider-specific options (e.g., mode: 'insensitive'), align dialect/provider or guard those tests.
Also applies to: 38-41
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
package.json(1 hunks)packages/cli/package.json(1 hunks)packages/cli/src/plugins/prisma.ts(1 hunks)packages/cli/test/plugins/prisma-plugin.test.ts(1 hunks)packages/common-helpers/package.json(1 hunks)packages/create-zenstack/package.json(1 hunks)packages/dialects/sql.js/package.json(1 hunks)packages/eslint-config/package.json(1 hunks)packages/ide/vscode/package.json(1 hunks)packages/language/package.json(1 hunks)packages/runtime/package.json(1 hunks)packages/runtime/src/client/crud-types.ts(5 hunks)packages/runtime/test/schemas/typing/models.ts(1 hunks)packages/runtime/test/schemas/typing/schema.ts(3 hunks)packages/runtime/test/schemas/typing/schema.zmodel(2 hunks)packages/runtime/test/schemas/typing/typecheck.ts(2 hunks)packages/sdk/package.json(1 hunks)packages/tanstack-query/package.json(1 hunks)packages/testtools/package.json(1 hunks)packages/typescript-config/package.json(1 hunks)packages/vitest-config/package.json(1 hunks)packages/zod/package.json(1 hunks)pnpm-workspace.yaml(1 hunks)samples/blog/package.json(1 hunks)tests/e2e/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/language/package.jsonpackages/create-zenstack/package.jsonpackages/vitest-config/package.jsonpackages/tanstack-query/package.jsontests/e2e/package.jsonpackages/cli/test/plugins/prisma-plugin.test.tspackages/runtime/test/schemas/typing/schema.tspackages/dialects/sql.js/package.jsonsamples/blog/package.jsonpackages/runtime/test/schemas/typing/models.tspackages/cli/package.jsonpackages/common-helpers/package.jsonpackages/ide/vscode/package.jsonpackages/runtime/test/schemas/typing/schema.zmodelpackages/sdk/package.jsonpackages/eslint-config/package.jsonpackages/zod/package.jsonpackages/typescript-config/package.jsonpackages/runtime/package.jsonpackages/testtools/package.jsonpackages/runtime/test/schemas/typing/typecheck.tspackages/cli/src/plugins/prisma.tspackages/runtime/src/client/crud-types.ts
tests/e2e/**
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests are in
tests/e2e/directory
Files:
tests/e2e/package.json
**/schema.zmodel
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
zenstack generateafter modifying ZModel schemas
Files:
packages/runtime/test/schemas/typing/schema.zmodel
🧠 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/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: No runtime dependency on prisma/client
Applied to files:
pnpm-workspace.yaml
🧬 Code graph analysis (4)
packages/cli/test/plugins/prisma-plugin.test.ts (1)
packages/cli/test/utils.ts (1)
runCli(20-23)
packages/runtime/test/schemas/typing/typecheck.ts (1)
packages/runtime/test/schemas/typing/models.ts (2)
Status(20-20)Status(21-21)
packages/cli/src/plugins/prisma.ts (1)
packages/sdk/src/index.ts (1)
PrismaSchemaGenerator(3-3)
packages/runtime/src/client/crud-types.ts (1)
packages/sdk/src/schema/schema.ts (7)
FieldIsArray(235-239)GetModelFieldType(151-155)GetEnums(123-123)ModelFieldIsOptional(217-221)NonRelationFields(190-194)RelationFields(196-200)GetModelField(137-141)
⏰ 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: claude-review
- GitHub Check: build-test (20.x)
🔇 Additional comments (20)
packages/runtime/package.json (1)
3-3: All package versions are consistently bumped to 3.0.0-alpha.30—no stale references remainThe consistency sweep passed with exit 0:
- Every
package.json(root, packages, samples, tests) reports version 3.0.0-alpha.30- No occurrences of “alpha.29” were found
- Catalog‐pinned dependencies match the entries in
pnpm‐workspace.yamlNo further changes are needed here.
packages/typescript-config/package.json (1)
3-3: TS config package version updated; OK.No API surface or config changes here; only the version field moved with the train.
If not already enforced elsewhere, consider adding a shared Node engines policy at the root to keep TypeScript features consistent across packages. If desired, I can open a follow-up PR to add an "engines" block and CI checks.
packages/testtools/package.json (1)
3-3: Catalog pins for prisma/typescript are correctI’ve verified that all references to prisma and typescript are sourced via the pnpm catalog and that the pinned versions match what we expect for this release:
• packages/testtools/package.json (lines 36–37):
• "prisma": "catalog:"
• "typescript": "catalog:"
• packages/sdk/package.json (line 45):
• "typescript": "catalog:"
• packages/cli/package.json (line 43):
• "prisma": "catalog:"pnpm-workspace.yaml (catalog section):
catalog: prisma: ^6.10.0 typescript: ^5.8.0No mismatches found—everything aligns with our release targets.
packages/dialects/sql.js/package.json (1)
3-3: Dialect package version bump acknowledged.
Dev and peer dependencies (kysely/sql.js) remain catalog-pinned and unchanged.Sanity check results:
- All
"kysely"entries point tocatalog:in
• packages/sdk/package.json
• packages/runtime/package.json
• packages/dialects/sql.js/package.json- No lingering
alpha.29versions inpackages/dialectspnpm-workspace.yaml (1)
8-12: Catalog Pins Verified: Prisma & TypeScript ConsistencyAll Prisma and TypeScript dependencies across the workspace are correctly routed through catalog pins, with no unintended direct version pins or runtime dependencies detected:
- Root package.json and sub-projects (samples/blog, packages/testtools, sdk, cli) reference both
prismaandtypescriptas"catalog:".- No direct
@prisma/cliententries found in anypackage.json, confirming no runtime client dependency leaks.- Unique TS versions referenced: only
"catalog:", ensuring a single source of truth.- Migration and generation workflows remain intact:
- samples/blog scripts invoke
zen db push,zen migrate dev, etc.- packages/cli actions call
prisma migratecommands as expected.Next steps:
- Manually confirm Prisma CLI 6.10.0 works seamlessly with our code generators and migration scripts (e.g., in samples/blog and packages/cli).
- Verify that any new TypeScript 5.8.0 features or compiler flags do not break builds in projects still using older TS configurations.
packages/vitest-config/package.json (1)
4-4: LGTM: version bump onlyNo functional changes here; change is consistent with the repo-wide release bump.
tests/e2e/package.json (1)
3-3: LGTM: version bump only, aligned with the monorepo releaseHowever, the optional smoke‐test script failed to complete cleanly. Please verify the following issues before merging:
• Build errors in
create-zenstack
- Cannot find modules
colors,commander,oraor their type declarations- Error: no default export from
"node:fs"- Missing base config file
@zenstackhq/typescript-config/base.json• Build errors in
@zenstackhq/kysely-sql-js
- Cannot find modules
kysely,sql.jsor their type declarations• Test invocation error
- Turbo does not recognize
--if-present; tests did not runPlease manually verify that nothing regressed with the catalog updates, fix any missing dependencies or configuration files, and adjust the test command syntax as needed before approving.
packages/language/package.json (1)
4-4: Version bump verified across workspace
- All @zenstackhq/* packages and
create-zenstackare at 3.0.0-alpha.30, as expected- No lingering references to alpha.29 found
- For awareness:
packages/tanstack-query/package.jsonlists a peer dependency on @tanstack/react-query ^5.0.0Approving these changes.
packages/common-helpers/package.json (1)
3-3: Straightforward metadata bump to 3.0.0-alpha.30.No functional or export changes. Looks good and aligned with the release train.
packages/create-zenstack/package.json (1)
3-3: Version bump acknowledged; CLI scaffolder remains unchanged.No behavioral diffs in this package beyond version. All good.
packages/eslint-config/package.json (1)
3-3: ESLint config package version synchronized.Private package; version bump only. Approved.
packages/runtime/test/schemas/typing/schema.zmodel (2)
33-33: User.status: Status[] addedArray-of-enum fits Postgres well and aligns with the typecheck usage (has: Status.ACTIVE). No default needed; Prisma doesn’t support defaults for list types anyway.
2-4: Ensure CI provisions PostgreSQL and DATABASE_URL for testsTests in both
packages/runtimeand the E2E suites now target Postgres and will actively connect, drop/create, and push schemas:
- In
packages/runtime/test/utils.ts, theusePrismaPushpath invokes
npx prisma db push --schema ./schema.prisma --skip-generate --force-reset,
and the fallback path drops/creates the database then runsclient.$pushSchema()against a Postgres dialect.- All
tests/e2e/**/schema.zmodelfiles use
provider = "postgresql"withurl = env("DATABASE_URL"), so those suites will also connect at runtime.Please verify that your CI pipeline:
- Spins up a Postgres instance (e.g., via Docker or a service container).
- Exports
DATABASE_URLpointing to that instance before running tests.- Ensures the test database names (as passed in
dbName) either exist or can be auto-created by the drop/create logic.Optional refactor: after modifying any
.zmodelschema, runnpx zenstack generateto update your generated TypeScript artifacts per repo guidelines.
packages/runtime/test/schemas/typing/models.ts (1)
20-21: Enum re-exports LGTMStatus export/type alias mirrors Role. This keeps consumer imports uniform.
packages/runtime/src/client/crud-types.ts (4)
429-431: OmitInput now accepts boolean per field: LGTMBroadening to boolean improves ergonomics while keeping semantics (only true omits) intact due to Omit[Key] extends true checks downstream.
445-454: SelectInput accepts boolean per field: LGTMThe selection mapping already ignores false/undefined keys, so this widens usability without changing behavior.
456-466: SelectCount boolean + optional relation keys: LGTM (matches usage); ensure nested false is handledThe shape aligns with tests (_count: true and nested selects). With the suggested SelectCountResult tweak, passing false for a relation will properly exclude it from the result type.
563-567: OptionalFieldsForCreate: array fields treated as optional and updatedAt check last: LGTMThis matches typical list semantics (optional on create) and leaves updatedAt as optional when applicable.
packages/cli/package.json (1)
6-6: Version bump to 3.0.0-alpha.30: LGTMNo dependency/peer changes here; consistent with a pre-release bump. Ensure changelog/release notes reflect CLI Prisma plugin output-path change.
Would you like me to scan the repo for references to the old “directory-style” output in docs/examples and open a follow-up PR to update them?
packages/runtime/test/schemas/typing/schema.ts (1)
11-12: Provider switched to PostgreSQL and Status enum/field added: LGTM
- Provider: postgresql enables case-sensitivity and distinct in type surfaces. Tests don’t rely on these yet, so fine.
- User.status: Status[] and enums.Status: conforms to the new array-first branch in WhereInput so has/hasEvery/hasSome become available.
Note: After tightening ArrayFilter as suggested, filters on status will be strictly typed to keyof Status instead of string literals.
Do you want the typing tests to also cover “bad enum value” rejections (e.g., // @ts-expect-error status: { has: 'NOT_A_STATUS' }) once we land the ArrayFilter change?
Also applies to: 52-56, 334-338
|
Claude finished @ymc9's task —— View job PR Review: Merge dev to mainI've completed a comprehensive review of this PR. Here are my findings: Review Tasks:
SummaryThis PR appears to be a release preparation merge from dev to main with minimal risk. The changes are primarily version bumps and minor fixes. Key Changes Analysis✅ Version ConsistencyAll packages correctly updated to ✅ CRUD Types Improvements (
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/runtime/src/client/crud-types.ts (1)
858-863: Update array operations use array-of-array types; fixset/pushelement typingHere
MapModelFieldType<..., Field>already includes the array dimension for list fields. Using[]again makessetaT[][], andpushexpectsT[]values, both incorrect.setshould beElement[], andpushshould acceptElement | Element[].- | (FieldIsArray<Schema, Model, Field> extends true - ? { - set?: MapModelFieldType<Schema, Model, Field>[]; - push?: OrArray<MapModelFieldType<Schema, Model, Field>, true>; - } - : never); + | (FieldIsArray<Schema, Model, Field> extends true + ? { + // set the entire list + set?: ArrayElement<Schema, GetModelFieldType<Schema, Model, Field>>[]; + // append one or many elements + push?: OrArray<ArrayElement<Schema, GetModelFieldType<Schema, Model, Field>>, true>; + } + : never);Support type (if not added already with ArrayFilter change):
type ArrayElement<Schema extends SchemaDef, T extends string> = T extends GetEnums<Schema> ? keyof GetEnum<Schema, T> : MapBaseType<T>;
♻️ Duplicate comments (2)
packages/runtime/src/client/crud-types.ts (2)
249-255: Redefine ArrayFilter to preserve enum element types (repeat)Current
ArrayFilter<T extends string>usesMapBaseType<T>, which yieldsstringfor enum arrays. Introduce a schema-aware element mapper sohas/hasEvery/hasSomeacceptkeyof GetEnum.-type ArrayFilter<T extends string> = { - equals?: MapBaseType<T>[]; - has?: MapBaseType<T>; - hasEvery?: MapBaseType<T>[]; - hasSome?: MapBaseType<T>[]; - isEmpty?: boolean; -}; +type ArrayFilter<Schema extends SchemaDef, T extends string> = { + equals?: ArrayElement<Schema, T>[]; + has?: ArrayElement<Schema, T>; + hasEvery?: ArrayElement<Schema, T>[]; + hasSome?: ArrayElement<Schema, T>[]; + isEmpty?: boolean; +};Support type to add (outside this hunk):
// maps an array field's element type, preserving enum keys type ArrayElement<Schema extends SchemaDef, T extends string> = T extends GetEnums<Schema> ? keyof GetEnum<Schema, T> : MapBaseType<T>;
220-225: Enum[] filters lose enum key typing; make ArrayFilter schema-aware (repeat)Routing arrays before enums is correct, but
ArrayFilter<GetModelFieldType<...>>maps elements viaMapBaseType<T>, erasing enum keys for enum[] fields. TightenArrayFilterto takeSchemaand map enum elements tokeyof GetEnum.Apply:
- : FieldIsArray<Schema, Model, Key> extends true - ? ArrayFilter<GetModelFieldType<Schema, Model, Key>> + : FieldIsArray<Schema, Model, Key> extends true + ? ArrayFilter<Schema, GetModelFieldType<Schema, Model, Key>> : // enum GetModelFieldType<Schema, Model, Key> extends GetEnums<Schema> ? EnumFilter<Schema, GetModelFieldType<Schema, Model, Key>, ModelFieldIsOptional<Schema, Model, Key>>And update the
ArrayFilterdefinition below (see next comment).
🧹 Nitpick comments (2)
packages/runtime/src/client/crud-types.ts (2)
456-456: _count accepts boolean; consider edge ofboolean-typed variablesAllowing
falseis useful. Minor edge: if a variable is typedboolean(not literal),_countpasses the gate and yieldsneverin the result type. Usually not an issue with literaltrue/false, but consider documenting or narrowing totrue | { select: ... }if this becomes noisy.Also applies to: 459-461
563-566: Array fields treated as optional during create — confirm provider parityMarking array fields as optional for create may be intended (e.g., default to empty list). Please confirm parity with Prisma and your providers. If not guaranteed, consider guarding via schema metadata (e.g., “hasDefaultList”) rather than blanket
FieldIsArray.Would you like a small typing test added under runtime tests that asserts
CreateInputoptionality for a required list field?
📜 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 (2)
packages/runtime/src/client/crud-types.ts(7 hunks)packages/runtime/src/client/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/runtime/src/client/index.tspackages/runtime/src/client/crud-types.ts
🧠 Learnings (1)
📓 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
⏰ 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: claude-review
- GitHub Check: build-test (20.x)
🔇 Additional comments (5)
packages/runtime/src/client/index.ts (1)
7-7: Type-only export is safe and desirable hereRe-exporting ZenStackPromise as a type avoids runtime edges and circulars. Looks good.
packages/runtime/src/client/crud-types.ts (4)
430-431: Allowing boolean in OmitInput is fineAccepting
falseis harmless; actual omission still requirestruedue to the check in DefaultModelResult. This increases ergonomics without breaking behavior.
445-446: SelectInput now accepts boolean — consistent with result key filtering
Select[Key] extends false | undefined ? never : Keyensuresfalseexcludes the key. Change looks good.
696-696: Create:setuses field value type — correctFor array scalars,
MapModelFieldType<...>resolves toElement[], which matches expected “set entire list” semantics.
1206-1216: Nested upsert:wherenow uses the relation model’s unique input — good tighteningUsing
WhereUniqueInput<Schema, RelationFieldType<...>>is more precise and matches the nested target model.
|
Claude finished @ymc9's task —— View job PR Review: Merge dev to main (v3.0.0-alpha.30)I've completed a comprehensive review of this PR. Here are my findings: Review Tasks:
SummaryThis PR appears to be a release preparation merge from dev to main with minimal risk. The changes are primarily version bumps and minor fixes. Key Changes Analysis✅ Version ConsistencyAll packages correctly updated to ✅ CRUD Types Improvements (
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/runtime/src/client/crud-types.ts (1)
220-225: Array-first dispatch + schema-aware ArrayFilter is a good fix; add nullability for optional arraysRouting arrays before enums and switching to
ArrayFilter<Schema, ...>fixes enum[] element typing (now enum keys instead of plain strings). Nice. However, optional array fields can no longer expressequals: nullsinceArrayFilterdoesn’t accept a nullability parameter (unlikeEnumFilter/PrimitiveFilter). This is a subtle regression for nullable array columns.Recommend threading field optionality into
ArrayFilter:- : FieldIsArray<Schema, Model, Key> extends true - ? ArrayFilter<Schema, GetModelFieldType<Schema, Model, Key>> + : FieldIsArray<Schema, Model, Key> extends true + ? ArrayFilter< + Schema, + GetModelFieldType<Schema, Model, Key>, + ModelFieldIsOptional<Schema, Model, Key> + > : // enum GetModelFieldType<Schema, Model, Key> extends GetEnums<Schema> ? EnumFilter<Schema, GetModelFieldType<Schema, Model, Key>, ModelFieldIsOptional<Schema, Model, Key>>
🧹 Nitpick comments (4)
packages/runtime/src/client/crud-types.ts (4)
434-435: OmitInput booleans: confirm runtime treatsfalsethe same as absentChanging omit entries from
truetobooleanbroadens the API, but it may cause accidental “present-but-false” states that differ from “key absent” at runtime. Verify that any runtime selection/serialization logic interprets{ omit: { foo: false } }equivalently toomit: {}.If not, keep
true-only or normalize booleans at the boundary.
449-451: SelectInput booleans: watch for boolean widening affecting ModelSelectResultAllowing
booleanfor non-relation selects is convenient, but when values widen toboolean(e.g., computed flags),ModelSelectResult’s conditionalSelect[Key] extends false | undefined ? never : ...still includes the key, which is fine, but downstream types may widen and lose precision. If you see surprisingnever/widening behavior, consider constraining totrueor require literaltrue(doc it) for precise inference.
459-465: _count: boolean shape is ergonomic; guard againstneverwhen widened
SelectCountnow acceptsboolean. Whenselect._countis a widenedboolean,SelectCountResult<C>may evaluate tonever, yet_countis still included byModelSelectResult, yielding_count: never. To harden inference without reverting the ergonomic boolean:
- Switch to non-distributive checks in the result helper:
Type-only (outside this hunk):
type SelectCountResult<Schema extends SchemaDef, Model extends GetModels<Schema>, C> = [C] extends [true] ? { [Key in RelationFields<Schema, Model> as FieldIsArray<Schema, Model, Key> extends true ? Key : never]: number } : C extends { select: infer S } ? { [Key in keyof S]: number } : never;This reduces surprising
neverwhen callers pass literaltruevs widenedboolean.
699-701: Create:seton array fields likely should accept an array, not a single element
ScalarCreatePayloadnow allows{ set?: MapModelFieldType<...> }for array fields. IfMapModelFieldTypefor array fields is the element type (common in your update payloads), this makesseta single element, which is inconsistent with update’sset?: MapModelFieldType<..., Field>[]. IfMapModelFieldTypeis alreadyElement[]for arrays, then create’ssetbecomesElement[][], which is also undesirable.For consistency with update and typical list semantics, prefer an array:
- ? { - set?: MapModelFieldType<Schema, Model, Field>; - } + ? { + // set entire list; mirrors update's shape + set?: MapModelFieldType<Schema, Model, Field>[]; + }Alternatively, drop
setin create and rely on direct assignment with the field value (i.e., pass the array directly).
📜 Review details
Configuration used: Path: .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 (2)
packages/runtime/src/client/crud-types.ts(8 hunks)packages/runtime/test/schemas/typing/typecheck.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/runtime/test/schemas/typing/typecheck.ts
🧰 Additional context used
📓 Path-based instructions (1)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/runtime/src/client/crud-types.ts
🧠 Learnings (1)
📓 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
🧬 Code graph analysis (1)
packages/runtime/src/client/crud-types.ts (3)
packages/sdk/src/schema/schema.ts (9)
FieldIsArray(235-239)GetModelFieldType(151-155)GetEnums(123-123)SchemaDef(10-18)GetEnum(125-125)NonRelationFields(190-194)RelationFields(196-200)GetModelField(137-141)RelationFieldType(208-215)packages/runtime/src/client/query-builder.ts (1)
MapBaseType(32-44)packages/runtime/src/utils/type-utils.ts (1)
MapBaseType(37-37)
⏰ 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 (3)
packages/runtime/src/client/crud-types.ts (3)
257-260: Enum-to-key mapping via MapScalarType looks correctMapping enums to
keyof GetEnumand other scalars throughMapBaseTypeis the right abstraction and unblocks strict typing for enum[] filters.
567-571: Optional array fields included in create-optionality: LGTMTreating array fields as optional during create makes sense and matches typical ORM semantics. Order change with
updatedAtis harmless here.
1210-1210: Nested upsertwherenow points to the relation model: correctSwitching
wheretoWhereUniqueInput<Schema, RelationFieldType<...>>fixes the prior type leak to the parent model. Good catch.
* chore: use zod v3 for dev dependency * update
|
Claude finished @ymc9's task —— View job PR Review: Merge dev to main (v3.0.0-alpha.30)I've completed a comprehensive review of this PR. Here are my findings: Review Tasks:
SummaryThis PR appears to be a release preparation merge from dev to main with minimal risk. The changes are primarily version bumps and minor fixes. Key Changes Analysis✅ Version ConsistencyAll packages correctly updated to ✅ CRUD Types Improvements (
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/runtime/src/client/crud-types.ts (2)
240-248: EnumFilter lacks support for aggregations used in having/groupBy; add gated _count/_min/_maxValidator permits enum aggregations; the type system should, too. Add a WithAggregations generic and gate allowed operators.
-type EnumFilter<Schema extends SchemaDef, T extends GetEnums<Schema>, Nullable extends boolean> = - | NullableIf<keyof GetEnum<Schema, T>, Nullable> - | { - equals?: NullableIf<keyof GetEnum<Schema, T>, Nullable>; - in?: (keyof GetEnum<Schema, T>)[]; - notIn?: (keyof GetEnum<Schema, T>)[]; - not?: EnumFilter<Schema, T, Nullable>; - }; +type EnumFilter< + Schema extends SchemaDef, + T extends GetEnums<Schema>, + Nullable extends boolean, + WithAggregations extends boolean = false, +> = + | NullableIf<keyof GetEnum<Schema, T>, Nullable> + | ({ + equals?: NullableIf<keyof GetEnum<Schema, T>, Nullable>; + in?: (keyof GetEnum<Schema, T>)[]; + notIn?: (keyof GetEnum<Schema, T>)[]; + not?: EnumFilter<Schema, T, Nullable, WithAggregations>; + } & (WithAggregations extends true + ? { + _count?: NumberFilter<Schema, 'Int', false, false>; + _min?: EnumFilter<Schema, T, false, false>; + _max?: EnumFilter<Schema, T, false, false>; + } + : {}));
862-867: Fix array update payload: set/push currently nest arrays; use element types instead
MapModelFieldType<..., Field>for an array field already yields element[], soset?: MapModelFieldType[]becomes element[][]. Similarly,pushshould accept element or element[], not element[] or element[][].Apply this diff:
- | (FieldIsArray<Schema, Model, Field> extends true - ? { - set?: MapModelFieldType<Schema, Model, Field>[]; - push?: OrArray<MapModelFieldType<Schema, Model, Field>, true>; - } - : never); + | (FieldIsArray<Schema, Model, Field> extends true + ? { + // replace element[][] with element[] + set?: MapModelFieldType<Schema, Model, Field>; + // push element | element[] + push?: OrArray<MapScalarType<Schema, GetModelFieldType<Schema, Model, Field>>, true>; + } + : never);packages/runtime/src/client/crud/validator.ts (2)
321-333: Validator still routes enum[] through enum filters; switch to array-first to match types and new testsTypes (WhereInput) handle array-of-enum via ArrayFilter. Validator branches enum before array, so enum[] fields won’t get array operators (equals: [], has, hasEvery/Some). Reorder and pass optionality to array filter.
- } else { - const enumDef = getEnum(this.schema, fieldDef.type); - if (enumDef) { - // enum - if (Object.keys(enumDef).length > 0) { - fieldSchema = this.makeEnumFilterSchema(enumDef, !!fieldDef.optional, withAggregations); - } - } else if (fieldDef.array) { - // array field - fieldSchema = this.makeArrayFilterSchema(fieldDef.type as BuiltinType); - } else { + } else { + if (fieldDef.array) { + // array field (including enum[]) + fieldSchema = this.makeArrayFilterSchema( + fieldDef.type as BuiltinType, + !!fieldDef.optional, + ); + } else { + const enumDef = getEnum(this.schema, fieldDef.type); + if (enumDef) { + // enum + if (Object.keys(enumDef).length > 0) { + fieldSchema = this.makeEnumFilterSchema(enumDef, !!fieldDef.optional, withAggregations); + } + } else { // primitive field fieldSchema = this.makePrimitiveFilterSchema( fieldDef.type as BuiltinType, !!fieldDef.optional, withAggregations, ); - } + } + } }And update makeArrayFilterSchema (see next comment).
436-444: Array filter schema should allow null for equals/has when the field is optional; add optionality parameterBring validator in line with the proposed type changes and with other filters’ NullableIf pattern.
- private makeArrayFilterSchema(type: BuiltinType) { - return z.strictObject({ - equals: this.makePrimitiveSchema(type).array().optional(), - has: this.makePrimitiveSchema(type).optional(), - hasEvery: this.makePrimitiveSchema(type).array().optional(), - hasSome: this.makePrimitiveSchema(type).array().optional(), - isEmpty: z.boolean().optional(), - }); - } + private makeArrayFilterSchema(type: BuiltinType, optional: boolean) { + const elem = this.makePrimitiveSchema(type); + return z.strictObject({ + equals: this.nullableIf(elem.array(), optional).optional(), + has: this.nullableIf(elem, optional).optional(), + hasEvery: elem.array().optional(), + hasSome: elem.array().optional(), + isEmpty: z.boolean().optional(), + }); + }And adjust the call site per prior comment.
♻️ Duplicate comments (1)
pnpm-workspace.yaml (1)
8-8: Prisma catalog downgraded to ^6.10.0: verify this is intentional and compatible with the CLI plugin path changesThis was flagged previously. Echoing it with added context: given our learnings that migrations still use Prisma CLI under the hood, please confirm 6.10.x is required and that newer 6.14.x features/bugfixes aren’t needed by the updated CLI Prisma plugin (schemaFile/out path resolution).
You can quickly sanity-check for tight coupling by scanning CLI/plugin code paths that shell out to Prisma:
#!/bin/bash # Inspect Prisma CLI integration points rg -nC3 -g 'packages/**' -P '\b(prisma\s+(db\s+push|migrate|generate|format)|PrismaClientKnownRequestError|P20\d{2})\b'If nothing in the plugin depends on >=6.14 behavior, consider leaving a brief comment in pnpm-workspace.yaml explaining the reason for pinning to ^6.10.0 to help future maintainers.
🧹 Nitpick comments (4)
packages/zod/src/index.ts (1)
31-31: Align DateTime acceptance with runtime validator: consider allowing Date objects tooRuntime validator accepts both
Dateand ISO string (per the PR context). To keep behavior consistent here, consider permitting either form:- .with('DateTime', () => z.string().datetime()) + .with('DateTime', () => z.union([z.date(), z.string().datetime({ offset: true })]))
- Adds
{ offset: true }to tolerate timezone offsets; adjust if you want to requireZ.- If you intentionally want string-only here (e.g., for select schema semantics), a file-level comment would prevent future “helpful” changes back to union.
If you want to assert parity, add a focused test that feeds both
Dateand ISO strings through the same path used by this schema builder and by the runtime validator.packages/zod/package.json (1)
33-35: Align Zod pins with the catalog and test Zod 4 in CIPinning Zod to
~3.25.0in two places diverges from our catalog’s^3.25 || ^4range. To keep a single source of truth and ensure compatibility with Zod 4, update both locations and add CI coverage:• packages/zod/package.json (line 34):
"zod": "~3.25.0"
• packages/runtime/package.json (line 103):
"zod": "~3.25.0"Options:
- Replace both with
then add a GitHub Actions matrix job that installs and tests against Zod 3 and Zod 4."zod": "catalog:zod"- Keep
~3.25.0locally, but add a CI job (using pnpm-override or similar) that forces Zod 4 for a compile/test run.Let me know if you’d like a draft of the GitHub Actions matrix snippet. Also please verify there are no other hard-pinned Zod versions in the repo.
packages/runtime/src/client/crud-types.ts (1)
433-435: Boolean shape for select/omit/_count verified – safe to landNo downstream code relies on strict
=== true(grep scan found only usage of boolean flags in tests and examples), so this relaxation won’t break existing call sites. All existing.trueliterals still satisfy the newboolean | undefinedtype.Optional documentation updates:
- README.md (around lines 192–196): revise the
_count: trueexample to clarify that booleans (includingfalseor omitting the field) are now accepted.- packages/runtime/src/client/contract.ts (around lines 291–297): adjust code samples for
_countandselectto mention the new optional‐boolean shape.packages/runtime/src/client/crud/validator.ts (1)
255-271: Type-def schema built with z.object(...).passthrough() — good trade-off for typed JSONThis enforces declared fields while preserving extra keys. Keep if policy allows unknown keys on typed JSON; otherwise consider
.strict()plus explicit allowlist later.
📜 Review details
Configuration used: Path: .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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
packages/runtime/package.json(2 hunks)packages/runtime/src/client/crud-types.ts(8 hunks)packages/runtime/src/client/crud/validator.ts(8 hunks)packages/runtime/test/client-api/mixin.test.ts(1 hunks)packages/runtime/test/client-api/typed-json-fields.test.ts(2 hunks)packages/zod/package.json(2 hunks)packages/zod/src/index.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/runtime/package.json
🧰 Additional context used
📓 Path-based instructions (1)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/runtime/test/client-api/mixin.test.tspackages/runtime/test/client-api/typed-json-fields.test.tspackages/zod/src/index.tspackages/zod/package.jsonpackages/runtime/src/client/crud/validator.tspackages/runtime/src/client/crud-types.ts
🧠 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: Schema-first approach with ZModel DSL extension of Prisma schema language
Applied to files:
packages/runtime/src/client/crud/validator.ts
📚 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: ZModel schema (`schema.zmodel`) defines database structure and policies
Applied to files:
packages/runtime/src/client/crud/validator.ts
🧬 Code graph analysis (2)
packages/runtime/src/client/crud/validator.ts (3)
samples/blog/zenstack/schema.ts (1)
schema(9-230)packages/runtime/src/client/crud/operations/base.ts (1)
schema(86-88)packages/runtime/test/schemas/basic/schema.ts (1)
schema(9-284)
packages/runtime/src/client/crud-types.ts (2)
packages/sdk/src/schema/schema.ts (10)
FieldIsArray(235-239)GetModelFieldType(151-155)GetEnums(123-123)ModelFieldIsOptional(217-221)SchemaDef(10-18)GetEnum(125-125)NonRelationFields(190-194)RelationFields(196-200)GetModelField(137-141)RelationFieldType(208-215)packages/runtime/src/client/query-builder.ts (1)
MapBaseType(32-44)
⏰ 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 (16)
pnpm-workspace.yaml (2)
12-12: All packages aligned to TypeScript ^5.8.0 with no deprecated tsconfig options – ready to mergeConfirmed that no
packages/**/package.jsonfile pins a different TS version outside the workspace catalog, and notsconfig*.jsonfiles use deprecated options likepreserveValueImportsorimportsNotUsedAsValues. All modules compile cleanly under TS 5.8.0.
7-7: ✅ Confirmed .datetime() parity across Zod v3.25 and v4
z.string().datetime()(alias forz.iso.datetime()) remains available and stable in Zod v4 with the same signature and options (offset,precision) as in v3.25, enforcing ISO 8601 date-time strings without introducing breaking changes to the validator itself. (v4.zod.dev, odocs-zod.vercel.app)- The method form (
z.string().datetime()) is still supported (though deprecated in favor of top-levelz.iso.datetime()), and no runtime or typing drift is expected. (v4.zod.dev)No further action needed.
packages/runtime/test/client-api/mixin.test.ts (1)
54-54: Broader, case-insensitive error assertion improves resilienceSwitching to
.rejects.toThrow(/invalid/i)decouples the test from exact error strings (driver/Prisma version differences). LGTM.packages/zod/package.json (1)
3-3: Version bump to alpha.30: LGTMpackages/runtime/test/client-api/typed-json-fields.test.ts (2)
130-130: Error assertion relaxed to /invalid/i: LGTMMatches the approach in mixin tests; avoids flakiness when error messages vary across engines.
218-218: Same here: resilient, case-insensitive matchingConsistent with the create case and mixin tests. Looks good.
packages/runtime/src/client/crud-types.ts (4)
257-260: LGTM: MapScalarType correctly maps enums to keyof and primitives via MapBaseTypeThis preserves enum key types inside array and scalar filters.
700-702: LGTM: Create array fields use set?: MapModelFieldType<...> which resolves to element[]Matches validator’s
{ set: <array> }path and avoids double-wrapping arrays.
567-571: Reordered OptionalFieldsForCreate checks; confirm semantics for updatedAt and arraysMoving the array check before updatedAt is fine but can alter optionality for array+updatedAt fields. Verify no regressions in create input optional keys.
1210-1213: LGTM: NestedUpsertInput.where now targets the related model’s unique inputThis prevents invalid cross-model unique filters and improves type safety.
packages/runtime/src/client/crud/validator.ts (6)
236-243: Numeric/Date refinements are correct and consistent with zod 3.x
- Int: z.number().int()
- BigInt: z.union([z.number().int(), z.bigint()])
- DateTime: z.union([z.date(), z.string().datetime()])
472-479: LGTM: DateTime filters accept z.string().datetime() | z.date() and aggregate gates are preservedMatches the primitive schema mapping and keeps having/aggregate behavior intact.
525-525: _count aggregation now uses z.number().int() — correctConsistent with integer nature of counts.
1025-1026: UpdateMany.limit uses z.number().int().nonnegative().optional() — good tighteningPrevents negative limits and non-integers.
1163-1166: DeleteMany.limit uses z.number().int().nonnegative().optional() — good tighteningSame improvement for deleteMany.
1345-1350: Skip/Take tightened to number().int() (skip nonnegative) — goodMatches typical pagination semantics and prevents fractional values.
Summary by CodeRabbit