-
-
Notifications
You must be signed in to change notification settings - Fork 11
merge dev to main (v3.0.0-beta.15) #354
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
WalkthroughThis PR introduces version 3.0.0-beta.15 across multiple packages, establishes a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude finished @ymc9's task —— View job Pull Request ReviewReviewing the merge from dev to main (v3.0.0-beta.15). This appears to be a significant release with architectural changes. Review Tasks
Review SummaryThis PR represents a well-structured release preparation with significant architectural improvements. Here's my detailed analysis: ✅ Strengths1. Schema Package Extraction
2. Version Consistency
3. GitHub Workflow Enhancement
🔍 Code Quality ObservationsImport Updates (
Client Implementation (
|
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 extracts schema-related type definitions from the @zenstackhq/sdk package into a new standalone @zenstackhq/schema package and bumps all package versions from 3.0.0-beta.14 to 3.0.0-beta.15. The refactoring reduces dependencies between packages by isolating runtime schema types.
- Created new
@zenstackhq/schemapackage with schema type definitions and expression utilities - Updated import paths from
@zenstackhq/sdk/schemato@zenstackhq/schemaacross the codebase - Added GitHub workflow to automatically update sample repositories with new package releases
Reviewed Changes
Copilot reviewed 36 out of 39 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
packages/schema/** |
New package containing schema type definitions, expression types, and utility functions |
packages/sdk/package.json, packages/sdk/tsup.config.ts |
Removed /schema export entry point |
packages/orm/** |
Updated to depend on @zenstackhq/schema instead of @zenstackhq/sdk for schema types |
packages/testtools/**, tests/e2e/** |
Updated import paths to use new @zenstackhq/schema package |
pnpm-lock.yaml |
Updated workspace dependencies to include new schema package |
.github/workflows/update-samples.yml |
New workflow for automatically updating sample repositories |
package.json, packages/*/package.json |
Version bumped to 3.0.0-beta.15 |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
packages/schema/src/index.ts:3
- The
OperandExpressiontype from kysely that was previously exported (as shown in the oldpackages/orm/src/schema/index.ts) is no longer exported from this package. If consumers needOperandExpression, they'll need to import it directly from kysely, which could be a breaking change. Consider documenting this in migration notes or re-exporting it here for compatibility.
packages/schema/src/expression-utils.ts:69 - Redundant property assignment. The
receiverparameter can be shortened to justreceiverusing ES6 object property shorthand syntax.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/update-samples.yml (1)
64-96: Consider package manager detection and error handling.The workflow assumes all sample repositories use npm, but they might use yarn, pnpm, or bun. Additionally, there's no error handling if
npm installornpx zen generatefail.Consider these improvements:
- Detect package manager before running install commands:
+ - name: Detect package manager + if: steps.check-package.outputs.exists == 'true' + id: detect-pm + run: | + if [ -f "pnpm-lock.yaml" ]; then + echo "manager=pnpm" >> $GITHUB_OUTPUT + elif [ -f "yarn.lock" ]; then + echo "manager=yarn" >> $GITHUB_OUTPUT + elif [ -f "bun.lockb" ]; then + echo "manager=bun" >> $GITHUB_OUTPUT + else + echo "manager=npm" >> $GITHUB_OUTPUT + fi
- Add error handling to the update step:
- name: Update @zenstackhq packages to next if: steps.check-package.outputs.exists == 'true' run: | + set -e # Exit on error # Get all @zenstackhq packages in the repo PACKAGES=$(cat package.json | jq -r ' [.dependencies, .devDependencies] | add | to_entries | map(select(.key | startswith("@zenstackhq/"))) | map(.key) | .[] ') if [ -z "$PACKAGES" ]; then echo "No @zenstackhq packages found in ${{ matrix.repo }}" exit 0 fi echo "Updating packages in ${{ matrix.repo }}:" echo "$PACKAGES" # Update each package to next tag for pkg in $PACKAGES; do echo "Updating $pkg to next" - npm install "$pkg@next" + npm install "$pkg@next" || { echo "Failed to install $pkg"; exit 1; } done # Finally run zenstack generate - npx zen generate + npx zen generate || { echo "Failed to run zen generate"; exit 1; }
- Update the cache setup to use detected package manager:
- name: Use Node.js if: steps.check-package.outputs.exists == 'true' uses: actions/setup-node@v4 with: node-version: 20.x - cache: 'npm' + cache: ${{ steps.detect-pm.outputs.manager }}
📜 Review details
Configuration used: Path: .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 (36)
.github/workflows/update-samples.yml(1 hunks)package.json(1 hunks)packages/cli/package.json(1 hunks)packages/common-helpers/package.json(1 hunks)packages/config/eslint-config/package.json(1 hunks)packages/config/typescript-config/package.json(1 hunks)packages/config/vitest-config/package.json(1 hunks)packages/create-zenstack/package.json(1 hunks)packages/dialects/sql.js/package.json(1 hunks)packages/language/package.json(1 hunks)packages/orm/package.json(2 hunks)packages/orm/src/client/client-impl.ts(1 hunks)packages/orm/src/client/contract.ts(1 hunks)packages/orm/src/client/crud/validator/utils.ts(1 hunks)packages/orm/src/schema.ts(1 hunks)packages/orm/src/schema/auth.ts(0 hunks)packages/orm/src/schema/index.ts(0 hunks)packages/orm/tsup.config.ts(1 hunks)packages/plugins/policy/package.json(1 hunks)packages/schema/eslint.config.js(1 hunks)packages/schema/package.json(1 hunks)packages/schema/src/expression-utils.ts(1 hunks)packages/schema/src/index.ts(1 hunks)packages/schema/tsconfig.json(1 hunks)packages/schema/tsup.config.ts(1 hunks)packages/sdk/package.json(1 hunks)packages/sdk/tsup.config.ts(0 hunks)packages/server/package.json(1 hunks)packages/tanstack-query/package.json(1 hunks)packages/testtools/package.json(2 hunks)packages/testtools/src/schema.ts(1 hunks)packages/zod/package.json(1 hunks)samples/blog/package.json(1 hunks)tests/e2e/orm/schemas/basic/helper.ts(1 hunks)tests/e2e/package.json(2 hunks)tests/regression/package.json(1 hunks)
💤 Files with no reviewable changes (3)
- packages/orm/src/schema/auth.ts
- packages/sdk/tsup.config.ts
- packages/orm/src/schema/index.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
packages/testtools/src/schema.ts
🧬 Code graph analysis (1)
packages/orm/src/client/contract.ts (2)
packages/schema/src/schema.ts (2)
SchemaDef(10-18)GetModels(109-109)packages/orm/src/client/crud-types.ts (1)
ModelResult(135-179)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cleanup artifacts
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: build-test (20.x, postgresql)
🔇 Additional comments (27)
packages/tanstack-query/package.json (1)
3-3: Version bump looks good.Consistent with the broader 3.0.0-beta.15 release across the monorepo. Workspace dependency remains unchanged.
packages/zod/package.json (1)
3-3: Version bump is consistent.Updated to 3.0.0-beta.15 as part of the release. No dependency or export changes.
packages/config/vitest-config/package.json (1)
4-4: Version bump is correct.Updated to 3.0.0-beta.15. Exports and configuration remain stable.
packages/language/package.json (1)
4-4: Version bump is correct.Updated to 3.0.0-beta.15. Public exports and API surface remain unchanged. DevDependencies (vitest-config) properly tracked.
packages/dialects/sql.js/package.json (1)
3-3: Version bump is good.Updated to 3.0.0-beta.15. All exports and dependencies remain stable.
packages/schema/tsconfig.json (1)
1-7: New schema package TypeScript config looks good.Follows the established pattern by extending the shared base TypeScript config. Disabling
noUnusedLocalsis reasonable during package consolidation. The include glob properly targets sources.packages/server/package.json (1)
3-3: Version bump is correct.Updated to 3.0.0-beta.15. All framework-specific exports and dependencies remain stable.
packages/cli/package.json (1)
6-6: Version bump is correct.Updated to 3.0.0-beta.15. CLI bin entries and all dependencies remain stable. Workspace references properly maintained.
packages/orm/src/client/crud/validator/utils.ts (1)
2-10: No issues found—import migration is complete and correct.All seven imported types (
AttributeApplication,BinaryExpression,CallExpression,Expression,FieldExpression,MemberExpression,UnaryExpression) are properly re-exported from the@zenstackhq/schemapackage entry point viapackages/schema/src/index.ts. Type definitions maintain compatible structures, the old import path has been completely removed from the codebase, and the dependency inpackages/orm/package.jsonhas been correctly updated.packages/orm/package.json (1)
64-76: No issues found—schema export consolidation is properly implemented.The ORM's
./schemaexport correctly re-exports from@zenstackhq/schemaviapackages/orm/src/schema.ts, with no circular dependencies or duplication. The consolidation follows a clean pattern: the dependency is properly added, schema types are made available through the export map, and no breaking changes are introduced.package.json (1)
3-3: LGTM! Version bump to beta.15.The version bump is clean and consistent with the PR objectives.
packages/schema/eslint.config.js (1)
1-4: LGTM! Standard ESLint configuration.The ESLint config follows the standard pattern used across the monorepo packages.
packages/orm/src/schema.ts (1)
1-2: LGTM! Clean schema re-export module.This module effectively consolidates schema exports from the new
@zenstackhq/schemapackage and Kysely'sOperandExpressiontype, providing a convenient single import point for ORM consumers.packages/orm/tsup.config.ts (1)
6-6: LGTM! Build entry updated to match new schema structure.The schema entry point has been correctly updated from the nested
src/schema/index.tsto the flatsrc/schema.tsmodule, aligning with the schema refactoring.packages/common-helpers/package.json (1)
3-3: LGTM! Version bump to beta.15.Consistent version update across the monorepo.
packages/sdk/package.json (2)
3-3: LGTM! Version bump to beta.15.
18-29: Verification confirms safe and complete refactoring.Migration to
@zenstackhq/schemais complete:
- ✓ No imports from old
@zenstackhq/sdk/schemapath found anywhere- ✓ New
@zenstackhq/schemapackage actively used in 4 files (orm, testtools, e2e tests)- ✓ Schema generation utilities preserved in main SDK export
- ✓ No broken references or orphaned code
The architectural consolidation is sound and poses no risk to the codebase.
packages/schema/tsup.config.ts (1)
1-13: LGTM! Standard build configuration for new schema package.The tsup configuration follows the established patterns used across the monorepo, with appropriate settings for dual-format builds (CJS/ESM), source maps, and type declarations.
packages/orm/src/client/client-impl.ts (1)
15-21: Refactoring verified as complete and correct.The verification confirms:
- AuthType is correctly imported from './contract' in client-impl.ts (line 16)
- AuthType is properly exported from contract.ts (line 808)
- No remaining references to the old schema/auth path exist in the codebase
- All usages throughout client-impl.ts and contract.ts are consistent with the new import path
packages/orm/src/client/contract.ts (1)
808-813: LGTM! AuthType relocation is clean.The inline definition of
AuthTypemaintains the same conditional logic as before. The type correctly handles three scenarios: generic string models, typed auth models, and invalid configurations.packages/schema/package.json (1)
1-37: LGTM! Well-structured package manifest.The new
@zenstackhq/schemapackage is properly configured with dual module exports (ESM/CJS), appropriate build scripts, and minimal dependencies.tests/e2e/orm/schemas/basic/helper.ts (1)
1-1: LGTM! Import path correctly updated.The import path has been updated to use the new
@zenstackhq/schemapackage, aligning with the schema consolidation effort.packages/schema/src/expression-utils.ts (1)
14-18: LGTM! Improved import clarity and documentation.The more explicit import path (
./expressionvs.) and the addition of JSDoc documentation improve code maintainability.packages/testtools/src/schema.ts (1)
3-3: LGTM! Import path correctly updated.The import path has been updated to use the new
@zenstackhq/schemapackage, maintaining consistency with the schema consolidation effort across the codebase.tests/e2e/package.json (1)
3-3: LGTM! Version and dependency updates are consistent.The version bump to 3.0.0-beta.15 and the addition of the
@zenstackhq/schemadependency align with the schema consolidation effort in this PR.Also applies to: 18-18
packages/testtools/package.json (1)
3-3: LGTM! Version and dependency updates are consistent.The version bump to 3.0.0-beta.15 and the addition of the
@zenstackhq/schemadependency support the import path changes insrc/schema.ts.Also applies to: 36-36
packages/schema/src/index.ts (1)
1-3: LGTM! Clean refactoring of public exports.The addition of
export * from './expression-utils'appropriately uses a non-type export (unlike lines 1 and 3), which is correct for a utilities module that likely exports runtime functions. This refactoring consolidates expression-related utilities into a local module rather than re-exporting from external dependencies.
Summary by CodeRabbit
Release Notes - v3.0.0-beta.15
Chores
Automation