Repository navigation
fix(downgrader): clear the 3.1 to 3.0 nullable-merge lists when conversion throws - #99
Conversation
…rsion throws downgradeV30 keeps two module-level lists, twoBranchAnyOfs and anyOfTargets, that were only cleared on success. A throw (such as the RangeError on very deep nesting) left them holding the input and partial output until the next call. Wrap the body in try/finally and clear them once there. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NerLPPYvjVTGwnV3s1in2V
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Wrap
downgradeV30intry/finally— clearstwoBranchAnyOfsandanyOfTargetseven when conversion throws, so a failed call no longer retains the input and partial output until the next call. - Drop the start-of-call clear — now redundant since every call ends with both lists empty.
- Iterate
twoBranchAnyOfsdirectly instead ofsplice(0)— valid becausemergeNullablenever pushes to the list.
The direct iteration is safe: mergeNullable only calls isOnlyNullSchema, isRecord, getRef, find, rejectsNull, and Object.assign, none of which re-enter conversion. downgradeV30 is also not re-entrant — the chained 3.2→3.1→3.0 path calls the two downgraders sequentially — so removing the defensive start-of-call clear cannot leak state between calls.
I confirmed the fix empirically with a throwaway probe (200k two-branch-anyOf schemas plus a >1000-deep schema to force the documented RangeError): the base retains ~142 MB after the throw, while this branch retains ~0 MB. A successful call retains ~0 MB on both. pnpm test passes (262 tests).
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
…lish, in 3.1 to 3.0 (#102) <!-- claude-projects-attribution: {"github_login":"dinwwwh"} --> _Requested by **dinwwwh** · [project thread](https://claude.ai/code/project/chan_01UcKMz91tMarPV1kWLVgLYE?thread=cmsg_01UcKMz91tMarPV1kWLVgLYE3x6dbnc7PCFZZ9Y1ggLhEU)_ ## Problem Effect's `S.NullishOr(X)` and `S.optional(S.NullOr(X))` come out of `@orpc/effect` as `anyOf: [X, { type: "null" }, { type: "null" }]`. The nullable merge from #87/#88 only looks at two-branch anyOfs, so 3.0 keeps the anyOf. NSwag 14.7 on an oRPC Effect model (`addr: S.NullishOr(S.Struct({ city }))`, `tags: S.optional(S.NullOr(S.Array(S.String)))`, `count: S.NullishOr(S.Int)`, `name: S.optional(S.NullOr(S.String))`): on main it makes empty wrapper classes, and the C# client fails on both a filled payload ("Cannot deserialize the current JSON array into type 'Tags'") and an all-null one ("Required property 'addr' expects a value but got null"). With this PR it makes a nullable class, `ICollection<string>`, `long?` and `string`, and both payloads deserialize. Kiota reads the kept anyOf as a string. ## Change - `finishSchema` queues every anyOf with more than one branch (the list is renamed `twoBranchAnyOfs` to `nullableAnyOfs`). - `mergeNullable` merges when all branches but one are only-null. - When the pair becomes a `oneOf`, it keeps the other branch and one null branch, in their order, since a `oneOf` rejects null when it matches two branches. It picks by position, not identity, so a doc that uses one null object twice still gets null once. - Tests: the #87 test that pinned `[string, null, null]` as a kept anyOf moves to the merge test, plus a `oneOf` case for each order and `[string, number, null]` staying an anyOf. One README row. ## Checks - 258 tests pass, `pnpm run check` clean. - 1,000 random Effect docs stay valid with no meaning lost. Zod/Valibot fuzz output and 79 of 80 real docs are byte-identical to main. - Speed (esbuild bundles, one process, gc and shuffled order, 250 rounds): generated api 100, path item chain and the corpus are within noise of main. A synthetic doc with 6,000 Effect nullish fields is about 15% slower, which is the cost of the merge it now gets. Note: #99 also edits `downgradeV30`, so whichever merges second needs a small rebase for the rename. --- _Generated by [Claude Code](https://claude.ai/code/session_01KxDhzz5VuQrnkwczU91Bv6)_ --------- Co-authored-by: Claude <noreply@anthropic.com>

Requested by dinwwwh · project thread
Problem
downgradeV30insrc/v3.1-to-v3.0.tskeeps two module-level lists,twoBranchAnyOfsandanyOfTargets, for the nullable merge. They were cleared at the start of a call and at its successful end, but not when conversion threw (for example theRangeErroron very deep nesting that the README lists). Until the next call, they kept the input and the partial output alive: on a 50k-schema doc, about 61.7 MB stayed retained after a throw versus 4.2 MB after a success.Fix
Wrap the body in
try/finallyand clear both lists once, infinally. Since every call now ends with them empty, the clear at the start is dropped. The loop iteratestwoBranchAnyOfsdirectly instead ofsplice(0), becausemergeNullablenever pushes to it.Checks
pnpm test: 262 tests pass;pnpm run checkclean.mainon 80 real 3.1 documents.RangeErrorstill mergesanyOf: [string, null]intonullable: true. No gc-based unit test, since that would be flaky.Generated by Claude Code