Support Map over the RPC

[srikrsna]

Adds serialization support for Map. Doesn't special case .map() like we do for arrays, uses the default.

Just like #229, we support reject lazy values in keys.

Closes #230

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/capnweb@232

npm i https://pkg.pr.new/cloudflare/capnweb/capnweb-validate@232

commit: 97f8cc1

[srikrsna]

/bonk review this

[teamchong]

/bonk review this

[ask-bonk]

Reviewed the Map serialization support. The implementation is well thought out and the test coverage is genuinely impressive — the promises inside a Map suite exercises the tricky bits (key/value slots resolving in either order, key collapse under Map semantics, insertion-order preservation, Blob round-trips, and the local deepCopy path). I ran the full node suite (165 passed) and test:types (clean) locally against this branch.

The core design decision — using entries as the source of truth and rebuilding the whole Map on each promise resolution via one-time-use setters — is a clever way to reuse the existing parent[property] = resolved delivery machinery for a structure that has no addressable positions. Nicely mirrors the Set-less-ness reasoning in the comments.

A few non-blocking observations below. None of them are correctness bugs — all tests pass — but the O(n^2) rebuild is worth at least acknowledging.

Notes

1. O(n^2) rebuild on resolution. Each resolved promise slot calls map.clear() and re-inserts every entry, so a Map with n promise slots does O(n^2) work total. For typical payloads this is fine, but a large Map full of pipelined promises could be pathological. Given the constraint (Map has no stable index to write into), this is a reasonable tradeoff — just flagging it. A future optimization could defer the rebuild until all slots for a given batch have resolved, but that adds bookkeeping complexity that probably isn't worth it now.

2. Type coverage for serialization. The Map<K,V> types already exist in src/types.d.ts (Stubify/Unstubify/RpcCompatible), so this PR correctly fills the runtime gap. Worth confirming there's a round-trip/type assertion somewhere in __type-tests__ that a Map value survives — roundTripHeaders(new Map(...)) exists but that's Headers-typed, not a Map-typed round trip. Optional.

Overall this looks good to merge. Nice work on the tests.

[ask-bonk]

Review submitted successfully. Here's a summary of what I reviewed and posted on PR #232:

Review Summary

The PR adds runtime serialization for Map over RPC. I read the full diff, traced the delivery/disposal machinery in src/core.ts and src/serialize.ts, and ran the tests locally.

Verification I did:

• Full node test suite: 165 passed
• Type tests (test:types): clean
• Confirmed Map<K,V> types already existed in src/types.d.ts — this PR fills the missing runtime piece.

Assessment: looks good to merge. The design is sound:

• Uses entries as the source of truth and rebuilds the Map on each promise resolution via one-time-use setters, cleverly reusing the existing parent[property] = resolved delivery path for a structure with no addressable positions.
• Handles key/value promise slots resolving in either order, key collapse under Map semantics, and insertion-order preservation.
• Test coverage is thorough (the promises inside a Map suite is excellent).

Non-blocking notes I posted (3 inline + summary):

1. O(n²) rebuild — map.clear() + full re-insert on every slot resolution; fine for normal payloads but quadratic for large Maps of pipelined promises. Reasonable tradeoff given the constraints, just flagged.
2. Error precision in the evaluate path — malformed ["map", ...] shapes fall through to the generic "unknown type" error rather than a Map-specific one. Optional.
3. Disposal invariant — suggested a clarifying comment noting that the disposeImpl map branch only runs for "return" payloads (where slot setters are never installed), so walking the collapsed Map is safe.

No correctness bugs found.

github run

[changeset-bot]

🦋 Changeset detected

Latest commit: 97f8cc1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
capnweb	Minor

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

[teamchong]

recheck

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[dimitropoulos]

some things