fix: elide stubs in RPC result types so Promise<RpcStub<T>> returns match Promise<T> - #251
Draft
ndisidore wants to merge 1 commit into
Draft
fix: elide stubs in RPC result types so Promise<RpcStub<T>> returns match Promise<T>#251ndisidore wants to merge 1 commit into
ndisidore wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 05c32ea The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This comment was marked as low quality.
This comment was marked as low quality.
This comment was marked as low quality.
This comment was marked as low quality.
commit: |
ndisidore
force-pushed
the
feat/result-stub-elision
branch
from
August 19, 2026 02:20
4abffdd to
6362382
Compare
…atch Promise<T> A method declared to return Promise<RpcStub<T>> (or a property typed RpcStub<T>) previously produced RpcPromise<RpcStub<T>>, a broken type that could neither be awaited into an RpcStub<T> nor passed as a pipelined argument. Root cause: the string-keyed __RPC_TARGET_BRAND survives Provider<T>'s key mapping (which only excludes symbol | keyof StubBase), so Stub<T> of a branded or callable T structurally matches Stubable, shadowing the intended StubBase handling in both Stubify and Result. Result (and the capnweb-validate mirror, StubResult) now elides the stub wrapper for Stubable payloads, so declared-stub returns produce the exact same RpcPromise<T> as returning the payload directly — and the same type the RpcPromise constructor produces for a promised stub. Plain-interface stubs (RpcStub<PlainApi>) are deliberately NOT elided: RpcPromise<U> only awaits back to Stub<U> when U extends Stubable, so eliding those would change the awaited value from a stub to a stubified record, breaking today's working shape. Stubify's arms are reordered to [Promise, StubBase, Stubable] so existing stubs pass through instead of double-wrapping, and promise-backed stubs resolve through the Promise arm. The eliding arm intentionally omits an RpcCompatible<R> re-check: evaluating RpcCompatible<R> there recurses through its Stub<Stubable> member back into Result, tripping TS2615 circularity errors in mapped types. Anything matching our StubBase already had the constraint enforced where the stub type was formed. Documented but not fixed here: - A hand-written RpcPromise<RpcStub<T>> annotation still takes the RpcPromise alias's own Stubable arm (awaits to Stub<Stub<T>>); the library no longer produces that type, and fixing it inside the alias risks the Result/RpcPromise identity short-circuit. - Native workers-types stubs (Rpc.Stub) are unaffected: their StubBase lacks onRpcBroken, so they never match our StubBase<infer U>. This consistency gap is unchanged from before. - The RpcPromise constructor's Promise<T | Stub<T>> union also elides plain-interface stubs (new RpcPromise(Promise.resolve(plainIfaceStub)) infers RpcPromise<PlainApi>), which method returns deliberately do not. Pre-existing on the parent branch; worth raising in review. The eliding arms guard `any` payloads explicitly (IsAny): `[any] extends [Stubable]` is true, so without the guards RpcStub<any> results collapsed to UnknownResult (awaiting to unknown), and capnweb-validate's StubResult<any> collapsed to Promise<unknown> & StubBase<unknown> even for plain Promise<any> returns. Both now keep their stub surface.
ndisidore
force-pushed
the
feat/result-stub-elision
branch
from
August 19, 2026 11:23
6362382 to
05c32ea
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Declaring a method as
viaStub(): Promise<RpcStub<T>>now yields the sameRpcPromise<T>as declaringPromise<T>, matching whatnew RpcPromise(promise)infers. This resolves @kentonv's| Stub<T>question on #242. The two forms had to converge on eliding because the declared-stub form is broken on main:RpcPromise<RpcStub<T>>can't be passed as a pipelined argument and its awaited value isn't assignable toRpcStub<T>. Only pipelining typechecks, which is why nobody noticed.The culprit is the string-keyed
__RPC_TARGET_BRAND. It survivesProvider<T>'s key mapping, so a stub of anRpcTarget(or function) matchesStubableand gets wrapped a second time. Fixed by reorderingStubify's arms and adding eliding arms toResult, mirrored incapnweb-validate.Plain-interface stubs (
RpcStub<PlainApi>) are deliberately not elided:RpcPromise<U>only awaits back to a stub whenUis stubable, so eliding those would break the shape that already works. There's a pinning test for it.Not touched here: hand-written
RpcPromise<RpcStub<T>>annotations still misbehave (the library just never produces that type now), and the constructor still elides plain-interface stubs where method returns don't. Happy to add the same guard there if we want them identical.Stacked on #242 because the equivalence test needs the constructor from that PR.