merge dev to main (v3.0.0-alpha.23) #163
Conversation
* fix: improve plugin config typing * update
Co-authored-by: ymc9 <[email protected]>
|
Claude finished @ymc9's task —— View job PR Review: merge dev to main (v3.0.0-alpha.23) ✅Review Checklist:
Analysis SummaryThis PR merges dev to main for v3.0.0-alpha.23 release. The changes include:
Detailed Review✅ Version Consistency ✅ Code Quality & Bug Fixes Fixed Property Naming Inconsistency (
✅ Security & Performance
✅ Test Coverage
✅ Best Practices
Recommendation: APPROVED ✅This is a solid release merge with important bug fixes:
The changes improve API consistency and fix potential bugs without introducing any regressions. Ready to merge to main. |
WalkthroughRenames mutation interception flags to pluralized forms across runtime code, plugin types, tests, and README. Updates executor logic to use plural fields for pre/post-mutation entity loading. Bumps versions to 3.0.0-alpha.23 across root, packages, samples, and tests. No other functional changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
✨ 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
|
There was a problem hiding this comment.
Pull Request Overview
This PR merges the dev branch to main, primarily bumping the version number from 3.0.0-alpha.22 to 3.0.0-alpha.23 across all packages and fixing API naming inconsistencies in mutation lifecycle hooks.
- Version bump from
3.0.0-alpha.22to3.0.0-alpha.23across all packages - API naming fix: renamed
loadBeforeMutationEntityandloadAfterMutationEntityto their plural forms - Updated corresponding test files and documentation to reflect the API changes
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Root package version bump |
| packages/*/package.json | Version updates for all package modules |
| packages/runtime/src/client/plugin.ts | API interface updates for mutation lifecycle hooks |
| packages/runtime/src/client/executor/zenstack-query-executor.ts | Implementation updates to use new plural property names |
| packages/runtime/test/plugin/mutation-hooks.test.ts | Test updates to use new API naming |
| README.md | Documentation updates showing new API usage |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
packages/runtime/src/client/executor/zenstack-query-executor.ts (2)
71-83: Don't clobber existing RETURNING and avoid changing the user-visible result shapeOverwriting any existing returning clause with SELECT * can change the rows shape returned to callers. If the original query specified a returning projection (or none), this will silently alter behavior. Only add returning when it's absent.
Apply this diff to preserve an existing returning:
- if ( - (InsertQueryNode.is(queryNode) || UpdateQueryNode.is(queryNode)) && - mutationInterceptionInfo?.loadAfterMutationEntities - ) { - // need to make sure the query node has "returnAll" - // for insert and update queries - queryNode = { - ...queryNode, - returning: ReturningNode.create([SelectionNode.createSelectAll()]), - }; - } + if ( + (InsertQueryNode.is(queryNode) || UpdateQueryNode.is(queryNode)) && + mutationInterceptionInfo?.loadAfterMutationEntities + ) { + // ensure the query node returns rows for insert/update, but don't clobber an existing returning + if (!queryNode.returning) { + queryNode = { + ...queryNode, + returning: ReturningNode.create([SelectionNode.createSelectAll()]), + }; + } + }Also, Line 100 still has a TODO about trimming the result to the original query node. Consider addressing it here to prevent returning a different shape than the original query intended. I can help sketch a minimal normalizer if needed.
316-326: beforeEntityMutation payload likely uses the wrong property nameYou’re passing
entitiestobeforeEntityMutation, while the rest of the file (and the pluralization change) suggests the new field should bebeforeMutationEntities. If the type has been updated (as per the PR), this will break consumers at compile-time or silently omit data at runtime.Apply this diff to align naming:
- await plugin.beforeEntityMutation({ - model: mutationModel, - action: mutationInterceptionInfo.action, - queryNode, - entities: mutationInterceptionInfo.beforeMutationEntities, - }); + await plugin.beforeEntityMutation({ + model: mutationModel, + action: mutationInterceptionInfo.action, + queryNode, + beforeMutationEntities: mutationInterceptionInfo.beforeMutationEntities, + });If the public plugin API intentionally still uses
entitiesfor the “before” hook, please confirm and keep it consistent across docs/tests.
🧹 Nitpick comments (1)
packages/runtime/src/client/executor/zenstack-query-executor.ts (1)
286-288: Normalize plugin flags to booleans and avoid undefined propagationUsing the ||= operator works, but the fields can remain undefined when no plugin sets them. Normalize to booleans for clarity and type safety.
Apply this diff:
- result.loadBeforeMutationEntities ||= filterResult.loadBeforeMutationEntities; - result.loadAfterMutationEntities ||= filterResult.loadAfterMutationEntities; + result.loadBeforeMutationEntities = + !!result.loadBeforeMutationEntities || !!filterResult.loadBeforeMutationEntities; + result.loadAfterMutationEntities = + !!result.loadAfterMutationEntities || !!filterResult.loadAfterMutationEntities;Optionally, initialize the fields where
resultis declared so they’re always boolean:- const result: MutationInterceptionFilterResult = { - intercept: false, - }; + const result: MutationInterceptionFilterResult = { + intercept: false, + loadBeforeMutationEntities: false, + loadAfterMutationEntities: false, + };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
README.md(1 hunks)package.json(1 hunks)packages/cli/package.json(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/executor/zenstack-query-executor.ts(3 hunks)packages/runtime/src/client/plugin.ts(2 hunks)packages/runtime/test/plugin/mutation-hooks.test.ts(5 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)samples/blog/package.json(1 hunks)tests/e2e/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{packages,samples,tests}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/sdk/package.jsonsamples/blog/package.jsonpackages/create-zenstack/package.jsonpackages/testtools/package.jsonpackages/runtime/package.jsonpackages/tanstack-query/package.jsonpackages/dialects/sql.js/package.jsonpackages/vitest-config/package.jsontests/e2e/package.jsonpackages/ide/vscode/package.jsonpackages/runtime/test/plugin/mutation-hooks.test.tspackages/eslint-config/package.jsonpackages/cli/package.jsonpackages/typescript-config/package.jsonpackages/zod/package.jsonpackages/language/package.jsonpackages/runtime/src/client/plugin.tspackages/common-helpers/package.jsonpackages/runtime/src/client/executor/zenstack-query-executor.ts
tests/e2e/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
E2E tests are in
tests/e2e/directory
Files:
tests/e2e/package.json
🧠 Learnings (2)
📚 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: Plugin Architecture: Runtime plugins for query interception and entity mutation hooks
Applied to files:
packages/runtime/src/client/plugin.tspackages/runtime/src/client/executor/zenstack-query-executor.ts
⏰ 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 (30)
packages/language/package.json (2)
4-4: Version bump looks correct and aligned with the PR scope.
This aligns with the repo-wide bump to 3.0.0-alpha.23.
4-4: ✅ Monorepo versions all at 3.0.0-alpha.23 with no stale referencesAll package.json files declare version 3.0.0-alpha.23, and the search for “3.0.0-alpha.22” returned no hits. No further action needed.
packages/common-helpers/package.json (1)
3-3: LGTM: simple version bump.
No other fields changed; consistent with monorepo-wide bump.packages/dialects/sql.js/package.json (1)
3-3: LGTM: version updated as expected.
Peer deps remain unchanged and compatible; nothing else modified.tests/e2e/package.json (1)
3-3: LGTM: e2e package version bump only.
No script or dependency changes; aligns with the overall version update.samples/blog/package.json (1)
3-3: LGTM: sample app version bump.
No functional changes; dependencies still use workspace references.packages/cli/package.json (1)
6-6: Monorepo Pre-release Version Consistency VerifiedAll automated checks passed for version 3.0.0-alpha.23 across the repository:
- 13 @zenstackhq/* packages (and create-zenstack) are correctly bumped.
- All internal @zenstackhq/* dependencies use
workspace:*.- No deprecated
loadBeforeMutationEntityorloadAfterMutationEntityflags remain.LGTM—no further action required.
packages/tanstack-query/package.json (1)
3-3: LGTM: version bump only.No API or config changes; aligns with the monorepo release.
packages/create-zenstack/package.json (1)
3-3: LGTM: version bump aligns with the release.CLI scaffolder metadata-only update; no other changes.
packages/zod/package.json (1)
3-3: LGTM: version bump only.Private package; dependencies unchanged; consistent with repo-wide bump.
packages/sdk/package.json (1)
3-3: LGTM: version bump only.Exports, deps, and build scripts unchanged; consistent with the release train.
packages/vitest-config/package.json (1)
4-4: Version bump looks goodMetadata-only change. No concerns.
package.json (1)
3-3: Root version bump approved; monorepo consistency verifiedVerified across all package.json files:
- All versions updated to 3.0.0-alpha.23
- No lingering 3.0.0-alpha.22 references
- All internal @zenstackhq/* dependencies use the
workspace:protocolReady to merge.
packages/runtime/package.json (1)
3-3: Runtime package version bump aligns with related API changesGiven the pluralization of mutation interception fields elsewhere in the PR, bumping to alpha.23 is appropriate. No further action in this file.
packages/typescript-config/package.json (1)
3-3: LGTM: metadata-only bumpPrivate package version updated; no impact on consumers.
packages/testtools/package.json (1)
3-3: Test tools version bump approvedNo script/export changes. Ensure peer dependency ranges remain satisfied in the monorepo CI matrix.
If helpful, extend the earlier script to assert that all packages depending on better-sqlite3 or pg declare compatible versions, by grepping for
"better-sqlite3"and"pg"in package.json and reviewing the ranges.packages/ide/vscode/package.json (1)
4-4: Version bump aligned with monorepo pattern.The version upgrade to 3.0.0-alpha.23 is consistent with the overall version uplift across the monorepo.
packages/eslint-config/package.json (1)
3-3: Version bump aligned with monorepo pattern.The version upgrade to 3.0.0-alpha.23 is consistent with the overall version uplift across the monorepo.
packages/runtime/src/client/plugin.ts (4)
42-42: Field name pluralization improves API consistency.The rename from
loadBeforeMutationEntitytoloadBeforeMutationEntitiesbetter reflects that multiple entities can be affected by a single mutation operation.
47-47: Field name pluralization improves API consistency.The rename from
loadAfterMutationEntitytoloadAfterMutationEntitiesbetter reflects that multiple entities can be affected by a single mutation operation.
152-152: Documentation correctly updated to reference plural field.The parameter documentation now properly references
loadBeforeMutationEntitieswhich aligns with the field name change.
159-162: Documentation correctly updated to reference plural fields.The parameter documentation now properly references both
loadBeforeMutationEntitiesandloadAfterMutationEntitieswhich aligns with the field name changes.README.md (1)
354-356: Documentation example updated to reflect API changes.The code sample correctly demonstrates the new plural field names
loadBeforeMutationEntitiesandloadAfterMutationEntitiesin the mutation interception filter configuration.packages/runtime/test/plugin/mutation-hooks.test.ts (5)
115-115: Test updated for pluralized API.The test correctly uses the new
loadBeforeMutationEntitiesfield name, maintaining the same boolean functionality while aligning with the updated API.
162-162: Test updated for pluralized API.The test correctly uses the new
loadAfterMutationEntitiesfield name, maintaining the same boolean functionality while aligning with the updated API.
208-208: Test updated for pluralized API.The test correctly uses the new
loadAfterMutationEntitiesfield name for multi-entity mutation testing.
266-266: Test updated for pluralized API.The test correctly uses the new
loadAfterMutationEntitiesfield name for nested mutation testing.
353-354: Test updated for pluralized API.The test correctly uses both new field names
loadBeforeMutationEntitiesandloadAfterMutationEntitiesfor comprehensive mutation lifecycle testing.packages/runtime/src/client/executor/zenstack-query-executor.ts (2)
292-294: LGTM: pre-mutation entity loading now correctly gated by plural flagThe condition now aligns with the new pluralized flag and applies only to update/delete, which is appropriate for pre-mutation reads.
352-361: After-mutation entities derivation: consider dialects without RETURNINGThis relies on rows being present for insert/update. For dialects that don’t support RETURNING, the adapter may ignore it and
rowscould be empty. If that’s a supported configuration, add a fallback (e.g., reloading by PKs when available) or guard by dialect capability.Would you confirm whether all supported dialects in this runtime guarantee RETURNING semantics for insert/update when requested? If not, I can propose a guarded fallback path.
Summary by CodeRabbit