Skip to content

refactor(orm): make ZenStackPromise compatible with standard Promise signature#2650

Merged
ymc9 merged 1 commit intodevfrom
refactor/zenstack-promise-signature
May 6, 2026
Merged

refactor(orm): make ZenStackPromise compatible with standard Promise signature#2650
ymc9 merged 1 commit intodevfrom
refactor/zenstack-promise-signature

Conversation

@ymc9
Copy link
Copy Markdown
Member

@ymc9 ymc9 commented May 6, 2026

Summary

  • Drop the Schema type parameter from ZenStackPromise and remove the public cb property from its type, so it is structurally assignable to a standard Promise<T>.
  • Update all CRUD/raw-query/transaction signatures in ClientContract and ClientImpl to the new single-parameter form.
  • The cb callback is still attached to the runtime object; ClientImpl.sequentialTransaction now reaches it via a typed getPromiseCallback helper that asserts its presence.

Test plan

  • pnpm build
  • pnpm test for the orm package

Fixes #2356

Summary by CodeRabbit

Release Notes

  • New Features

    • Added model and procedure filtering through slicing configuration in the dynamic client proxy
    • Enabled plugins to define computed fields on query results with recursive support for nested relations
  • Improvements

    • Enhanced transaction handling with improved robustness and clearer separation between transaction flows

…signature

Drop the `Schema` type parameter from `ZenStackPromise` and remove the
public `cb` property from its type, so the type is structurally
assignable to a standard `Promise<T>`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

📝 Walkthrough

Walkthrough

This PR refactors the ZenStackPromise type system to remove the Schema generic parameter and introduces lazy, on-demand execution with callback extraction. The changes propagate through contract signatures and implementation to support the new promise model and add extended-result-field support utilities.

Changes

Promise Type Simplification and Lazy Execution

Layer / File(s) Summary
Promise Type Definition
packages/orm/src/client/promise.ts
ZenStackPromise interface simplified from a dual-generic type to ZenStackPromise<T> extends Promise<T>. Implementation refactored to support lazy, on-demand callback execution with internal caching via valueToPromise() and exposed cb property.
Contract API Signatures
packages/orm/src/client/contract.ts
All return types updated to use the simplified ZenStackPromise<T> (removing Schema wrapper). Affected methods: $executeRaw, $executeRawUnsafe, $queryRaw, $queryRawUnsafe, $transaction, all CRUD operations (create, findMany, findUnique, update, delete, etc.), and aggregation methods (count, aggregate, groupBy, exists).
Implementation and Callback Extraction
packages/orm/src/client/client-impl.ts
Added getPromiseCallback() helper to robustly extract callbacks from ZenStackPromise objects. Updated $transaction overload signature to ZenStackPromise<any>[] and refactored sequentialTransaction to use the new callback extraction pattern. Added support for model/procedure slicing via isModelIncluded/isProcedureIncluded and extended-result-field utilities (EXT_RESULT_OPERATIONS, hasExtResultFieldDefs, collectExtResultFieldDefs, prepareArgsForExtResult, applyExtResult, applyExtResultToRow).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 A promise once wrapped in Schema's embrace,
Now dances alone through lazy-time space!
Callbacks extracted with care and with grace,
The ORM's heart beats at a swifter pace. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: refactoring ZenStackPromise to be compatible with standard Promise signatures by removing the Schema type parameter.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/zenstack-promise-signature

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 6, 2026

Claude finished @ymc9's task —— View job


Code Review Complete

I've thoroughly reviewed the ZenStackPromise refactor. This is a well-structured change that improves Promise compatibility while maintaining functionality. Here's my analysis:

Strengths

  1. Excellent Promise Compatibility: The refactor successfully makes ZenStackPromise<T> structurally assignable to standard Promise<T> by:

    • Removing the Schema type parameter (eliminates type incompatibility)
    • Hiding the public cb property from the interface (maintains structural compatibility)
    • Using interface with extends Promise<T> for proper inheritance
  2. Type Safety Preserved: The getPromiseCallback helper in client-impl.ts:271-276 provides runtime type safety:

    private getPromiseCallback(promise: ZenStackPromise<any>) {
        invariant((promise as any).cb, 'Invalid ZenStackPromise, missing cb property');
        const cb = (promise as any).cb;
        invariant(typeof cb === 'function', 'Invalid ZenStackPromise, cb property is not a function');
        return (promise as any).cb;
    }
  3. Consistent Type Signature Updates: All 23 CRUD operation signatures consistently updated from ZenStackPromise<Schema, T> to ZenStackPromise<T>

  4. Runtime Behavior Maintained: The cb property still exists at runtime and is accessible through type assertion

⚠️ Potential Concerns

  1. Breaking Change Warning: This is technically a breaking change for code that:

    • Directly accesses the cb property: promise.cb()
    • Uses type guards checking for the cb property
    • Relies on the Schema type parameter for type inference
  2. Type Assertion Dependency: The new approach relies on (promise as any).cb assertions, which could fail if promise objects are malformed

🔍 Security & Performance

  • Security: ✅ No security concerns. Runtime validation with invariant prevents unsafe access
  • Performance: ✅ Minimal impact. Type erasure at runtime means no performance degradation
  • Memory: ✅ No memory leaks. The cb property is still properly managed

🧪 Test Coverage Recommendations

The test plan looks good, but consider adding:

  • Tests for sequential transactions with the new getPromiseCallback helper
  • Type compatibility tests ensuring ZenStackPromise<T> assignments to Promise<T> work
  • Runtime error handling tests for malformed promise objects

📝 Minor Code Quality Notes

  1. Line formatting: Some long type signatures could benefit from better line breaks (e.g., contract.ts:415-419)
  2. Documentation: Consider updating JSDoc comments to reflect the new Promise-compatible interface

Overall Assessment

This is a high-quality refactor that achieves its goals effectively:

  • ✅ Makes ZenStackPromise structurally compatible with standard Promise
  • ✅ Maintains all existing functionality
  • ✅ Uses appropriate runtime safety measures
  • ✅ Follows consistent patterns throughout the codebase

The change improves TypeScript interoperability while preserving ZenStack's lazy execution semantics. The implementation is solid and the approach is well-reasoned.

Recommendation: Approve with awareness of breaking changes


Todo List:

  • Read and analyze changed files
  • Review Promise signature compatibility changes
  • Check for potential bugs or breaking changes
  • Evaluate performance and security considerations
  • Assess test coverage implications
  • Provide comprehensive feedback

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a 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

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/orm/src/client/contract.ts`:
- Around line 208-211: The array overload of $transaction declares it accepts
ZenStackPromise<any>[] but ClientImpl still expects the internal branded
transaction promise with the hidden callback carrier (p.cb), causing a
type/runtime mismatch; update the declaration so the array overload uses the
internal/branded transaction-promise type (the same shape produced by the
factory) instead of public ZenStackPromise, or alternatively move the hidden
callback off the magic string property and introduce/export a single internal
interface used by both the factory that creates promises and
ClientImpl::$transaction so both the compile-time signature and runtime check
reference the same branded type (refer to $transaction, ClientImpl,
ZenStackPromise and the hidden p.cb callback carrier when making the change).

In `@packages/orm/src/client/promise.ts`:
- Around line 14-16: The exported createZenStackPromise lost its generic result
type (T) and now returns ZenStackPromise<unknown>; restore a generic type
parameter (e.g., <T>) so the function signature remains
createZenStackPromise<T>(callback: (txClient?: ClientContract<any>) =>
Promise<T>): ZenStackPromise<T>, and update any internal uses/returns to
propagate that T while keeping the Schema parameter removal intact so callers
preserve their fulfilled type instead of being forced to unknown.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 48a454ed-891b-467c-b87d-27708b820b46

📥 Commits

Reviewing files that changed from the base of the PR and between d5e7900 and 8f6b8f4.

📒 Files selected for processing (3)
  • packages/orm/src/client/client-impl.ts
  • packages/orm/src/client/contract.ts
  • packages/orm/src/client/promise.ts

Comment thread packages/orm/src/client/contract.ts
Comment thread packages/orm/src/client/promise.ts
@ymc9 ymc9 merged commit 2ef5a99 into dev May 6, 2026
15 checks passed
@ymc9 ymc9 deleted the refactor/zenstack-promise-signature branch May 6, 2026 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make ZenStackPromise's typing compatible with Promise

1 participant