Fix TS recursion limit in deep types#904
Conversation
Co-authored-by: whoiskatrin <whoiskatrin@users.noreply.github.com>
🦋 Changeset detectedLatest commit: 04fbded 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: |
|
/bonk review this PR |
|
Here's my review of this PR: PR Review: Fix TS recursion limit in deep typesSummaryThis PR adds a tuple-based depth counter to Verdict: Approve with minor nitsThe core approach is sound and follows a well-established pattern for depth-limiting recursive TypeScript types. The implementation matches the suggestion in the issue exactly. What's good
Issues to fix
Minor observations
Suggested fix for issue #1In -// See: https://github.com/cloudflare/agents/issues/XXX
+// See: https://github.com/cloudflare/agents/issues/903Shall I apply that fix? |
|
/bonk apply the issue fix |
Co-authored-by: whoiskatrin <whoiskatrin@users.noreply.github.com>
|
Fixed the stale issue URL in |
Here's a summary of the changes:
Changes
packages/agents/src/serializable.tsAdded a tuple-based depth counter to
CanSerializeandIsSerializableParamtypes that bails out totrueafter 10 levels of recursion. Three new helper types were added:MaxDepth— a 10-element tuple defining the recursion limitIncrement<D>— prepends an element to the depth tupleIsMaxDepth<D>— checks if the depth tuple has reached the limitBoth
CanSerialize<T, Seen, Depth>andIsSerializableParam<T, Seen, Depth>now checkIsMaxDepth<Depth>before recursing deeper. If the depth limit is reached without encountering aNonSerializabletype, the type is assumed to be JSON-safe (returnstrue).The existing
Seenparameter for circular type detection is preserved — the depth counter addresses the orthogonal problem of types that are simply deeply nested (many levels of unions and nested objects) without being circular.packages/agents/src/tests-d/serializable.test-d.tsAdded type-level tests with a simulated
CoreMessage-like type structure (deeply nested discriminated unions withTextPart | ToolCallPart | ImagePart, metadata objects, etc.) to verify these types don't trigger TypeScript's recursion limit when used withuseAgent.Verification
Closes #903
github run