feat: serialize RegExp objects over RPC - #225
Conversation
🦋 Changeset detectedLatest commit: 9cb6d72 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: |
![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.
Adds by-value ["regexp", source, flags] serialization for RegExp, mirroring the existing Date handling.
Issue (high severity): RegExp round-trip is broken at the structuredClonable encoding level.
The Devaluator passes a native RegExp through unencoded when encodingLevel === "structuredClonable" (src/serialize.ts:340-341), exactly like Date (src/serialize.ts:331-332). But the Evaluator's structuredClonable passthrough (src/serialize.ts:818-822) only accepts raw Date and bigint values:
if (this.encodingLevel === "structuredClonable") {
if (value instanceof Date || typeof value === "bigint") {
return value;
}
}A raw RegExp is not an Array, so it never reaches the case "regexp" tuple decoder you added. Instead it falls through to the generic value instanceof Object branch (src/serialize.ts:1248), which iterates enumerable own properties and returns a plain object. Since RegExp has no enumerable own properties, an incoming RegExp at this level is silently corrupted into {} — the value is lost, not round-tripped.
This is exactly the path the PR targets ("aligning with Cloudflare Workers RPC structured-clonable type support"), and it isn't covered by tests: SERIALIZE_TEST_CASES only exercises the default serialize/deserialize (tuple) path, never structuredClonable.
Fix: mirror Date in the Evaluator passthrough at src/serialize.ts:819 (that unchanged line is outside this diff, so I can't attach the suggestion directly there):
if (value instanceof Date || value instanceof RegExp || typeof value === "bigint") {
return value;
}Also please add a structuredClonable-level round-trip test for RegExp so this path is covered.
Minor note: flags normalizes flag order (re.flags is sorted), so /x/mi round-trips as "im". That's harmless (equivalent semantics) and worth a mention only if strict flag-string preservation matters.
|
Review posted successfully to PR #225. I reviewed PR #225 (feat: serialize RegExp objects over RPC) and found one high-severity issue: the |
b4d9912 to
4ec4f99
Compare
There was a problem hiding this comment.
Pull request overview
Adds first-class RPC support for RegExp values, matching Cloudflare Workers’ structured-clonable type support and documenting the new wire encoding.
Changes:
- Introduces a new
regexpRPC kind and corresponding (de)serialization logic using["regexp", source, flags]. - Extends core payload handling to treat
RegExpas an immutable/pass-by-value type in relevant traversal/copy/dispose paths. - Updates docs (README + protocol) and adds round-trip tests for
RegExp.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/serialize.ts | Adds RegExp tuple encoding/decoding and structured-clonable passthrough handling. |
| src/core.ts | Adds regexp to RPC type classification and payload traversal/copy/dispose switch cases. |
| README.md | Moves RegExp into the supported pass-by-value types list. |
| protocol.md | Documents the ["regexp", source, flags] wire format with an example. |
| .changeset/serialize-regexp.md | Declares a minor release for the new serialization capability. |
| tests/index.test.ts | Adds serialize/deserialize round-trip cases for RegExp. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
(Bonk's comment looks like a real issue.) |
4ec4f99 to
09db4b7
Compare
09db4b7 to
9cb6d72
Compare
🚀 Deploying Preview to Cloudflare 🚀Preview URL: https://feat-serialize-regexp.pr.capnweb.com (commit 110d2a6)This URL reflects your latest Preview deploymentPreview Deployments by commit
|
Docs preview
Updates on every push. Deleted when this pull request closes. Not indexed by search engines.
|
110d2a6 to
9cb6d72
Compare
Adds by-value serialization for
RegExp, aligning with Cloudflare Workers RPC structured-clonable type support.Wire format:
["regexp", source, flags](flagsis always present, empty string when none).From what I can tell,
capnweb-validatealready supports RegExp.Based on the RegExp portion of the prior work in #99 (credit to @lmaccherone).
Closes #222.