Skip to content

fix(diff-source): preserve source edits when inserting Markdown - #995

Open
huyanxius wants to merge 5 commits into
mdx-editor:mainfrom
huyanxius:fix/source-insert-markdown-636
Open

huyanxius wants to merge 5 commits into
mdx-editor:mainfrom
huyanxius:fix/source-insert-markdown-636

Conversation

@huyanxius

@huyanxius huyanxius commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #636.

Problem

insertMarkdown() always updates the hidden Lexical editor. In source and diff mode that editor can still contain the last rich-text document, so an insertion replaces newer source edits with stale content.

Changes

  • Route source/diff insertions through CodeMirror's current selection. This preserves raw Markdown, multiple selections, native undo/redo, and the existing change notifications.
  • Keep source and diff on the same raw document; import into Lexical when returning to rich text. Importing during a source-to-diff handoff otherwise lets a delayed Lexical update overwrite the new insertion.
  • Apply an insertion immediately to a CodeMirror state when a mode switch has happened but the view has not mounted yet. Transfer that state's selection and history to the mounted view, and honor custom transaction filters. This also preserves content if another mode switch cancels that mount.
  • Add component regressions, an interactive example, and two Playwright scenarios covering source and diff.

The public API and the existing rich-text selection requirement are unchanged.

Verification

Latest head: 53887ad239839a53c9128d66098310023032c1de, incorporating current main and the Firefox clipboard fixture correction.

  • Upstream CI: library build, unit tests, documentation build, and native browser tests all passed.
  • Local full unit/component suite: 236 passed, with the existing 1 skipped and 1 todo.
  • Local typecheck, lint, and library build passed.
  • Independent regression review: 10 cases passed, including immediate mode switches and preserved selection/history.

The expanded regressions cover external setMarkdown, source-to-diff transitions, StrictMode remounts, pending insertions, selection/history preservation, and read-only behavior. Native browser verification is now complete through upstream CI.

@petyosi petyosi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. Routing source/diff insertions through CodeMirror is the right direction, and the tests separate the old and new behaviour well (20 of the 27 unit tests fail on main). A few things need to change before this can go in.

1. setMarkdown is dropped after a source → diff switch (src/plugins/core/index.ts, the viewMode$ subscription)

Source ↔ diff switches no longer publish setMarkdown$, so markdown$ keeps the last rich-text value. setMarkdown$ then compares new values against that stale value (newMarkdown.trim() !== oldMarkdown.trim()) and drops calls that should change the document.

Repro: load A, edit to B in source mode, switch to diff, call setMarkdown('A') (e.g. a "reset" button). The editor keeps showing B. On main it shows A.

The single-mode case is already broken on main, but this PR makes it reachable through ordinary mode switches. In source/diff mode, compare against markdownSourceEditorValue$ and publish the new document to it directly. Re-publishing an unchanged markdown$ won't emit, so the existing link can't carry it.

2. Pending selection/history is lost under StrictMode (getSourceEditorConfig in diff-source/index.tsx)

getSourceEditorConfig clears pendingSourceEditorState$ as it reads it. StrictMode's throwaway first mount takes the pending state, and the real mount gets only { doc }: cursor at 0 and nothing to undo. Content is kept, but the "selection and history carry over" guarantee doesn't hold in development. Please make the read side-effect free (the doc-equality check already covers stale state) and clear the pending state on viewMode$ changes instead.

3. Inserting right after source → diff ignores the source cursor

When sourceEditorView$ still holds the outgoing view, the pending state is built from scratch with the cursor at 0. Put the source cursor at the end of Original rich text, switch to diff, call insertMarkdown('X'), and you get XOriginal rich text. Building the pending state from editor.view.state when an outgoing view exists keeps both the selection and the history.

Smaller things

  • EditorState.readOnly.of(...) in the pending state has no effect, because programmatic replaceSelection ignores it. Either drop it or add an explicit readOnly$ guard in the handler.
  • The pending-state construction is duplicated between the insert handler and getSourceEditorConfig. A shared helper would keep them from drifting.
  • Please note the behaviour change in the description: source ↔ diff no longer round-trips through Lexical, so the parse-error banner and onError now fire only when returning to rich-text.

@huyanxius
huyanxius marked this pull request as ready for review October 16, 2026 17:14

This branch has not been deployed

No deployments
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.

[BUG] insertMarkdown() doesn't work correctly when editing in "source" mode

2 participants