fix: convert cyclic Portable Text schemas without recursing forever#2956
Merged
Conversation
🦋 Changeset detectedLatest commit: 3ac3a5d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Contributor
📦 Bundle Stats —
|
| Metric | Value | vs main (1a64c0e) |
|---|---|---|
| Internal (raw) | 800.4 KB | - |
| Internal (gzip) | 153.4 KB | - |
| Bundled (raw) | 1.41 MB | - |
| Bundled (gzip) | 317.6 KB | - |
| Import time | 102ms | +1ms, +1.4% |
@portabletext/editor/behaviors
| Metric | Value | vs main (1a64c0e) |
|---|---|---|
| Internal (raw) | 467 B | - |
| Internal (gzip) | 207 B | - |
| Bundled (raw) | 424 B | - |
| Bundled (gzip) | 171 B | - |
| Import time | 2ms | +0ms, +3.0% |
@portabletext/editor/plugins
| Metric | Value | vs main (1a64c0e) |
|---|---|---|
| Internal (raw) | 2.7 KB | - |
| Internal (gzip) | 894 B | - |
| Bundled (raw) | 2.5 KB | - |
| Bundled (gzip) | 827 B | - |
| Import time | 7ms | -0ms, -2.3% |
@portabletext/editor/selectors
| Metric | Value | vs main (1a64c0e) |
|---|---|---|
| Internal (raw) | 81.2 KB | - |
| Internal (gzip) | 14.9 KB | - |
| Bundled (raw) | 76.7 KB | - |
| Bundled (gzip) | 13.8 KB | - |
| Import time | 8ms | -0ms, -1.0% |
@portabletext/editor/traversal
| Metric | Value | vs main (1a64c0e) |
|---|---|---|
| Internal (raw) | 28.1 KB | - |
| Internal (gzip) | 5.6 KB | - |
| Bundled (raw) | 27.9 KB | - |
| Bundled (gzip) | 5.5 KB | - |
| Import time | 6ms | +0ms, +3.4% |
@portabletext/editor/utils
| Metric | Value | vs main (1a64c0e) |
|---|---|---|
| Internal (raw) | 29.7 KB | - |
| Internal (gzip) | 6.2 KB | - |
| Bundled (raw) | 27.1 KB | - |
| Bundled (gzip) | 5.8 KB | - |
| Import time | 6ms | +0ms, +0.2% |
🗺️ . · ./behaviors · ./plugins · ./selectors · ./traversal · ./utils · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @portabletext/markdown
Compared against main (1a64c0ec)
| Metric | Value | vs main (1a64c0e) |
|---|---|---|
| Internal (raw) | 53.8 KB | - |
| Internal (gzip) | 9.8 KB | - |
| Bundled (raw) | 348.9 KB | - |
| Bundled (gzip) | 96.3 KB | - |
| Import time | 40ms | +2ms, +4.4% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
christianhg
force-pushed
the
linear-bridge-conversion
branch
from
0d67163 to
8a99245
Compare
christianhg
force-pushed
the
linear-bridge-conversion
branch
from
8a99245 to
880a853
Compare
christianhg
force-pushed
the
linear-bridge-conversion
branch
from
880a853 to
62c5270
Compare
christianhg
force-pushed
the
linear-bridge-conversion
branch
from
62c5270 to
969c84f
Compare
vercel
Bot
temporarily deployed
to
Preview – portable-text-editor-documentation
christianhg
force-pushed
the
linear-bridge-conversion
branch
from
1f1cea8 to
7e276bd
Compare
vercel
Bot
temporarily deployed
to
Preview – portable-text-editor-documentation
christianhg
force-pushed
the
linear-bridge-conversion
branch
from
7e276bd to
e432b38
Compare
vercel
Bot
temporarily deployed
to
Preview – portable-text-editor-documentation
Two crashes lived in `sanitySchemaToPortableTextSchema`'s recursive walk. A block's inline objects and annotations expanded their fields unconditionally: block members bypass both the memo and the ancestor cut (`findBlockType` short-circuits before either), so a block whose inline object or annotation transitively carries another Portable Text field recursed forever, an inline footnote containing rich text froze the studio outright. And the first descent through n mutually-embedding types chained call-stack frames through every type before any memo entry landed, overflowing the stack between 1,400 and 1,600 types. The walk now runs on an explicit LIFO work stack: children are pushed in reverse so the drain order is exact DFS pre-order (which the memo's first-expansion-wins semantics depend on), results land in pre-indexed holes so construction matches the recursive version, and ancestor names live in one reference-counted map scoped by pop-markers (reference-counted because the same name can be seeded at several depths of one branch, and a plain shared set's `delete` would clobber the outer scope, the recursive version's per-branch copies were naturally reentrant). Inline-object and annotation expansions track in-flight instances with the ancestor-set size at entry: re-entering the same instance at the same size is the exact recursion state the old code looped forever on (ancestors grow monotonically along a branch, so equal size means equal set), and only there do the fields cut to empty. Every schema the old implementation could convert produces byte-identical output, pinned by a fuzz oracle against the old implementation kept verbatim in `reference-conversion-for-oracle.ts`, plus a does-not-throw pin at 1,600 mutually-embedding types and a footnote-cycle regression test. A drain circuit breaker (100M work items) turns any future uncut cycle into a diagnostic error instead of an exhausted heap. Left deliberately unfixed: the output of n mutually-embedding types is inherently O(n^2) entries, each embedding position's `of` list genuinely differs (a type's self-reference is ancestor-cut at its own position and memo-shared at others), so conversion time on such shapes stays quadratic as long as the output is eager and identical. Bounding that requires a semantic change (a depth cap or a lazy schema), tracked separately.
christianhg
force-pushed
the
linear-bridge-conversion
branch
from
e432b38 to
dc8a7b2
Compare
christianhg
force-pushed
the
linear-bridge-conversion
branch
from
dc8a7b2 to
a2bb626
Compare
vercel
Bot
temporarily deployed
to
Preview – portable-text-editor-documentation
vercel
Bot
temporarily deployed
to
Preview – portable-text-editor-documentation
…verses `getSanitySubSchema` (raw Sanity types plus value) and `@portabletext/schema`'s `getSubSchema` (the expanded Portable Text schema) answer "what is allowed at this position?" in their respective type universes, and nothing forced them to agree. The agreement test walks both resolvers at the root and inside a restricted nested block and asserts the bucketizations correspond under the name mapping (Portable Text names are Sanity `value`s for styles, lists, and decorators, type names for everything else), plus an explicit pin of the restricted position's members so agreement cannot be satisfied by both sides drifting identically.
christianhg
force-pushed
the
linear-bridge-conversion
branch
from
8ea92b0 to
3ac3a5d
Compare
vercel
Bot
temporarily deployed
to
Preview – portable-text-editor-documentation
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This started as the quadratic scaling ladder and the stack-overflow cliff in
sanity-bridge's conversion, and the fuzz oracle built for that fix surfaced something worse: an unbounded recursion on cyclic schemas that ships in production today. A block's inline objects and annotations expand their fields unconditionally, block members bypass both the memo and the ancestor cut (findBlockTypeshort-circuits before either), so a schema where a block's inline object or annotation transitively carries another Portable Text field (an inline footnote containing rich text) recurses forever:RangeErrorif the stack dies first, a frozen tab otherwise, triggered byPortableTextInputmounting. #2774's regression fixtures only nested through top-level block objects, which cut correctly, so this survived that fix unseen.The walk now runs on an explicit LIFO work stack instead of the call stack. Three design points carry the byte-identical claim: children are pushed in reverse so the drain order is exact DFS pre-order, which the memo's first-expansion-wins semantics depend on; results land in pre-indexed holes so construction order matches the recursive version; and ancestor names live in one reference-counted map scoped by pop-markers, reference-counted because the same name can be seeded at several depths of one branch, and a plain shared set's
deleteclobbers the outer scope where the recursive version's per-branch copies were naturally reentrant (the fuzz oracle caught exactly this during development). The cycle cut is deliberately surgical: inline-object and annotation expansions track in-flight instances together with the ancestor-set size at entry, and only a re-entry of the same instance at the same size, provably the exact state the old code looped forever on, since ancestors grow monotonically along a branch, cuts to empty fields. Name-based cutting was tried first and rejected by the oracle: it changed output for schemas the old code terminated on.Verification is three-layered: a fuzz oracle comparing against the old implementation kept verbatim in
reference-conversion-for-oracle.ts(identical output wherever the reference terminates, termination where it crashes), a footnote-cycle regression test pinning the crash fix, and a 1,600-type does-not-throw pin at the old stack cliff. All 43 pre-existing output pins pass unmodified. A 100M-work-item circuit breaker turns any future uncut cycle into a diagnostic error instead of an exhausted heap.Honest scope notes. The original ladder quadratic is not fixed, because measurement showed it cannot be while output stays eager and byte-identical: each of n embedding positions carries its own
oflist and those lists genuinely differ per position (a type's self-reference is ancestor-cut at its own position, memo-shared at others), so the output itself is O(n^2) entries, ~762ms and ~115MB at 1,600 types, which previously crashed outright. Bounding that requires a semantic change, a configurable expansion depth cap or a lazy schema mirroring Sanity's own compiled-schema laziness, tracked as a follow-up design ticket. The work-stack machinery also costs ~40% on pathological-shape constants (191ms vs 138ms at 800 types) in exchange for never overflowing; common schemas (~300 types, ~17ms) are unaffected.Two companion commits ride along. A docs commit rewrites the README's framing (what the expansion is and why the editor wants plain data), documents how recursive schemas resolve, and replaces two documented-but-nonexistent functions with the real API (
createPortableTextMemberSchemaTypes,getSanitySubSchema). And a test commit adds a sub-schema agreement oracle:getSanitySubSchemaand@portabletext/schema'sgetSubSchemaanswer "what is allowed at this position?" in their respective type universes with nothing previously forcing them to agree, so the new test walks both at the root and inside a restricted nested block and asserts the bucketizations correspond under the name mapping (Portable Text names are Sanityvalues for styles, lists, and decorators).