feat: serialize URL objects over RPC - #224
Conversation
Adds by-value serialization for URL, extracted from the URL portion of #99. (Headers, the other type in that PR, already landed on main separately.) Co-authored-by: Larry Maccherone <927864+lmaccherone@users.noreply.github.com>
🦋 Changeset detectedLatest commit: dbae7db The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
commit: |
Co-authored-by: Larry Maccherone <927864+lmaccherone@users.noreply.github.com>
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
Summary: adds by-value serialization for URL via a new ["url", href] wire type, wired through typeForRpc/RpcPayload in core.ts and the devaluator/evaluator in serialize.ts, with a round-trip test.
The implementation itself looks correct and consistent with the existing patterns (exact-prototype match, structuredClonable short-circuit mirrors Date, evaluator validates value.length === 2 && typeof value[1] === 'string' and otherwise falls through to the unknown special value error). Cross-runtime URL is a global everywhere, so the URL.prototype reference is safe.
One actionable item:
-
(medium) New wire type not documented in
protocol.md.protocol.mddocuments every serialization type (["headers", ...],["request", ...],["response", ...], etc.), but this PR introduces["url", href]without adding it there. Per the repo's protocol-compatibility policy, intentional wire-protocol additions must be reflected inprotocol.md. Suggested text to add near the other Fetch/Datetypes (e.g. just before the["headers", pairs]entry around line 201):`["url", href]` A JavaScript `URL` value. `href` is the string form of the URL (i.e. `url.href`), suitable to pass to the `URL` constructor. For example: `["url", "https://example.com/path?q=1"]`.
(protocol.md isn't part of this diff, so I couldn't attach this as an inline suggestion.)
|
Review posted to PR #224. Summary of my review: The URL serialization implementation is correct and consistent with existing patterns (exact-prototype matching, One actionable item flagged: the new |
There was a problem hiding this comment.
Pull request overview
Adds support for passing URL objects by value over Cap’n Web RPC, aligning the supported serialization types with the previously proposed work in #99.
Changes:
- Adds
URLdetection to RPC type classification and payload handling. - Implements URL wire format encoding/decoding as
["url", url.href]. - Updates documentation, tests, and versioning (changeset) to reflect new support.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/serialize.ts | Adds URL tuple encoding in Devaluator and decoding in Evaluator. |
| src/core.ts | Extends TypeForRpc and related switch statements to include "url". |
| README.md | Documents URL as a supported RPC type. |
| .changeset/serialize-url.md | Declares a minor release for URL serialization support. |
| tests/index.test.ts | Adds a URL round-trip serialization/deserialization test case. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The initial commit only wired URL into the runtime serializer. Also add it to the BaseType union and give capnweb-validate full URL support (introspector kind + mapping, emitter, and runtime brand), matching how RegExp/Headers/etc. are handled. Co-authored-by: Larry Maccherone <927864+lmaccherone@users.noreply.github.com>
Adds by-value serialization for
URL, extracted from the URL portion of #99. Headers (the other type in #99) already landed on main.Credit to @lmaccherone for the original work in #99.