Repository navigation
[lexical-mdast] Feature: Add a horizontal rule shortcut for ---, *** and ___ - #9484
conghuydever wants to merge 6 commits into
Conversation
…and ___ MdastShortcutsExtension had typing shortcuts for headings, quotes, lists and code blocks, but none for a horizontal rule. A line of ---, *** or ___ stayed text and was exported escaped, as \---. Now ---, *** or ___ alone on a line, followed by Space or Enter, becomes a HorizontalRuleNode, with the caret in a new paragraph under it. The rule keeps the typed marker, so export writes the same one. Like the other block shortcuts, it needs MdastHorizontalRuleExtension in the editor. At the start of a quote the rule goes into the quote. With text after the marker, or in a table cell, the line stays text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
lexical
@lexical/a11y
@lexical/clipboard
@lexical/code
@lexical/code-core
@lexical/code-prism
@lexical/code-shiki
@lexical/compiler
@lexical/devtools-core
@lexical/dragon
@lexical/eslint-plugin
@lexical/extension
@lexical/fast-check
@lexical/file
@lexical/hashtag
@lexical/headless
@lexical/history
@lexical/html
@lexical/internal
@lexical/link
@lexical/list
@lexical/mark
@lexical/markdown
@lexical/mdast
@lexical/offset
@lexical/overflow
@lexical/plain-text
@lexical/react
@lexical/rich-text
@lexical/selection
@lexical/table
@lexical/tailwind
@lexical/text
@lexical/utils
@lexical/y
@lexical/yjs
commit: |
![meta-code-review[bot]](https://avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for this! The shortcut behaves as described: in the mdast-editor dev example, ---, *** or ___ converts on Space, on Enter and at the start of a > quote, and Undo restores the typed marker. The tests are solid too: the seven conversion cases fail on main and pass here, and the table-cell cases really guard their check.
Please change before merge
-
@lexical/tablenow ends up in every bundle that uses the shortcuts (packages/lexical-mdast/src/MdastShortcuts.ts:27, used at line 116).$isTableCellNodekeepsTableCellNodereachable fromMdastShortcutsExtension, so editors without tables ship it too.MdastTableExtensionis documented as opt-in "because it pulls in the@lexical/tablenodes it ships" (packages/lexical-mdast/src/MdastTableExtension.ts), andpackages/lexical-website/docs/serialization/markdown-mdast.mdsays each feature extension ships the nodes it needs. The repository's own size probe (dev-examples/mdast-editor:pnpm run build-prod, thenpnpm run size; CommonMark, strikethrough, task lists and shortcuts, no tables) grows from 483,772 to 493,377 bytes minified and from 122,600 to 125,252 bytes gzipped. One way to keep the behavior without the import is to convert only where Markdown can write a rule, at the top level or inside a quote, replacing the cell check with$findMatchingParent(container.getParentOrThrow(), node => !$isQuoteNode(node))
and dropping the
@lexical/tableimport. With that change all 38 tests inMdastShortcuts.test.tsstill pass, both table-cell cases included, and the table code leaves the bundle. LettingMdastTableExtensioncontribute the cell check would work too. -
Add a changeset. CONTRIBUTING.md requires one for every user-facing change, and this changes what
@lexical/mdastdoes when someone types---. AGENTS.md also asks for aVerify:paragraph when a change alters editing behavior. For example.changeset/mdast-thematic-break-shortcut.md:--- '@lexical/mdast': patch --- [feature] Type `---`, `***` or `___` on its own line, then Space or Enter, to insert a horizontal rule with `MdastShortcutsExtension` Verify: with `MdastHorizontalRuleExtension` (part of `MdastCommonMarkExtension`), a line that is only `---`, `***` or `___` becomes a horizontal rule on Space or Enter, with the caret on a new line below; a line with text after the marker stays text.
Smaller suggestions
MAX_BLOCK_MARKER_LENGTH(MdastShortcuts.ts:57) still skips the Space scan past column 24, so a rule of 24 or more characters converts on Enter but not on Space, while CommonMark (section 4.1) accepts "three or more matching-,_, or*characters". Either let a line of one repeated marker character past that cap or note the Enter-only behavior in the comment there.- The description notes inline code and Shift+Enter were not tested. Consider adding Undo cases for Space and Enter (the test editor needs
HistoryExtensionwithdisabled: false, since it is off when there is nowindow) and a---inside a code block case toMdastShortcuts.test.ts, so the undo step and the code exclusion stay pinned.
The CI workflows for this head are waiting for maintainer approval, so none of the test suites have run on it yet.
| if ( | ||
| line.length !== markerLength || | ||
| !container.getChildren().every($isTextNode) || | ||
| $findMatchingParent(container, $isTableCellNode) |
There was a problem hiding this comment.
This check is what pulls @lexical/table (TableCellNode and its generated JSON schema) into every bundle that uses MdastShortcutsExtension, editors without tables included, while MdastTableExtension is documented as opt-in to keep those nodes out. The repository's size probe (dev-examples/mdast-editor, pnpm run size after pnpm run build-prod) grows by 9,605 bytes minified and 2,652 gzipped with this PR.
One option that needs no import: convert only where Markdown can write a rule, at the top level or in a quote, and drop the import on line 27:
$findMatchingParent(container.getParentOrThrow(), node => !$isQuoteNode(node))All 38 tests in MdastShortcuts.test.ts still pass with it, both table-cell cases included.
There was a problem hiding this comment.
Done in 5c6917a, as suggested. The rule now converts only at the top level or inside a quote, and the @lexical/table import is gone.
With the size probe (pnpm run build-prod, then pnpm run size in dev-examples/mdast-editor), the mdast bundle goes from 493,377 to 484,294 bytes minified (481.8 kB to 472.9 kB as the script prints it, and 122.3 kB to 119.9 kB gzipped), and "tablecell" no longer appears in it. Both table-cell tests still pass, and they fail if the check is removed.
This also means --- typed in a container that isn't a quote (a layout or collapsible block, for example) stays text, since Markdown can't write a rule there either.
…le shortcut Convert a typed rule only at the top level or inside a quote, the places Markdown can write one, instead of checking for a table cell. This drops the @lexical/table import, so editors without tables no longer ship it. Let a space convert a rule longer than 24 characters, as Enter already does. Add tests for undo, a code block, inline code and Shift+Enter, and a changeset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the review. All points are handled in 5c6917a:
Checks: on the PR head, |
There was a problem hiding this comment.
This looks good, thanks for the follow-up. ---, *** and ___ convert on Space and on Enter, also at the start of a > quote and past 24 characters; the caret lands in the new paragraph, one Undo gives back the typed marker, and the export keeps the marker. The @lexical/table import is gone and the changeset follows CONTRIBUTING.md.
Two optional, non-blocking suggestions:
- Decline quietly when
HorizontalRuleNodeisn't registered (packages/lexical-mdast/src/MdastShortcuts.ts:135).scanBlockoffers the match whenever some import rule handlesthematicBreak, so an editor that mapsthematicBreakto its own node withoutHorizontalRuleExtensionnow getsNode HorizontalRuleNode has not been registeredinonErrorwhen someone types---; onmainthe line just stayed text. Adding!$getEditor().hasNode(HorizontalRuleNode)to the early-return checks in$applyThematicBreakwould avoid it. The heading shortcut has the same shape today, so this is fine to leave for later. - Refresh the Test plan output. AGENTS.md (Commits and Pull Requests) asks for the real Before and After output of the command you ran. The description still shows
Tests 38 passed (38)and says Shift+Enter and inline code aren't tested, butMdastShortcuts.test.tsnow has 47 tests, those cases included, and 11 of them fail onmain.
The CI workflows for this commit are waiting for maintainer approval, so none of the test suites have run on it yet.
| return false; | ||
| } | ||
| $stripLeading(container, markerLength); | ||
| const rule = $createHorizontalRuleNode(); |
There was a problem hiding this comment.
Optional, non-blocking: scanBlock offers this match whenever some import rule handles thematicBreak, so an editor that maps it to its own node without HorizontalRuleExtension gets Node HorizontalRuleNode has not been registered in onError when --- is typed. Adding !$getEditor().hasNode(HorizontalRuleNode) to the checks above would let the shortcut decline quietly. The heading shortcut behaves the same way today, so this can wait.
There was a problem hiding this comment.
Done in f785646. $applyThematicBreak now returns early when !$getEditor().hasNode(HorizontalRuleNode), so the line stays text.
I reproduced it first: an editor with MdastRichTextExtension, MdastExtension configured to import thematicBreak as a paragraph, and MdastShortcutsExtension, but no HorizontalRuleExtension, reported Node HorizontalRuleNode has not been registered. Ensure node has been passed to createEditor. to onError when --- was typed. A new test does this with Space and with Enter and checks that onError gets nothing; both cases fail without the check. I left the heading shortcut as it is.
…as no HorizontalRuleNode ## Description An editor can import thematicBreak as its own node, without HorizontalRuleExtension. Typing --- and then Space or Enter there reported "Node HorizontalRuleNode has not been registered" to onError, while on main the line stays text. The horizontal rule shortcut now checks that the editor has HorizontalRuleNode first, and leaves the line as text when it doesn't. ## Test plan ### Before $ pnpm exec vitest run packages/lexical-mdast/src/__tests__/unit/MdastShortcuts.test.ts × --- + space stays text when the editor has no HorizontalRuleNode × --- + Enter stays text when the editor has no HorizontalRuleNode Tests 2 failed | 47 passed (49) ### After $ pnpm exec vitest run packages/lexical-mdast/src/__tests__/unit/MdastShortcuts.test.ts Tests 49 passed (49) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the approval. Both suggestions are handled:
|
There was a problem hiding this comment.
This looks good, thanks for the follow-up. ---, *** and ___ convert on Space and on Enter, also at the start of a > quote and past 24 characters; the caret lands in the new paragraph, one Undo gives back the typed marker, and the export keeps the marker. The new hasNode(HorizontalRuleNode) check keeps the line as text in editors that import thematicBreak as their own node, the Test plan now carries real Before and After output as AGENTS.md asks, and the changeset follows CONTRIBUTING.md.
One optional, non-blocking suggestion:
- Pin the all-text-children guard with a test (
packages/lexical-mdast/src/MdastShortcuts.ts:135). Every other guard in$applyThematicBreakhas a test that fails without it, but removing!container.getChildren().every($isTextNode)leaves all 49 tests inMdastShortcuts.test.tspassing. That check is what keeps---followed by an inline node with no text (an inline decorator) from converting and dropping that node along with the paragraph; a Space case and an Enter case with such a node would keep it from regressing.
The CI workflows for this commit are waiting for maintainer approval, so none of the test suites have run on it yet.
| if ( | ||
| !$getEditor().hasNode(HorizontalRuleNode) || | ||
| line.length !== markerLength || | ||
| !container.getChildren().every($isTextNode) || |
There was a problem hiding this comment.
Optional, non-blocking: this is the one guard in $applyThematicBreak that no test pins; removing it leaves all 49 tests in MdastShortcuts.test.ts passing. It keeps --- followed by an inline node with no text (an inline decorator) from converting and dropping that node along with the paragraph, so a Space case and an Enter case with such a node would keep it covered.
Description
MdastShortcutsExtensionhas typing shortcuts for headings, quotes, lists and code blocks, but none for a thematic break. A line of---,***or___stays text, and the export escapes it:main---Spaceafter\--- after---\n\nafter***Spaceafter\*\*\* after***\n\nafter>Space---Spaceafter> \--- after> ---\n>\n> afterWith this PR,
---,***or___alone on a line, followed by Space or Enter, turns the line into aHorizontalRuleNode, and the caret goes to a new empty paragraph under it.MarkdownStreamScanner.scanBlocknow returns athematicBreakmatch, with the whole line as the marker, as for a code fence. Like the other block shortcuts, it only does so when the registry importsthematicBreak, so whenMdastHorizontalRuleExtension(part ofMdastCommonMarkExtension) is in the editor.$applyThematicBreakputs the rule in place of the line, and adds the empty paragraph for the new line the key asked for.hrMarkerState, the same state import sets, so a typed___is exported as___.>, the rule goes into the quote, through the same$placeBlocka heading or list uses.MAX_BLOCK_MARKER_LENGTH(24). A rule of 24 or more characters converts on Space as it does on Enter.The line stays text:
--- textis a paragraph in Markdown. The space path only scans the text up to the caret, so a space typed between---andtextwould match without its own check.---typed in a cell, a common placeholder, would move out of the table on export. The check looks at the ancestors instead of for a table cell, so@lexical/tablestays out of editors that don't use tables.HorizontalRuleNode, for example because it importsthematicBreakas its own node withoutHorizontalRuleExtension. Without this check, typing---there would reportNode HorizontalRuleNode has not been registeredtoonError.* * *and- - -don't convert when typed, because*and-make a list first.The size probe (
pnpm run build-prod, thenpnpm run sizeindev-examples/mdast-editor) gives 484,315 bytes minified for the mdast bundle with this PR, against 483,772 onmainas measured in the review.AI disclosure: the code, tests and this description were prepared with AI assistance.
Changeset
.changeset/mdast-thematic-break-shortcut.md:@lexical/mdastpatch,[feature], with aVerify:paragraph.Test plan
New cases in
packages/lexical-mdast/src/__tests__/unit/MdastShortcuts.test.ts:---,***and___, each with Space and with Enter: the root holds the rule and then the paragraph with the caret, and the export is the marker, an empty line andafter.---at the start of a quote: the rule and the next line are in the quote, and the export is> ---\n>\n> after.HistoryExtension,disabled: false).--and Enter, a space typed between---and text,---with Space or Enter in a table cell, in a code block and in inline code,---with Shift+Enter, and---with Space or Enter in an editor withoutHorizontalRuleNode(no error reported toonError).Before
The tests with
MdastShortcuts.tsandMdastStream.tsfrommain(e28f4dd):The stays-text cases pass on
maintoo, since nothing converts there. With this PR but without the ancestor check, both table cell cases fail; without theHorizontalRuleNodecheck, both no-HorizontalRuleNodecases fail; and without the scan past 24 characters, the long rule on Space fails.After
The four
lexical-devtoolsfiles could not load in my checkout because.wxt/tsconfig.json, whichwxt preparewrites on install, was missing; afterwxt preparethey pass, as above.changelog.test.tsfails on and off in my checkout, withENOTEMPTYwhile removing its own temp folder or a failedgit commitin its temp repository, a different case each run; this PR changes no file underscripts/.pnpm run tscis clean, andpnpm exec eslintonpackages/lexical-mdast/src,pnpm exec prettier --checkandpnpm run check-changesetsare clean.Tested on macOS with Node 24.21.0. Browser tests, E2E and the playground were not run. The playground does not use
MdastShortcutsExtension.🤖 Generated with Claude Code