-
Notifications
You must be signed in to change notification settings - Fork 111
fix(types): Add default generic for untyped RPC sessions #63
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
base: main
Are you sure you want to change the base?
fix(types): Add default generic for untyped RPC sessions #63
Conversation
…ixes cloudflare#53) Add `= Empty` default type parameter to newHttpBatchRpcSession to match newWebSocketRpcSession and prevent "Type instantiation is excessively deep and possibly infinite" TypeScript error when calling without explicit type parameters. Before: newHttpBatchRpcSession<T extends Serializable<T>>(...) After: newHttpBatchRpcSession<T extends Serializable<T> = Empty>(...) This allows untyped usage like: const session = newHttpBatchRpcSession("http://example.com"); - Add comprehensive test coverage for type instantiation issue - Ensure typed sessions continue to work correctly - Validate that untyped sessions return proper RpcStub<Empty>
|
This function is not intended to be called without a type. If the type is unknown, you should use
|
|
Thanks for explaining the design philosophy - that makes perfect sense! The distinction between symmetrical vs client-only functions is a great insight. I tried both suggested alternatives, but they hit compilation issues:
This puts developers in a tough spot - they want to follow your guidance but can't due to the constraint design. But would you prefer an overload approach instead? Something like: // Helpful error when no type provided
export function newHttpBatchRpcSession(...): {
Error: 'newHttpBatchRpcSession requires a type argument. e.g. newHttpBatchRpcSession<MyApi>(...)'
};
// Properly typed version
export function newHttpBatchRpcSession<T extends Serializable<T>>(...): RpcStub<T>;
// Implementation
export function newHttpBatchRpcSession(
urlOrRequest: string | Request,
init?: RequestInit & RpcSessionOptions
): any {
return newHttpBatchRpcSessionImpl(urlOrRequest, init);
}This approach:
It doesn't solve the underlying recursive constraint issue, but it does limit the problematic usage patterns for clients and guides them toward the correct API usage. I believe the recursion stems from the Side note: I also noticed a potential issue where the client-side |
|
I suspect there is a more fundamental change we need to make to the types to fix these infinite recursion errors. This isn't the only place where it comes up. The types were based on types written for Workers RPC originally, and although I made a number of changes for Cap'n Web, I have to admit I still don't fully grok them.
I don't see the problem. |
Resolves Issue #53
This PR fixes a TypeScript compilation error that occurred when creating RPC sessions without an explicit type parameter.
Problem
When
newHttpBatchRpcSession()was called without a generic type, the TypeScript compiler would fail with the error:This happened because the function's generic
T extends Serializable<T>did not have a default type. When TypeScript tried to inferT, the recursive nature of theSerializable<T>constraint caused an infinite loop in the type checker.Reproduction
Solution
This PR aligns the behavior of
newHttpBatchRpcSessionwithnewWebSocketRpcSessionby adding= Emptyas the default for the generic type parameterT.This simple change provides a concrete type for TypeScript to use when one isn't specified, breaking the recursive loop and resolving the compilation error.
Result
After this fix, both HTTP and WebSocket sessions can be created without explicit types, providing a consistent developer experience.
Testing
Comprehensive test coverage has been added to
test/issue53.test.tsto:RpcStub<Empty>behavior