-
-
Notifications
You must be signed in to change notification settings - Fork 12
chore: update zod, add several helpers, CLI loads config from pkg.json #84
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
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces new configuration resolution logic for CLI schema and output paths using Changes
Poem
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 upgrades Zod to v4, adds helper exports, and extends the CLI to load schema/output configuration from package.json.
- Bump Zod dependency and update related imports
- Introduce shared
findUphelper and re-export SQL helper - Enhance CLI
generateaction withgetPkgJsonConfigandgetOutputPath
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Updated Zod version constraint to ^4.0.0 |
| packages/zod/src/types.ts | Switched import from 'zod/v4' to 'zod' |
| packages/zod/src/index.ts | Switched import from 'zod/v4' to 'zod' |
| packages/runtime/tsup.config.ts | Added helpers entry for bundling runtime helpers |
| packages/runtime/src/helpers.ts | Exported sql helper from Kysely |
| packages/runtime/src/client/crud/validator.ts | Switched import from 'zod/v4' to 'zod' |
| packages/runtime/src/client/contract.ts | Added TransactionClientContract type |
| packages/runtime/package.json | Exposed ./helpers entrypoint and moved Zod to dependencies |
| packages/common-helpers/src/index.ts | Re-exported new find-up utility |
| packages/common-helpers/src/find-up.ts | Implemented findUp file resolution helper |
| packages/cli/src/actions/generate.ts | Added getOutputPath leveraging package.json config |
| packages/cli/src/actions/action-utils.ts | Added schema/output config loader getPkgJsonConfig |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (3)
packages/common-helpers/src/find-up.ts:18
- The
@paramtag is missing the parameter name. Change to@param result An array of strings representing the accumulated results used in multiple resultsfor proper JSDoc.
* @param An array of strings representing the accumulated results used in multiple results
packages/cli/src/actions/action-utils.ts:64
- New logic for loading
package.jsonconfig is added but there are no corresponding tests. Consider adding unit tests forgetPkgJsonConfigto cover scenarios with and without azenstacksection.
export function getPkgJsonConfig(startPath: string) {
packages/cli/src/actions/action-utils.ts:1
- The file uses
fs,path, andCliErrorbut none are imported. Addimport fs from 'node:fs';,import path from 'node:path';, and import or defineCliErrorto avoid reference errors.
import { findUp } from '@zenstackhq/common-helpers';
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/common-helpers/src/find-up.ts (1)
9-20: Clarify JSDoc documentation for the multiple parameter.The JSDoc comment on line 17 is misleading about the
multipleparameter's purpose.- * @param multiple A boolean flag indicating whether to search for multiple levels. Useful for finding node_modules directories... + * @param multiple A boolean flag indicating whether to return all found paths or just the first oneThe parameter controls whether to return single or multiple results, not whether to search multiple levels (the function always searches up to the root).
packages/runtime/package.json (1)
76-82: Re-classifyingzodfrom peer → runtime dep can cause duplication in consumer appsBy shipping
zodas a hard dependency:
- Projects that already depend on a different
zod@4patch level will get two copies unless their package-manager hoists/dedupes.- Tree-shaking doesn’t help because Zod is side-effect free but fully included.
Consider the hybrid approach adopted by many libraries:
"dependencies": { ... - "zod": "catalog:" + // keep no direct dependency on zod }, "peerDependencies": { ... + "zod": "^4.0.0", }, "peerDependenciesMeta": { + "zod": { "optional": false }, ... }This preserves a single Zod instance and lets the host control the version, while still signalling a strict requirement.
📜 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 (13)
packages/cli/src/actions/action-utils.ts(3 hunks)packages/cli/src/actions/generate.ts(3 hunks)packages/cli/test/generate.test.ts(1 hunks)packages/common-helpers/src/find-up.ts(1 hunks)packages/common-helpers/src/index.ts(1 hunks)packages/runtime/package.json(2 hunks)packages/runtime/src/client/contract.ts(1 hunks)packages/runtime/src/client/crud/validator.ts(1 hunks)packages/runtime/src/helpers.ts(1 hunks)packages/runtime/tsup.config.ts(1 hunks)packages/zod/src/index.ts(1 hunks)packages/zod/src/types.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
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)
packages/runtime/src/client/contract.ts (1)
packages/sdk/src/schema/schema.ts (1)
SchemaDef(10-17)
packages/cli/test/generate.test.ts (1)
packages/cli/test/utils.ts (2)
createProject(12-18)runCli(20-23)
⏰ 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 (16)
packages/common-helpers/src/index.ts (1)
1-1: LGTM: Clean export addition following established pattern.The new export for the find-up utility follows the consistent pattern used throughout the index file and makes the utility available through the package's public API.
packages/cli/src/actions/generate.ts (3)
7-7: LGTM: Clean import addition for new configuration functionality.The import properly includes the new
getPkgJsonConfigfunction needed for package.json configuration support.
23-23: LGTM: Good refactoring to extract output path logic.The refactoring improves code organization by extracting the output path determination logic into a dedicated helper function.
58-68: LGTM: Well-structured priority hierarchy for output path determination.The function correctly implements a three-tier priority system:
- Explicit
--outputoption (highest priority)- Package.json configuration (fallback)
- Schema file directory (default)
This provides a logical and user-friendly configuration hierarchy.
packages/cli/test/generate.test.ts (1)
45-58: LGTM: Comprehensive test coverage for package.json configuration.The test case properly validates the end-to-end functionality of the new configuration resolution mechanism by:
- Setting up a custom directory structure
- Configuring package.json with zenstack settings
- Verifying the CLI respects the configuration without explicit arguments
This ensures the feature works correctly and provides good regression protection.
packages/cli/src/actions/action-utils.ts (2)
1-1: LGTM: Proper import for the new findUp utility.The import correctly includes the findUp function needed for upward directory traversal.
17-23: LGTM: Good integration of package.json configuration into schema resolution.The modification properly integrates the new configuration mechanism as a fallback option between explicit file specification and default locations. The error handling is consistent with the existing pattern.
packages/common-helpers/src/find-up.ts (1)
1-7: LGTM: Clean imports and well-designed conditional type.The imports are appropriate and the
FindUpResultconditional type properly handles the different return types based on themultipleflag.packages/zod/src/types.ts (1)
2-2: Ensure mainzodexport provides all v4 types
This update replaces imports from'zod/v4'with the package root. Please confirm that the primaryzodentrypoint still exports all of the following v4 types:
- ZodBoolean
- ZodNumber
- ZodObject
- ZodString
- ZodUnknown
Location:
- packages/zod/src/types.ts:2
packages/zod/src/index.ts (1)
3-3: Verify Zod main export compatibilityPlease confirm that replacing
import { z, ZodType } from 'zod/v4';with
import { z, ZodType } from 'zod';provides identical
zandZodTypeAPIs. You may want to:
- Check your
package.jsonto ensurezodis at v4.x- Inspect the published
zodpackage exports (e.g., via a quick Node REPL or build)- Run the existing schema-related tests or a minimal import test to validate there are no missing or changed types
packages/runtime/src/client/crud/validator.ts (1)
5-5: Ensure Zod is pinned to v4 and main export supportsz/ZodTypePlease confirm that your central version-catalog (where
"zod": "catalog:"is defined) points to Zod v4, and that the unscopedzodpackage indeed exports bothzandZodType. All TS imports now use:
- packages/zod/src/types.ts
- packages/zod/src/index.ts
- packages/runtime/src/client/crud/validator.ts
packages/runtime/src/helpers.ts (1)
1-1: Build Configuration Verified forhelpersEntryThe
tsup.config.tsfile includes the following entry, confirming thatsrc/helpers.tswill be bundled as expected:• helpers: 'src/helpers.ts'
No further changes needed—approving the re-export as-is.
packages/runtime/src/client/contract.ts (1)
161-167: Excellent type safety improvement!The new
TransactionClientContracttype provides a restricted interface for transaction contexts by omitting methods that aren't supported during transactions. This prevents developers from accidentally calling unsupported methods and improves the developer experience with better compile-time checks.pnpm-workspace.yaml (1)
8-8: No remainingzod/v4imports detectedA repository-wide search for any
zod/v4references—both ES module imports and CommonJS requires—returned no results. It’s safe to proceed with merging the upgrade tozod@^4.packages/runtime/tsup.config.ts (1)
7-9: Newhelpersbundle entry looks correctEntry is wired into
tsupand placed adjacent toindex/schema.
No issues spotted. 👍packages/runtime/package.json (1)
42-51: Export map for./helpersmatches tsup output – LGTMThe map exposes both ESM/CJS and declaration files consistently with the other sub-paths.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores