Skip to content

refactor: make event-chain depth part of PerformEventArgs#2991

Merged
christianhg merged 1 commit into
mainfrom
refactor-event-chain-depth-field
Jul 21, 2026
Merged

refactor: make event-chain depth part of PerformEventArgs#2991
christianhg merged 1 commit into
mainfrom
refactor-event-chain-depth-field

Conversation

@christianhg

@christianhg christianhg commented Jul 21, 2026

Copy link
Copy Markdown
Member

Grooming follow-up to the depth backstop that shipped with the annotation-recursion fix (#2985), internal-only.

The depth counter moves from a module-level WeakMap<PortableTextEditorEngine, number> with try/finally unwinding into PerformEventArgs itself. The depth is purely call-stack-shaped state and the call graph supports expressing it that way: the three raise/forward/execute sites are the only recursive callers and the editor machine is the only external entry, all direct synchronous calls. The machine passes depth: 0, the recursive sites pass depth + 1, the guard becomes the first statement of performEvent, and the WeakMap, the try/finally, and the performEventInternal wrapper (which existed only to bolt the guard onto the original body) all disappear.

depth is required rather than defaulted, deliberately: a future recursive call site that omits it fails to compile instead of silently resetting the chain and disabling the backstop for that path. The one path worth calling out: sendBack re-entries go through the actor's event queue (sequential, not nested), so a fresh chain correctly starts at zero there, exactly as the ambient counter behaved after its finally, and no stack growth is possible through that route.

The maxEventDepth = 100 comment now states the two-sided window the value sits in instead of only the lower bound: it must exceed any legitimate raise-within-raise nesting (real Behavior stacks nest ~10-15 deep at the pathological end; the limit measures nesting, not sequential fan-out), and it must stay far below genuine stack exhaustion, since each nesting level costs dozens of real stack frames and browser limits vary. That puts the workable window at roughly 50-500, with asymmetric failure modes: too low silently drops legitimate events, too high only delays cycle detection by milliseconds, so when in doubt, raise it.

Net behavior unchanged: the cyclic-Behavior pin and the full unit (861) and browser (1734) suites pass without modification. No changeset, nothing consumer-observable.

@vercel

vercel Bot commented Jul 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
portable-text-editor-documentation Building Building Preview, Comment Jul 21, 2026 9:27am
portable-text-example-basic Ready Ready Preview, Comment Jul 21, 2026 9:27am
portable-text-playground Ready Ready Preview, Comment Jul 21, 2026 9:27am

Request Review

@changeset-bot

changeset-bot Bot commented Jul 21, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 68432a5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

📦 Bundle Stats — @portabletext/editor

Compared against main (3296788c)

@portabletext/editor

Metric Value vs main (3296788)
Internal (raw) 803.9 KB -166 B, -0.0%
Internal (gzip) 154.3 KB -65 B, -0.0%
Bundled (raw) 1.41 MB -166 B, -0.0%
Bundled (gzip) 318.5 KB -66 B, -0.0%
Import time 103ms -3ms, -2.7%

@portabletext/editor/behaviors

Metric Value vs main (3296788)
Internal (raw) 467 B -
Internal (gzip) 207 B -
Bundled (raw) 424 B -
Bundled (gzip) 171 B -
Import time 2ms -0ms, -10.0%

@portabletext/editor/plugins

Metric Value vs main (3296788)
Internal (raw) 2.7 KB -
Internal (gzip) 894 B -
Bundled (raw) 2.5 KB -
Bundled (gzip) 827 B -
Import time 7ms -0ms, -4.1%

@portabletext/editor/selectors

Metric Value vs main (3296788)
Internal (raw) 81.8 KB -
Internal (gzip) 15.2 KB -
Bundled (raw) 77.6 KB -
Bundled (gzip) 14.1 KB -
Import time 8ms -0ms, -1.0%

@portabletext/editor/traversal

Metric Value vs main (3296788)
Internal (raw) 28.1 KB -
Internal (gzip) 5.6 KB -
Bundled (raw) 28.1 KB -
Bundled (gzip) 5.5 KB -
Import time 6ms +0ms, +0.6%

@portabletext/editor/utils

Metric Value vs main (3296788)
Internal (raw) 29.7 KB -
Internal (gzip) 6.2 KB -
Bundled (raw) 27.4 KB -
Bundled (gzip) 5.9 KB -
Import time 6ms +0ms, +1.8%

🗺️ . · ./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 (3296788c)

Metric Value vs main (3296788)
Internal (raw) 53.8 KB -
Internal (gzip) 9.8 KB -
Bundled (raw) 348.9 KB -
Bundled (gzip) 96.3 KB -
Import time 39ms -2ms, -5.2%

🗺️ 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.

The depth backstop kept its counter in a module-level
`WeakMap<PortableTextEditorEngine, number>` with try/finally
unwinding, and `performEvent` was split into a guard wrapper plus
`performEventInternal` purely to bolt that on. But the depth is
call-stack-shaped state and all recursion is direct: the three
`raise`/`forward`/`execute` sites are the only recursive callers
and the editor machine is the only external entry.

`depth` becomes a required field of `PerformEventArgs`: the machine
passes `0`, the recursive sites pass `depth + 1`, the guard is the
first statement of the single `performEvent` function, and the
wrapper, the WeakMap and the try/finally all disappear. Required
rather than defaulted so a future recursive call site cannot omit it
and silently reset the chain, disabling the backstop for that path.
`sendBack` re-entries go through the actor's event queue (sequential,
not nested), so fresh chains correctly start at zero there, exactly as
the ambient counter behaved after its `finally`.

Also documents the two-sided window the depth limit sits in: it must
exceed legitimate raise-within-raise nesting (~10-15 at the
pathological end) and stay far below genuine stack exhaustion (dozens
of real frames per nesting level, browser limits vary), with the
asymmetry spelled out: too low silently drops legitimate events, too
high only delays cycle detection by milliseconds.

Net behavior unchanged; the cyclic-Behavior pin and the full suites
pass without modification.
@christianhg
christianhg force-pushed the refactor-event-chain-depth-field branch from 0de0bce to 68432a5 Compare July 21, 2026 09:26
@christianhg christianhg changed the title refactor: thread Behavior event-chain depth as a performEvent parameter refactor: make event-chain depth part of PerformEventArgs Jul 21, 2026
@christianhg
christianhg marked this pull request as ready for review July 21, 2026 09:28
@christianhg
christianhg enabled auto-merge (rebase) July 21, 2026 09:28
@christianhg
christianhg merged commit 9407ce4 into main Jul 21, 2026
17 checks passed
@christianhg
christianhg deleted the refactor-event-chain-depth-field branch July 21, 2026 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant