refactor(server): improve naming & types for hibernation apis - #685
refactor(server): improve naming & types for hibernation apis#685dinwwwh wants to merge 4 commits into
Conversation
…Iterator type by defining .then The client `await` the result, so the reflected result type will be more accurate by utilize .then
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes update the handling of the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant HibernationEventIterator
Client->>Server: Initiate request
Server->>HibernationEventIterator: Create instance (with hibernationCallback)
Server->>HibernationEventIterator: Call hibernationCallback(id)
HibernationEventIterator-->>Server: Callback executed
Server-->>Client: Respond
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code Graph Analysis (1)packages/shared/src/object.test.ts (2)
🪛 Biome (1.9.4)packages/standard-server/src/hibernation.ts[error] 22-22: Do not add then to a class. (lint/suspicious/noThenProperty) ⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (4)
✨ 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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Summary of Changes
Hello @unnoq, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request focuses on refining the experimental_HibernationEventIterator API to enhance type safety and improve the client-side experience. The primary changes involve making the hibernationCallback property mandatory and enabling the iterator to be directly awaitable by implementing the PromiseLike interface. Additionally, several naming conventions within the hibernation plugin have been updated for improved code clarity and maintainability.
Highlights
- Type Refinement for HibernationEventIterator: The
hibernationCallbackproperty onexperimental_HibernationEventIteratoris no longer optional (?). This indicates it's always expected to be present, simplifying usage and type checking across the codebase. - Promise-like HibernationEventIterator: The
experimental_HibernationEventIteratornow implementsPromiseLikeby providing athenmethod. This allows the iterator to beawaited directly by clients, which, as per the PR description, improves type accuracy for awaited results. - Naming Improvements in Hibernation Plugin: The
experimental_HibernationContextinterface was renamed toexperimental_HibernationPluginContext, and the associated internal context symbol was also renamed for better clarity and distinction within the server's hibernation plugin. - Code Simplification: The removal of optional chaining (
?.) forhibernationCallbackcalls in various files (tests, server peer) is a direct consequence of its new non-optional type, leading to cleaner code.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the hibernation APIs to improve naming and type safety, distinguishing server and client behavior by implementing a Promise-like then method. Key changes include:
- Introducing a then method in experimental_HibernationEventIterator to support awaiting.
- Removing optional chaining on hibernationCallback to enforce its presence.
- Renaming the hibernation context interface and symbol in the Hibernation plugin for increased clarity and consistency.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/standard-server/src/hibernation.ts | Adds a then method and changes hibernationCallback from optional to required. |
| packages/standard-server/src/hibernation.test.ts | Updates tests to call hibernationCallback without optional chaining. |
| packages/standard-server-peer/src/server.ts | Updates the usage of hibernationCallback to align with the new non-optional type. |
| packages/server/src/hibernation/plugin.ts | Renames the context interface and symbol for better consistency. |
| packages/durable-event-iterator/src/durable-object/handler.test.ts | Modifies tests to invoke hibernationCallback directly after the refactor. |
Comments suppressed due to low confidence (5)
packages/standard-server/src/hibernation.ts:11
- Since hibernationCallback is now required rather than optional, please update the documentation comments to reflect its non-optional nature and intended usage.
public readonly hibernationCallback: experimental_HibernationEventIteratorCallback,
packages/standard-server/src/hibernation.ts:22
- [nitpick] Consider adding an inline comment explaining the use of Promise.resolve(this) in the then method to clarify its intent and behavior for future maintainers.
then<TResult1 = AsyncIteratorClass<T, TReturn, TNext>, TResult2 = never>(
packages/standard-server-peer/src/server.ts:94
- Ensure that response.body always provides a valid hibernationCallback since the optional chaining has been removed and the callback is now mandatory.
response.body.hibernationCallback(id)
packages/server/src/hibernation/plugin.ts:6
- [nitpick] The renaming to experimental_HibernationPluginContext improves clarity; consider ensuring that all related documentation reflects this new naming convention.
export interface experimental_HibernationPluginContext {
packages/durable-event-iterator/src/durable-object/handler.test.ts:43
- Since hibernationCallback is now required, verify that the tests consistently assume its presence and remove any redundant optional chaining checks.
output.hibernationCallback(123)
There was a problem hiding this comment.
Code Review
This pull request refactors the hibernation APIs to improve naming and type safety. The renaming of experimental_HibernationContext to experimental_HibernationPluginContext and the context symbol makes the code clearer and less prone to collisions.
The core change is making experimental_HibernationEventIterator implement PromiseLike. This is a clever approach to distinguish the server-side type (with hibernationCallback) from the type inferred by consumers (a standard AsyncIteratorClass), improving type accuracy for clients of the RPC framework.
The changes are well-implemented and consistent across the affected files. I have one suggestion to improve maintainability by adding a comment to explain the PromiseLike pattern in experimental_HibernationEventIterator.
More templates
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/experimental-durable-event-iterator
@orpc/hey-api
@orpc/nest
@orpc/openapi
@orpc/openapi-client
@orpc/react
@orpc/react-query
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-aws-lambda
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/standard-server-peer
@orpc/svelte-query
@orpc/tanstack-query
@orpc/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
|
Using |
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
refactor(standard-server): distinguish server/client HibernationEventIterator type by defining .then
The client
awaitthe result, so the reflected result type will be more accurate by utilize .thenSummary by CodeRabbit
New Features
.then().Refactor
Tests