Fix code block highlights lost after re-import and editing by samuelpecher · Pull Request #1008 · basecamp/lexxy

samuelpecher · 2026-04-24T09:22:31Z

Summary

Editing a re-imported code block with highlights caused both a crash (Lexical error 226) and permanent loss of all highlights
Crash fix: added #clampSelectionOffset to prevent stale element selection offsets exceeding child count after retokenization
Highlight preservation fix: The @lexical/code retokenizer replaces all children with fresh unstyled tokens, permanently destroying highlight styles. Added CodeHighlightNode and CodeNode transforms that snapshot existing highlight ranges into pendingCodeHighlights before retokenization, so the mutation listener can re-apply them afterward. Deferred the mutation listener's editor.update() via queueMicrotask() to avoid stale-node errors in the markdown transform.

Fixes Basecamp card #9773564467: Uncaught error when trying to edit a previously posted code block with highlights

samuelpecher · 2026-04-24T09:54:52Z

Manual Validation: Validated

Set editor value to a code block containing highlighted text (<mark> elements), clicked into it, positioned cursor after trailing space, pressed Enter twice. In production, the editor threw Lexical error #226 (ElementNode.splice: start + deleteCount > oldSize) and became non-functional — typed text was lost. Locally with the fix, Enter twice escaped the code block cleanly and typed text appeared in a paragraph below with no errors.

Copilot

Pull request overview

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Fixes a Lexical editor crash when pressing Enter inside a re-imported code block containing highlights, by preventing stale element-selection offsets (after retokenization) from exceeding the code node’s current child count.

Changes:

Clamp element-type selection offsets in EarlyEscapeCodeNode.insertNewAfter before delegating to Lexical’s CodeNode insertion behavior.
Add a Playwright regression test that reproduces the re-import + highlight + Enter sequence and asserts no uncaught errors occur.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
test/browser/tests/formatting/bug_code_highlight_edit_error.test.js	Adds an end-to-end regression test covering the previously crashing Enter behavior in highlighted, re-imported code blocks.
src/nodes/early_escape_code_node.js	Clamps invalid element selection offsets to current child count to avoid `splice` range errors during Enter handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

When a code block with highlighted characters is saved and later re-edited, pressing Enter twice throws "ElementNode.splice: start + deleteCount > oldSize". The root cause is a stale element-type selection offset left by the code retokenizer's $updateAndRetainSelection. The highlight import flow splits CodeHighlightNodes to apply styles (e.g. "asdf " becomes ["as", "df"(highlighted), " "]). This runs with skipTransforms, so the retokenizer doesn't see the extra children. On the next user action (Enter), the retokenizer replaces all children back to its own tokens (reducing child count), then restores the selection offset from before retokenization — which now exceeds the child count. CodeNode's insertNewAfter passes that stale offset to splice(), which throws. The fix clamps element-type selection offsets in EarlyEscapeCodeNode's insertNewAfter before delegating to the parent CodeNode implementation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The previous fix (d50821b) addressed the crash when pressing Enter in a re-imported code block with highlights, but the underlying problem remained: any edit to the code block caused all highlights to be lost. Root cause: the @lexical/code retokenizer registers a CodeHighlightNode transform that calls $codeNodeTransform directly, replacing all children with fresh unstyled tokens. The highlight ranges stored during HTML import were consumed on first application and never recaptured, so subsequent retokenizations (triggered by any edit) permanently lost the highlight styles. Fix: register CodeHighlightNode and CodeNode transforms (before the retokenizer's, by registration order) that snapshot existing highlight ranges into pendingCodeHighlights before retokenization can destroy them. The mutation listener then re-applies them via a microtask-deferred editor.update({ skipTransforms: true }) after the retokenizer finishes. The microtask scheduling avoids a stale-node error in the markdown transform that occurred when running a discrete editor.update() inside Lexical's mutation-dispatch cycle. Also renames "Collect console errors" to "Collect uncaught page errors" in the existing test (the listener is for pageerror, not console). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Verifies that typing text at the end of a code block line after a highlighted region does not cause the highlight <mark> to be lost during retokenization. The existing fix already covers this case. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-24T12:58:06Z

+    for (const point of [ selection.anchor, selection.focus ]) {
+      if (point.type === "element" && point.key === this.__key && point.offset > childrenSize) {
+        point.set(this.__key, childrenSize, "element")


this.__key is an internal Lexical implementation detail. To reduce coupling to non-public APIs, prefer using this.getKey() for comparisons and when calling point.set(...) (store it once in a local const key = this.getKey() and use that).

Suggested change

for (const point of [ selection.anchor, selection.focus ]) {

if (point.type === "element" && point.key === this.__key && point.offset > childrenSize) {

point.set(this.__key, childrenSize, "element")

const key = this.getKey()

for (const point of [ selection.anchor, selection.focus ]) {

if (point.type === "element" && point.key === key && point.offset > childrenSize) {

point.set(key, childrenSize, "element")

Copilot · 2026-04-24T12:58:06Z

+  // Schedule the re-application as a microtask so we don't run
+  // inside Lexical's mutation-dispatch cycle. Running a discrete
+  // editor.update() during mutation dispatch can trigger enqueued
+  // transforms (e.g. markdown) that hold stale node references from
+  // the just-committed state.
+  queueMicrotask(() => {
+    editor.update(() => {
+      for (const { key, highlights } of highlightData) {
+        if (!highlights) continue

-      $applyHighlightRangesToCodeNode(codeNode, highlights)
-    }
-  }, { skipTransforms: true, discrete: true })
+        const codeNode = $getNodeByKey(key)
+        if (!codeNode || !$isCodeNode(codeNode) || !codeNode.isAttached()) continue
+
+        $applyHighlightRangesToCodeNode(codeNode, highlights)
+      }
+    }, { skipTransforms: true, discrete: true })
+  })


queueMicrotask is not available in some older browser environments. If this editor supports those, add a small fallback (e.g., schedule via a resolved Promise) so highlight re-application doesn't break on platforms without queueMicrotask.

Copilot · 2026-04-24T12:58:07Z

+    // Allow any async error handlers to fire
+    await page.waitForTimeout(100)
+
+    // Should have no errors
+    expect(errors).toEqual([])
+


Using a fixed waitForTimeout(100) is prone to test flakiness on slower CI machines. Prefer waiting on an explicit condition (e.g., polling until the editor has flushed and errors.length is still 0 over a short window) rather than a hard-coded sleep.

Suggested change

// Allow any async error handlers to fire

await page.waitForTimeout(100)

// Should have no errors

expect(errors).toEqual([])

// Wait for the editor to flush and verify no async page errors occur

await expect(async () => {

await editor.flush()

expect(errors).toEqual([])

}).toPass({ timeout: 5_000 })

Covers the scenario where pressing Enter at the end of a highlighted code block line to create a new line, then typing text, should not cause retokenization to strip the existing highlight mark. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-24T13:44:03Z

+      for (const { start, end, node } of splitOffsets) {
+        if (point.offset >= start && point.offset <= end) {


The remapping logic for split text-node selection uses point.offset >= start && point.offset <= end, which will map boundary offsets to the earlier segment (e.g., offset at the start of a highlight maps to the preceding unstyled node). This can change the formatting that newly-typed text inherits at range boundaries. Prefer half-open intervals (start <= offset < end) and handle the final segment explicitly so boundaries map to the following segment.

Suggested change

for (const { start, end, node } of splitOffsets) {

if (point.offset >= start && point.offset <= end) {

for (let i = 0; i < splitOffsets.length; i++) {

const { start, end, node } = splitOffsets[i]

const isLastSegment = i === splitOffsets.length - 1

if (point.offset >= start && (point.offset < end || (isLastSegment && point.offset === end))) {

Copilot · 2026-04-24T13:44:04Z

+    } else if (point.type === "element" && point.key === codeNodeKey && point.offset > 0) {
+      // Element selection on the CodeNode. Splits insert additional
+      // children before the original offset position, shifting it.
+      // Since all highlights are typically on the first line, and the
+      // selection is on a later line (after a LineBreak), the full
+      // totalShift applies.
+      point.set(codeNodeKey, point.offset + totalShift, "element")
+    }


$remapSelectionAfterHighlightSplits adjusts element-type selection offsets by adding totalShift from all splits, regardless of whether those splits happened before the original point.offset. If the cursor is between children before some later split, this will over-shift the selection and can move the caret to the wrong position. Track the original child index for each split (e.g., node.getIndexWithinParent() before replacement) and only add the cumulative shift for splits whose index is < the original point.offset.

Copilot · 2026-04-24T13:44:04Z

+    $remapSelectionAfterHighlightSplits(selection, codeNodeKey, codeNode, replacementMap)
+  }
+}
+
+// After splitting CodeHighlightNodes for highlight application, remap
+// the selection's anchor/focus so the cursor doesn't jump.
+// `childIndexMap` maps original child index → number of extra children
+// inserted before that position (cumulative shift).
+function $remapSelectionAfterHighlightSplits(selection, codeNodeKey, codeNode, replacementMap) {


The doc comment above $remapSelectionAfterHighlightSplits refers to a childIndexMap parameter, but the function actually takes (selection, codeNodeKey, codeNode, replacementMap) and does not use any childIndexMap. Please update/remove the comment and consider dropping the unused codeNode parameter to keep the intent clear.

Suggested change

$remapSelectionAfterHighlightSplits(selection, codeNodeKey, codeNode, replacementMap)

}

}

// After splitting CodeHighlightNodes for highlight application, remap

// the selection's anchor/focus so the cursor doesn't jump.

// `childIndexMap` maps original child index → number of extra children

// inserted before that position (cumulative shift).

function $remapSelectionAfterHighlightSplits(selection, codeNodeKey, codeNode, replacementMap) {

$remapSelectionAfterHighlightSplits(selection, codeNodeKey, replacementMap)

}

}

// After splitting CodeHighlightNodes for highlight application, remap

// the selection's anchor/focus so the cursor doesn't jump.

// `replacementMap` stores split metadata for each original node,

// including replacement counts and offset-to-node mappings.

function $remapSelectionAfterHighlightSplits(selection, codeNodeKey, replacementMap) {

Copilot · 2026-04-24T13:44:05Z

+    // Read debug info
+    const debugInfo = await page.evaluate(() => window.__hlDebug || [])
+    console.log("Debug info:", JSON.stringify(debugInfo, null, 2))
+
+    const plainText = await editor.plainTextValue()
+    console.log("Plain text:", JSON.stringify(plainText))


This test leaves console.log statements in place, which can add noise/flakiness to CI output (and it currently depends on window.__hlDebug, which is being populated from production code). Prefer removing the logs and asserting only on editor-visible behavior (e.g., plain text / DOM) so tests don’t require debug instrumentation in shipped code.

Suggested change

// Read debug info

const debugInfo = await page.evaluate(() => window.__hlDebug || [])

console.log("Debug info:", JSON.stringify(debugInfo, null, 2))

const plainText = await editor.plainTextValue()

console.log("Plain text:", JSON.stringify(plainText))

const plainText = await editor.plainTextValue()

Copilot · 2026-04-24T13:44:05Z

+      const selBefore = $getSelection()
+      if ($isRangeSelection(selBefore)) {
+        window.__hlDebug = window.__hlDebug || []
+        window.__hlDebug.push({ when: "before", anchorType: selBefore.anchor.type, anchorKey: selBefore.anchor.key, anchorOffset: selBefore.anchor.offset, focusType: selBefore.focus.type, focusKey: selBefore.focus.key, focusOffset: selBefore.focus.offset })
+      }


$applyPendingCodeHighlights writes selection debug data into window.__hlDebug on every re-application. This introduces a persistent global side effect in production code (and can leak potentially sensitive editor state into the page). Consider removing this entirely, or gating it behind a dedicated debug flag used only in tests/dev builds.

samuelpecher self-assigned this Apr 24, 2026

samuelpecher added the fix-validated Bug fix manually validated: broken in production, fixed locally label Apr 24, 2026

samuelpecher marked this pull request as ready for review April 24, 2026 11:32

Copilot AI review requested due to automatic review settings April 24, 2026 11:32

Copilot started reviewing on behalf of samuelpecher April 24, 2026 11:32 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Comment thread test/browser/tests/formatting/bug_code_highlight_edit_error.test.js Outdated

samuelpecher changed the title ~~Fix crash when pressing Enter in re-imported code block with highlights~~ Fix code block highlights lost after re-import and editing Apr 24, 2026

samuelpecher and others added 3 commits April 24, 2026 13:53

Copilot AI review requested due to automatic review settings April 24, 2026 12:55

samuelpecher force-pushed the fix-code-highlight-edit-error branch from 37fcc3c to bb15ade Compare April 24, 2026 12:55

Copilot started reviewing on behalf of samuelpecher April 24, 2026 12:55 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

samuelpecher removed the fix-validated Bug fix manually validated: broken in production, fixed locally label Apr 24, 2026

WIP: Fix selection jumping in re-imported code blocks with highlights

de461ca

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings April 24, 2026 13:40

Copilot started reviewing on behalf of samuelpecher April 24, 2026 13:41 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

samuelpecher marked this pull request as draft April 24, 2026 15:05

		for (const { start, end, node } of splitOffsets) {
		if (point.offset >= start && point.offset <= end) {

-      for (const { start, end, node } of splitOffsets) {
-        if (point.offset >= start && point.offset <= end) {
+      for (let i = 0; i < splitOffsets.length; i++) {
+        const { start, end, node } = splitOffsets[i]
+        const isLastSegment = i === splitOffsets.length - 1
+        if (point.offset >= start && (point.offset < end || (isLastSegment && point.offset === end))) {

Conversation

samuelpecher commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

samuelpecher commented Apr 24, 2026

Manual Validation: Validated

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

samuelpecher commented Apr 24, 2026 •

edited

Loading