-
-
Notifications
You must be signed in to change notification settings - Fork 12
merge dev to main #87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
#84) * chore: update zod, add several helpers, CLI loads config from pkg.json * add test * addressing comments
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
WalkthroughThis update introduces configuration-based schema and output path resolution in the CLI, adds a utility for upward file searching, and re-exports helpers. It also updates the Changes
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/cli/src/actions/db.tsOops! Something went wrong! :( ESLint: 9.29.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by packages/cli/src/actions/migrate.tsOops! Something went wrong! :( ESLint: 9.29.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by packages/runtime/src/client/crud/operations/group-by.tsOops! Something went wrong! :( ESLint: 9.29.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ 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). (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR merges the dev branch into main, updating package versions, dependencies, and refactoring key tooling and CLI behaviors.
- Bumps all packages to
3.0.0-alpha.10and updates Zod to v4. - Adds a reusable SQL helper export with corresponding bundler and package exports.
- Refactors CLI actions to respect
package.json-based schema/output configuration and consolidates command-building logic.
Reviewed Changes
Copilot reviewed 39 out of 40 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/package.json | Bumped e2e test package version to 3.0.0-alpha.10 |
| samples/blog/package.json | Bumped sample-blog package version to 3.0.0-alpha.10 |
| pnpm-workspace.yaml | Updated Zod dependency to ^4.0.0 |
| packages/zod/src/types.ts | Changed Zod imports from zod/v4 to zod |
| packages/zod/src/index.ts | Changed Zod imports from zod/v4 to zod |
| packages/zod/package.json | Bumped @zenstackhq/zod version |
| packages/typescript-config/package.json | Bumped @zenstackhq/typescript-config version |
| packages/testtools/package.json | Bumped @zenstackhq/testtools version |
| packages/tanstack-query/package.json | Bumped @zenstackhq/tanstack-query version |
| packages/sdk/package.json | Bumped @zenstackhq/sdk version |
| packages/runtime/tsup.config.ts | Added helpers entry to bundle the new helper file |
| packages/runtime/src/helpers.ts | Introduced export { sql } from 'kysely' helper |
| packages/runtime/src/client/errors.ts | Simplified InternalError class declaration |
| packages/runtime/src/client/crud/validator.ts | Updated Zod import path |
| packages/runtime/src/client/crud/operations/update.ts | Renamed normalizeArgs to normalizedArgs for clarity |
| packages/runtime/src/client/crud/operations/group-by.ts | Renamed normalizeArgs to normalizedArgs for clarity |
| packages/runtime/src/client/crud/operations/delete.ts | Renamed normalizeArgs to normalizedArgs for clarity |
| packages/runtime/src/client/crud/operations/create.ts | Renamed normalizeArgs to normalizedArgs for clarity |
| packages/runtime/src/client/crud/operations/count.ts | Renamed normalizeArgs to normalizedArgs for clarity |
| packages/runtime/src/client/crud/operations/aggregate.ts | Renamed normalizeArgs to normalizedArgs for clarity |
| packages/runtime/src/client/contract.ts | Added TransactionClientContract type |
| packages/runtime/package.json | Bumped runtime version & added exports for ./helpers |
| packages/language/src/utils.ts | Reformatted getLiteralArray signature to new line style |
| packages/language/package.json | Bumped language package version |
| packages/ide/vscode/package.json | Bumped VSCode extension version |
| packages/eslint-config/package.json | Bumped ESLint config package version |
| packages/create-zenstack/package.json | Bumped create-zenstack version |
| packages/common-helpers/src/param-case.ts | Reformatted paramCase chain & switched to single quotes |
| packages/common-helpers/src/index.ts | Exported newly added find-up utility |
| packages/common-helpers/src/find-up.ts | Added findUp utility implementation |
| packages/common-helpers/package.json | Bumped common-helpers package version |
| packages/cli/test/generate.test.ts | Added E2E test to verify package.json-based CLI config |
| packages/cli/src/actions/migrate.ts | Refactored migrate commands to build multi-part command strings |
| packages/cli/src/actions/generate.ts | Integrated package.json config via new getPkgJsonConfig hook |
| packages/cli/src/actions/db.ts | Refactored db push command construction |
| packages/cli/src/actions/action-utils.ts | Added getPkgJsonConfig and findUp import; extended utils |
| packages/cli/package.json | Bumped CLI package version |
| package.json | Bumped root project version |
| .prettierignore | Added ignore patterns for generated files |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (4)
packages/common-helpers/src/find-up.ts:18
- The JSDoc @param is missing the parameter name. It should read
@param result An array of strings representing the accumulated results when multiple is true.
* @param An array of strings representing the accumulated results used in multiple results
packages/cli/src/actions/action-utils.ts:1
- This file uses fs and path (e.g., fs.existsSync, path.resolve) but does not import them. Add
import fs from 'fs'andimport path from 'path'at the top.
import { findUp } from '@zenstackhq/common-helpers';
packages/cli/src/actions/migrate.ts:64
- Removing the
{ stdio: 'inherit' }option may suppress command output. Confirm thatexecPackagedefaults to inheriting stdio or explicitly pass it to preserve interactive feedback.
await execPackage(cmd);
packages/cli/src/actions/db.ts:38
- Similar to the migrate action, removing stdio inheritance may hide output during
prisma db push. EnsureexecPackagesupports stdio inheritance or pass{ stdio: 'inherit' }.
await execPackage(cmd);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🔭 Outside diff range comments (2)
packages/zod/src/index.ts (1)
31-32:z.iso.datetime()may be undefined in vanilla ZodCore Zod (v3 & v4) does not ship an
isonamespace; that method is typically provided by community plugins or local augmentation. With the import path change above, any module-augmentation must still run before this file is executed, or the call will be undefined at runtime.If no explicit augmentation exists, replace with the canonical helper:
- .with('DateTime', () => z.iso.datetime()) + .with('DateTime', () => z.string().datetime({ offset: true }))or whatever helper your plugin exposes.
packages/cli/src/actions/migrate.ts (1)
99-101: Inconsistency: runStatus uses different pattern and retains stdio option.The runStatus function still uses the old template literal approach and retains the
stdio: 'inherit'option, while other functions were refactored to use array joining and had the stdio option removed.For consistency, consider refactoring runStatus to match the pattern used in other functions:
- await execPackage(`prisma migrate status --schema "${prismaSchemaFile}"`, { - stdio: 'inherit', - }); + const cmd = [ + 'prisma migrate status', + ` --schema "${prismaSchemaFile}"`, + ].join(''); + + await execPackage(cmd);Or if the
stdio: 'inherit'is intentionally kept for status command, document why this function needs different behavior.
🧹 Nitpick comments (3)
packages/runtime/src/client/crud/operations/update.ts (1)
14-23: Duplication in validator + runner dispatch could be collapsed
Every branch repeats the same pattern: validate with X → run X. A small helper (e.g. aconst dispatch = { … }) would remove the boilerplate and make future additions less error-prone:- return match(operation) - .with('update', () => this.runUpdate(this.inputValidator.validateUpdateArgs(this.model, normalizedArgs))) - .with('updateMany', () => - this.runUpdateMany(this.inputValidator.validateUpdateManyArgs(this.model, normalizedArgs)), - ) - ... +const dispatch = { + update: () => this.runUpdate(this.inputValidator.validateUpdateArgs(this.model, normalizedArgs)), + updateMany: () => + this.runUpdateMany(this.inputValidator.validateUpdateManyArgs(this.model, normalizedArgs)), + updateManyAndReturn: () => + this.runUpdateManyAndReturn( + this.inputValidator.validateUpdateManyAndReturnArgs(this.model, normalizedArgs), + ), + upsert: () => this.runUpsert(this.inputValidator.validateUpsertArgs(this.model, normalizedArgs)), +} as const; + +return dispatch[operation]();Not critical, but would tighten the code and sidestep accidental drift if more operations are added.
packages/runtime/src/client/errors.ts (1)
22-22: LGTM - Simplification maintains identical behaviorThe removal of the explicit constructor is valid since the base
Errorclass already provides the same functionality. However, this creates inconsistency with other error classes in the same file (InputValidationError,QueryError,NotFoundError) which all have explicit constructors.Consider applying the same simplification to other error classes for consistency, or restore the explicit constructor to maintain uniformity:
-export class InternalError extends Error {} +export class InternalError extends Error { + constructor(message: string) { + super(message); + } +}packages/common-helpers/src/find-up.ts (1)
18-18: Fix incomplete JSDoc parameter documentation.The parameter description is missing the parameter name.
- * @param An array of strings representing the accumulated results used in multiple results + * @param result An array of strings representing the accumulated results used in multiple results
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (39)
.prettierignore(1 hunks)package.json(1 hunks)packages/cli/package.json(1 hunks)packages/cli/src/actions/action-utils.ts(3 hunks)packages/cli/src/actions/db.ts(1 hunks)packages/cli/src/actions/generate.ts(3 hunks)packages/cli/src/actions/migrate.ts(1 hunks)packages/cli/test/generate.test.ts(1 hunks)packages/common-helpers/package.json(1 hunks)packages/common-helpers/src/find-up.ts(1 hunks)packages/common-helpers/src/index.ts(1 hunks)packages/common-helpers/src/param-case.ts(1 hunks)packages/create-zenstack/package.json(1 hunks)packages/eslint-config/package.json(1 hunks)packages/ide/vscode/package.json(1 hunks)packages/language/package.json(1 hunks)packages/language/src/utils.ts(1 hunks)packages/runtime/package.json(3 hunks)packages/runtime/src/client/contract.ts(1 hunks)packages/runtime/src/client/crud/operations/aggregate.ts(1 hunks)packages/runtime/src/client/crud/operations/count.ts(1 hunks)packages/runtime/src/client/crud/operations/create.ts(1 hunks)packages/runtime/src/client/crud/operations/delete.ts(1 hunks)packages/runtime/src/client/crud/operations/group-by.ts(1 hunks)packages/runtime/src/client/crud/operations/update.ts(1 hunks)packages/runtime/src/client/crud/validator.ts(1 hunks)packages/runtime/src/client/errors.ts(1 hunks)packages/runtime/src/helpers.ts(1 hunks)packages/runtime/tsup.config.ts(1 hunks)packages/sdk/package.json(1 hunks)packages/tanstack-query/package.json(1 hunks)packages/testtools/package.json(1 hunks)packages/typescript-config/package.json(1 hunks)packages/zod/package.json(1 hunks)packages/zod/src/index.ts(1 hunks)packages/zod/src/types.ts(1 hunks)pnpm-workspace.yaml(1 hunks)samples/blog/package.json(1 hunks)tests/e2e/package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
packages/language/src/utils.ts (1)
packages/language/src/generated/ast.ts (4)
Expression(114-114)Expression(116-116)ConfigExpr(106-106)ConfigExpr(108-108)
packages/runtime/src/client/contract.ts (2)
packages/sdk/src/schema/schema.ts (1)
SchemaDef(10-17)packages/runtime/src/client/index.ts (1)
ClientContract(2-2)
packages/cli/test/generate.test.ts (1)
packages/cli/test/utils.ts (2)
createProject(12-18)runCli(20-23)
packages/cli/src/actions/db.ts (1)
packages/cli/src/utils/exec-utils.ts (1)
execPackage(20-26)
packages/cli/src/actions/generate.ts (1)
packages/cli/src/actions/action-utils.ts (1)
getPkgJsonConfig(64-85)
packages/cli/src/actions/action-utils.ts (2)
packages/cli/src/cli-error.ts (1)
CliError(4-4)packages/common-helpers/src/find-up.ts (1)
findUp(21-34)
⏰ 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). (1)
- GitHub Check: build-test (20.x)
🔇 Additional comments (43)
packages/runtime/src/client/crud/operations/delete.ts (1)
10-16: Consistent variable naming – change looks correctRenaming the local binding to
normalizedArgsimproves clarity and aligns with the same refactor across sibling handlers. All subsequent references were updated, so there’s no functional impact or type safety regression. ✅packages/runtime/src/client/crud/operations/count.ts (1)
8-12: Variable rename removes shadowing risk and clarifies intentRenaming the local variable to
normalizedArgscleanly distinguishes it from thenormalizeArgsmethod, eliminating the previous self-shadowing ambiguity and improving readability. Nice catch.packages/runtime/src/client/crud/operations/create.ts (1)
11-22: Consistent variable naming ‒ LGTMRenaming to
normalizedArgskeeps this handler in line with the rest of the CRUD modules and removes the minor ambiguity present before. No functional or type-level concerns spotted.packages/runtime/src/client/crud/operations/update.ts (1)
11-11: Variable rename increases clarity and consistency
The switch fromnormalizeArgstonormalizedArgsbetter reflects that the value is already transformed. Matches the convention adopted in the sibling handlers.packages/runtime/src/client/crud/operations/group-by.ts (1)
10-14: Variable-name tweak looks goodRenaming
normalizeArgs→normalizedArgsremoves the misleading verb-noun mismatch and reads clearer in the subsequent code. No functional impact observed.pnpm-workspace.yaml (1)
8-8: No lingeringzod/v4sub-path imports detected – verify v3-specific API usage
Ran:rg -n 'from .+["'\''"]zod/v4["'\''"]' -g '*.ts' -g '*.tsx' -g '*.js' -g '*.cjs'Result: no matches.
• Although the deprecated sub-path imports are gone, please manually review the codebase for any v3-only APIs (e.g., legacy schema methods or types that were removed/renamed in v4) to ensure a full migration.
tests/e2e/package.json (1)
3-3: Version bump looks consistent with the mono-repo release trainNo functional changes – only the version moved to
3.0.0-alpha.10.packages/eslint-config/package.json (1)
3-3: Package version synced – nothing else changedChange is straightforward.
packages/typescript-config/package.json (1)
3-3: TS config package bumped in lock-stepAll good here.
samples/blog/package.json (1)
3-3: Sample app version updated – OKPure metadata update; no action needed.
packages/zod/src/index.ts (1)
3-3: Confirmzodmajor‐version alignment after dropping the/v4sub-path
import { z, ZodType } from 'zod'assumes the default entry now exposes the v4 API.
If the repo still pulls a v3 build (or a pre-release tag that only exposes v4 underzod/v4), this line will break compilation. Double-check both:
peerDependencies.zod/ lock-file version resolves to^4.- All other packages have moved off the
/v4sub-path as well.Otherwise re-introduce the sub-path or pin the correct version.
packages/language/package.json (1)
4-4: Version bump LGTM – no other manifest fields changed.packages/zod/package.json (1)
3-3: Consistent version bump acknowledgedRemember to publish the tag only after verifying the
zodpeer range matches the import change highlighted insrc/index.ts.packages/common-helpers/package.json (1)
3-3: Package version updated successfullyNo further issues spotted.
packages/ide/vscode/package.json (1)
4-4: VS Code extension version increment looks goodEnsure the marketplace listing is published with the matching pre-release tag.
packages/testtools/package.json (1)
3-3: Version bump OK
@zenstackhq/testtoolscorrectly follows the repository version bump. No additional issues spotted.packages/tanstack-query/package.json (1)
3-3: Package version updated successfully
@zenstackhq/tanstack-querynow points to3.0.0-alpha.10. Looks good.packages/create-zenstack/package.json (1)
3-3: Version synchronized
create-zenstackpackage version updated to3.0.0-alpha.10. No concerns.packages/sdk/package.json (1)
3-3: SDK version bump aligns with mono-repo release
@zenstackhq/sdknow at3.0.0-alpha.10. Ensure any external documentation or API references are adjusted accordingly.packages/zod/src/types.ts (1)
2-2: LGTM! Zod v4 import path update is correct.The import path change from 'zod/v4' to 'zod' is correct for the Zod v4 upgrade, where types are now exported directly from the main package.
packages/runtime/src/client/crud/validator.ts (1)
5-5: LGTM! Consistent Zod v4 import path update.The import path change is consistent with the Zod v4 upgrade and matches the pattern used in other files.
packages/runtime/src/client/crud/operations/aggregate.ts (1)
10-13: LGTM! Variable renaming improves clarity.The change from
normalizeArgstonormalizedArgsmakes the variable name more descriptive and clearly indicates it contains the result of normalization rather than being a function.packages/common-helpers/src/param-case.ts (1)
7-22: LGTM! Style consistency improvement.The change from double quotes to single quotes improves code style consistency without affecting functionality.
packages/runtime/src/client/contract.ts (1)
161-167: LGTM! Well-designed type for transaction safety.The new
TransactionClientContracttype provides excellent type safety by restricting the client interface to only methods supported within transactions. The use ofOmitwithTransactionUnsupportedMethodsis a clean pattern that will prevent unsupported operations at compile time.packages/cli/src/actions/db.ts (1)
29-35: LGTM - Improved command string constructionThe refactoring from template literal to array joining makes the conditional flag inclusion cleaner and more maintainable.
packages/language/src/utils.ts (1)
334-336: LGTM - Code formatting improvementThe function signature formatting change improves consistency and follows standard TypeScript formatting conventions.
packages/cli/package.json (1)
6-6: LGTM - Version bump aligns with monorepo releaseThe version increment from
3.0.0-alpha.9to3.0.0-alpha.10is consistent with the coordinated release across the monorepo packages.packages/runtime/tsup.config.ts (1)
7-7: Verified:helpersentry point properly configured
- Confirmed
packages/runtime/src/helpers.tsexists and exportssqlfromkysely.- Confirmed
packages/runtime/package.jsonincludes"./helpers"underexportswith both ESM and CJS entry points (and type definitions).LGTM.
packages/runtime/src/helpers.ts (1)
1-1: Clean re-export implementation.The re-export of the
sqlfunction from kysely is correctly implemented and aligns with the new export entry in package.json.packages/common-helpers/src/index.ts (1)
1-1: Consistent export addition.The new export for the find-up module follows the established pattern and maintains proper alphabetical ordering.
.prettierignore (1)
1-2: Appropriate ignore patterns for generated code.The ignore patterns correctly exclude generated files and schema files from Prettier formatting, which prevents conflicts with auto-generated code.
packages/runtime/package.json (3)
3-3: Consistent version bump.The version increment to
3.0.0-alpha.10aligns with the alpha release cycle.
42-51: Proper export configuration for helpers module.The new "./helpers" export entry is correctly configured with appropriate import/require paths for both types and modules, matching the new helpers.ts file.
76-77: Appropriate dependency management changes.Moving zod from peerDependencies to dependencies makes sense if it's now a direct dependency, and the uuid version update is reasonable.
packages/cli/test/generate.test.ts (1)
45-58: Well-structured test for package.json configuration support.The test comprehensively validates the new functionality by creating a custom directory structure, configuring schema and output paths in package.json, and verifying the generate command respects these settings. This properly covers the integration between the CLI configuration reading and the generate command.
packages/cli/src/actions/generate.ts (3)
7-7: Good addition of the configuration utility import.The import of
getPkgJsonConfigenables the new configuration-based path resolution functionality.
23-23: Clean refactoring to use helper function.Extracting the output path logic into a dedicated helper function improves code organization and readability.
58-68: Correct implementation of configuration precedence.The
getOutputPathfunction implements the proper precedence order: command line options take priority, followed by package.json configuration, then default to schema directory. This provides a logical hierarchy for configuration resolution.packages/cli/src/actions/action-utils.ts (3)
1-1: Appropriate import for upward file search utility.The
findUpimport enables the package.json discovery functionality needed for configuration-based schema resolution.
17-23: Proper integration of package.json configuration in schema resolution.The schema file resolution now correctly prioritizes package.json configuration after explicit file arguments but before default locations. The error handling maintains consistency with the existing pattern.
64-85: Robust implementation of package.json configuration reading.The
getPkgJsonConfigfunction handles all the necessary cases:
- Graceful handling when package.json is not found
- Safe JSON parsing with error recovery
- Proper path resolution relative to package.json location
- Type-safe configuration extraction
The implementation follows good practices with defensive programming and appropriate error handling.
packages/common-helpers/src/find-up.ts (2)
7-7: Well-designed conditional return type.The
FindUpResulttype correctly uses conditional types to provide type safety for both single and multiple result scenarios.
21-34: Correct implementation of upward directory traversal.The
findUpfunction properly implements the following:
- Input validation to ensure at least one valid name is provided
- Efficient search by stopping at first match when multiple is false
- Proper termination condition when reaching filesystem root
- Correct path resolution and recursive traversal
- Type-safe return value casting
The logic handles both single and multiple result modes correctly, and the recursive approach with proper termination ensures it won't infinite loop.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
zoddependency to version 4.