fix: elide stubs in RPC result types so Promise<RpcStub<T>> returns match Promise<T> - #251
Draft
ndisidore wants to merge 5 commits into
Draft
fix: elide stubs in RPC result types so Promise<RpcStub<T>> returns match Promise<T>#251ndisidore wants to merge 5 commits into
ndisidore wants to merge 5 commits into
Conversation
🦋 Changeset detectedLatest commit: bb7c807 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
2 times, most recently
from
August 19, 2026 11:23
6362382 to
05c32ea
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.
…ruction with result elision - Exclude __RPC_TARGET_BRAND (and workers brands in capnweb-validate) from Provider/ValidatedStub key mapping so the string-keyed brand no longer leaks onto stub surfaces. - Extract the Result stub elision into ElideStub and apply it to the RpcPromise constructor signature, so constructing from a promised stub produces exactly the type a method returning that stub would. - Make validate's StubResult distributive so unions like ValidatedStub<T> | null elide and never stays never. - Bump the changeset to minor: explicit RpcPromise<RpcStub<T>> annotations on elided results should now be written RpcPromise<T>.
ndisidore
force-pushed
the
feat/result-stub-elision
branch
from
August 19, 2026 21:14
05c32ea to
3bd5a25
Compare
All provably behavior-preserving, each verified independently against the type-test suite (strict Equal<> assertions) and the full build: - Simplify IsUnknown: the inner bracket check was a dead branch (it never excluded `any`; call sites that care check IsAny first). - Drop MethodOrProperty's IsAny guard: Awaited<any> is `any` and Result already opens with the same check. - Drop Result's IsUnknown arm: Result<unknown> falls through to the RpcCompatible arm and yields the structurally identical RpcPromise<unknown>. - Drop Result's Stubable arm: Stubable is a member of the RpcCompatible union, so stubable returns produce the same RpcPromise<R> one arm later. - Delete UnknownResult in favor of RpcPromise<unknown>, which expands to the same type and keeps Result's arms on a single alias for identity short-circuiting. Also document in capnweb-validate why MaybeCallableStub deliberately repeats StubMethodOrProperty's function arm: delegating tips ValidatedStub's recursive instantiation over TS's depth limit (TS2589). Compiler diagnostics improve slightly: types 50,773 -> 50,194, instantiations 199,986 -> 193,650.
…parameter The inline `T | NoInfer<Stub<Extract<T, Stubable>>>` union was doing three jobs at once (accept stubs, restrict to stubable payloads, stay out of inference) with no room to explain any of them. Name it and document why each piece is load-bearing. NoInfer's inference suppression survives the alias: the strict Equal<> constructor-inference tests (bare, callable, plain-interface, and union payloads) are unchanged and pass.
ndisidore
marked this pull request as ready for review
August 19, 2026 22:39
ndisidore
marked this pull request as draft
August 19, 2026 22:50
ndisidore
commented
Aug 19, 2026
| let stub = new NativeRpcStub(new RpcStub(new JsCounter())); | ||
| // Cast: now that the `__RPC_TARGET_BRAND` no longer leaks onto stub surfaces, a userspace | ||
| // stub doesn't statically match workers-types' `Stubable` (runtime interop still works). | ||
| let stub = new NativeRpcStub(<any>new RpcStub(new JsCounter())); |
Collaborator
Author
There was a problem hiding this comment.
ugh this is probably a non-starter
- capnweb stub → new NativeRpcStub(...): needs now
but its because runtime suffers from the same poised branded stub issue
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.Stacked on #242 because the equivalence test needs the constructor from that PR.