Skip to content

refactor: remove compound op types from Path.transform and Point.transform#2343

Closed
christianhg wants to merge 1 commit intomainfrom
refactor/nuke-compound-ops
Closed

refactor: remove compound op types from Path.transform and Point.transform#2343
christianhg wants to merge 1 commit intomainfrom
refactor/nuke-compound-ops

Conversation

@christianhg
Copy link
Member

@christianhg christianhg commented Mar 10, 2026

Slate's Path.transform and Point.transform still handled merge_node, split_node, and move_node operations even though these compound types are no longer emitted through editor.apply - they were decomposed into lower-level operations in #2327. This PR removes the compound types from the Slate type system entirely and inlines the transform logic into the three apply helpers that still need it.


After #2327, applyMergeNode, applySplitNode, and applyMoveNode decompose compound operations into insert_node, remove_node, insert_text, and remove_text. But they still need the compound operation's ref-transform semantics: a merge shifts paths differently than the equivalent insert_text + remove_node sequence. The helpers handle this by pre-transforming all active refs with the compound semantics, then suppressing ref transforms for the decomposed operations.

Previously, this pre-transform step delegated to Path.transform and Point.transform, which meant those functions had to keep the compound operation cases around even though no other code path used them. This PR inlines the transform logic directly into each helper as local transformPathForMerge, transformPointForMerge (and equivalents for split and move). The compound types are then removed from Path.transform, Point.transform, Operation.inverse, operationCanTransformPath, the Operation union, and CustomTypes.

Path.transform now only handles insert_node and remove_node. The 130-line function drops to 30 lines. Point.transform loses three cases. Operation.inverse loses three cases (including the most complex one - move_node inverse required transforming paths through the operation itself).

The inlining surfaced a subtle bug in how RangeRef affinities were being resolved. When a RangeRef has 'inward' affinity, the anchor and focus need different per-point affinities: on a forward range, the anchor gets 'forward' and the focus gets 'backward' (they contract toward each other). The old code got this for free through the Range.transform -> Point.transform chain. The inlined version needs it explicitly. A shared rangeRefAffinities helper resolves per-point affinities for all three apply helpers.

Also removes a dead split_node check in diff-text.ts (transformPendingPoint) that could never trigger since split_node operations are no longer emitted.

…sform

Remove merge_node, split_node, and move_node from the Operation union,
Operation.inverse, Path.transform, Point.transform, and
operationCanTransformPath. These compound operations are no longer
emitted through editor.apply (they were decomposed into lower-level
operations in the patch-compliant operations work).

The apply helpers (applyMergeNode, applySplitNode, applyMoveNode) still
construct virtual compound ops internally for ref pre-transformation,
but now inline the transform logic directly instead of delegating to
Path.transform/Point.transform.

Adds rangeRefAffinities helper to correctly resolve per-point affinity
for RangeRefs during inlined transforms (inward affinity on a forward
range needs anchor=forward, focus=backward).

Also removes dead split_node check in diff-text.ts.
@vercel
Copy link

vercel bot commented Mar 10, 2026

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

Project Deployment Actions Updated (UTC)
portable-text-editor-documentation Ready Ready Preview, Comment Mar 10, 2026 0:39am
portable-text-example-basic Ready Ready Preview, Comment Mar 10, 2026 0:39am
portable-text-playground Ready Ready Preview, Comment Mar 10, 2026 0:39am

Request Review

@changeset-bot
Copy link

changeset-bot bot commented Mar 10, 2026

⚠️ No Changeset found

Latest commit: a6925b1

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
Copy link
Contributor

github-actions bot commented Mar 10, 2026

📦 Bundle Stats — @portabletext/editor

Compared against main (d312abd5)

@portabletext/editor

Metric Value vs main (d312abd)
Internal (raw) 801.3 KB +291 B, +0.0%
Internal (gzip) 150.4 KB +82 B, +0.1%
Bundled (raw) 1.41 MB +291 B, +0.0%
Bundled (gzip) 313.3 KB +98 B, +0.0%
Import time 92ms +1ms, +0.6%

@portabletext/editor/behaviors

Metric Value vs main (d312abd)
Internal (raw) 467 B -
Internal (gzip) 207 B -
Bundled (raw) 424 B -
Bundled (gzip) 171 B -
Import time 6ms -0ms, -1.0%

@portabletext/editor/plugins

Metric Value vs main (d312abd)
Internal (raw) 2.5 KB -
Internal (gzip) 910 B -
Bundled (raw) 2.3 KB -
Bundled (gzip) 839 B -
Import time 11ms -0ms, -0.7%

@portabletext/editor/selectors

Metric Value vs main (d312abd)
Internal (raw) 60.2 KB -
Internal (gzip) 9.4 KB -
Bundled (raw) 56.7 KB -
Bundled (gzip) 8.6 KB -
Import time 10ms -0ms, -0.5%

@portabletext/editor/utils

Metric Value vs main (d312abd)
Internal (raw) 24.2 KB -
Internal (gzip) 4.7 KB -
Bundled (raw) 22.2 KB -
Bundled (gzip) 4.4 KB -
Import time 9ms +0ms, +2.6%
Details
  • Import time regressions over 10% are flagged with ⚠️
  • Treemap artifacts are attached to the CI run for detailed size analysis
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

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