Repository navigation
Conversation
… with punctuation next to a letter as Markdown that keeps its format ## Description A run of tags with punctuation on its inner side and a letter or digit on its outer side cannot close or open by CommonMark's flanking rules (https://spec.commonmark.org/0.31.2/#left-flanking-delimiter-run). The export wrote such runs as they came: bold `Hello,` followed by plain `world` became `**Hello,**world`, `x` followed by italic `(beta)` became `x*(beta)*`, and `$convertFromMarkdownString` (and any CommonMark parser) read the asterisks back as text, so a save and reload lost the format. The export now writes that letter or digit as a numeric character reference, which is punctuation to the run and the same character once imported: `**Hello,**world`, `x*(beta)*`. The text a reader sees and every other export stay as they were; only the character that touches a run that could not flank changes. It covers: - one side's tags closing or opening next to the other side's text (the cases above), by both definitions of punctuation in play: CommonMark's (Unicode P and S, so `*€*5` becomes `*€*5`) and this package's import (ASCII); - tags of two kinds closing or opening together (`**==a,==**`), where the outer run has the inner run's delimiter on its inner side, so only white space or punctuation may touch it; - one run that closes one side's tags and opens the other's (`**a,***b*`), which flanks both ways only with the same kind of character on both sides. A character written as a reference changes both ends of a text of one character, so the decision runs over the text siblings around a node up to the first text of more than one character (bounded at 64 nodes, so a long chain stays linear). Closes #9660 ## Test plan `unit/EmphasisFlanking.test.ts`: the six cases of the issue plus inline code, Unicode punctuation and a shared run, each with its exact Markdown and an import that gives back every character with its formats; five controls that export as before; and a property over 1,500 random paragraphs of two to four text nodes (letters, digits, ASCII and Unicode punctuation, spaces; bold, italic, strikethrough, highlight, code), round-tripped through export and import. ### Before On main (c363dd9): ``` $ vitest --run --project unit packages/lexical-markdown/src/__tests__/unit/EmphasisFlanking.test.ts × [["Hello,",["bold"]],["world",[]]] exports as **Hello,**world and imports the same × [["Note:",["bold"]],["see",[]]] exports as **Note:**see and imports the same × [["x",[]],["(beta)",["italic"]]] exports as x*(beta)* and imports the same × [["a.",["italic"]],["b",[]]] exports as *a.*b and imports the same × [["(",["bold"]],["b",[]]] exports as **(**b and imports the same × [["*",["bold"]],["b",[]]] exports as **\***b and imports the same × [["x",[]],["tag",["bold","code"]]] exports as x**`tag`** and imports the same × [["€",["italic"]],["5",[]]] exports as *€*5 and imports the same × [["a,",["bold"]],["b",["italic"]]] exports as **a,***b* and imports the same ✓ control: ... (5) × 1500 random paragraphs Tests 10 failed | 5 passed (15) ``` Main's Markdown for the first seven loses the format on import; `*€*5` and `**a,***b*` import back here (this package's import counts only ASCII punctuation and is lenient on the shared run) but not in a CommonMark parser (markdown-it renders them as `*€*5` and `**a,**<em>b</em>`). ### After ``` $ vitest --run --project unit packages/lexical-markdown/src/__tests__/unit/EmphasisFlanking.test.ts Tests 15 passed (15) $ LEXICAL_MARKDOWN_SEEDS=8000 vitest --run --project unit packages/lexical-markdown/src/__tests__/unit/EmphasisFlanking.test.ts Tests 15 passed (15) $ vitest --run --project unit packages/lexical-markdown Test Files 10 passed (10) Tests 939 passed (939) $ pnpm run ci-check EXIT 0 $ vitest --run --project unit --project scripts-unit Test Files 657 passed | 4 skipped (661) Tests 28184 passed | 424 expected fail | 135 skipped (28743) ``` The exported Markdown of 2,137 random paragraphs (the property without highlight, which CommonMark lacks) also parses in markdown-it (CommonMark) to the same characters and formats, 0 mismatches (a scratch check, not committed: markdown-it is not a dependency of this package). The playground's Markdown e2e specs were not run.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
zurfyx
requested review from
acywatson,
etrepum,
ivailop7 and
potatowagon
as code owners
October 31, 2026 05:36
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
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: |
…flank, as written, and leave every other export byte for byte as it was ## Description Review follow-up. The first version decided per text node, from a model of the tags the export would write, which character to write as a reference. The model missed tags the export closes and opens again in one run (`****` between two nodes that share some formats), counted ASCII-only punctuation as CommonMark does not, stopped following one-character texts after 64 nodes, and could write a C1 control as a reference (`€`, which CommonMark reads as `€`). So it changed exports that already read back (`*(beta)***€**` got `€`) and broke one (`***😀****a\_b*`). Now the export of a block's inline content is built as pieces (tags, the first and last character of each text node, everything else as written), and each run of tags is checked against the characters actually next to it. A run is changed only where it fails CommonMark's flanking rules both as CommonMark classes characters and as parsers that read UTF-16 units do (they disagree on emoji and other characters outside the Basic Multilingual Plane); then the letter or digit on the failing side is written as a reference, and the runs next to that character are checked again, over the whole block. Where this package's import, which counts only ASCII punctuation, would then see a reference next to a character it takes for a letter (`€`), that character is written as a reference too. Only letters, marks, digits, symbols and punctuation are ever written as references (never control characters). Markdown that every reading accepts as written is unchanged. ## Test plan `EmphasisFlanking.test.ts` now reads every case back with this package's import and with a CommonMark parser (micromark, through @lexical/mdast), and adds the review's byte-identity controls (`*(beta)***€**`, `**a.***€*`, `*—***.**`), the emoji run parsers disagree on (unchanged), and a C1 control (unchanged); the properties run 1,500 random paragraphs through this package's import (ASCII punctuation, all formats) and through micromark (Unicode punctuation; bold, italic, strikethrough, code). ### Before On 3f30041, with the review's repros and six parsers (commonmark.js, markdown-it, marked, micromark, micromark with GFM): ``` review Regressions: byte-identity controls fail (`*(beta)***€**` and two more), shared-format reference fails commonmark.js review C1: `**Hello,**€` reads back as `€` ``` On main (c363dd9): review Contexts 9 of 15 and Chain 7 of 7 fail (the issue). ### After ``` $ vitest --run --project unit packages/lexical-markdown/src/__tests__/unit/EmphasisFlanking.test.ts Tests 20 passed (20) $ LEXICAL_MARKDOWN_SEEDS=6000 vitest --run ... EmphasisFlanking.test.ts Tests 20 passed (20) review repros on this head: Contexts 15/15, Chain 7/7 (16 to 256 one-character nodes), C1 5/5, Reference 1/1, Regressions: the three byte-identity controls and commonmark.js pass; micromark, markdown-it and marked read the shared-format case (`***😀****a\_b*`, byte for byte main's export) as main does. $ pnpm run ci-check EXIT 0 $ vitest --run --project unit --project scripts-unit Test Files 657 passed | 4 skipped (661) Tests 28189 passed | 424 expected fail | 135 skipped (28748) ``` A corpus of 6,000 random paragraphs (ASCII and Unicode punctuation, emoji, `*`, `_`, a C1 control; bold, italic, code), compared with main's export: 939 differ, none where main's export read back in all four parsers and this package's import; 0 parser regressions; 847 to 867 cases fixed per parser; this package's import fixed in 531 and broken in 1 (`*—\_***Bc:…`, where main's export failed every parser; this package's import misreads the run after the escaped `\_`; not investigated further).
zurfyx
marked this pull request as draft
October 31, 2026 06:04
![meta-code-review[bot]](https://avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
This looks good to me. A numeric character reference keeps the text identical while putting punctuation on the run's outer side, which is exactly what CommonMark's flanking rules (§6.2) need, and the new tests fail on main and pass here. A few optional, non-blocking suggestions:
- Preserve-newlines mode and a literal backslash (
packages/lexical-markdown/src/MarkdownExport.ts:1329). WithshouldPreserveNewLinesset, backslashes are written as typed, buttextPiecestreats\as part of a unit only when ASCII punctuation follows it. So plaina\xfollowed by italic(beta)exports asa\x*(beta)*. Both$convertFromMarkdownStringand a CommonMark parser then read the text back asax, because\&is an escaped ampersand (§2.4). Matching\\[\s\S]in place of\\[!-/:-@[-`{-~]leaves default mode unchanged, since every backslash there already precedes the character it escapes, and stopsencodefrom rewriting a character a backslash precedes. - Export cost on large documents (same function).
text.match(/…|[\s\S]/gu)allocates one string per character of every text node, although only the first and last units are used. In a quick measurement, exporting 2,000 paragraphs (2.6 MB of Markdown) went from 43 ms to 159 ms, and a single 2.2 MB text node from 1 ms to 114 ms. Taking the first unit with an anchored match and the last by scanning back from the end would avoid that. - Tests for the other entry points.
EmphasisFlanking.test.tscovers$convertToMarkdownStringwith default options. One case through$convertSelectionToMarkdownStringand one withshouldPreserveNewLines, which would also cover point 1, would pin the paths that share the new pass. - This package's own import on a few mixed-format shapes. In a 20,000-paragraph random comparison, three exports that
$convertFromMarkdownStringread back onmainare misread on this branch, even though CommonMark parsers now read them correctly. For example, bold€7, italic_a~, italic+highlight±*:and bold+strikethrough«export as**€7***\_a\~==±\*:==***~~«~~**, and the import makes_a~bold as well. The cause is the importer's pairing of a both-flanking***run, like the*—\_***Bc:…case you noted, so a follow-up issue seems enough.
The changeset follows .changeset/README.md and includes the Verify: paragraph that CONTRIBUTING.md asks for on fixes that change output.
| */ | ||
| function textPieces(text: string): Piece[] { | ||
| const units = | ||
| text.match(/\\[!-/:-@[-`{-~]|&#(?:\d+|[xX][\da-fA-F]+);|[\s\S]/gu) || []; |
There was a problem hiding this comment.
Two optional suggestions for this line:
- With
shouldPreserveNewLines, backslashes are written as typed, so plaina\x+ italic(beta)becomesa\x*(beta)*, and that reads back asax(\&is an escaped ampersand, CommonMark §2.4). Taking\\[\s\S]as one unit, in place of\\[!-/:-@[-`{-~], leaves default mode unchanged and keepsencodefrom touching a character a backslash precedes. match(...gu)allocates one string per character of every text node, although only the first and last units are used. Exporting 2.6 MB of Markdown went from 43 ms to 159 ms in a quick measurement. An anchored match for the first unit and a backward scan for the last would avoid that.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Parked (10/30)
Parked after review. Fixing CommonMark flanking on export conflicts with this package's own import, which does not follow CommonMark around escaped delimiters and counts only ASCII punctuation: a 6,000-case corpus of three nodes around escaped delimiters finds 28 exports that main reads back in this package (while CommonMark readers fail on main there) and that this change breaks for this package's import (italic
_, boldx, plain_exports as*\_***x**\_, which loses the bold and leaves literal asterisks; italicx, bold*, italicxgets italic on the star). The likely way forward is to make the import CommonMark-conformant first (escapes next to runs, Unicode punctuation), then fix the export on top of it, or both together. The tests and the review's repros are kept on this branch; #9660 stays open.Description
A run of tags with punctuation on its inner side and a letter or digit on its outer side cannot close or open by CommonMark's flanking rules (https://spec.commonmark.org/0.31.2/#left-flanking-delimiter-run). The export wrote such runs as they came: bold
Hello,followed by plainworldbecame**Hello,**world,xfollowed by italic(beta)becamex*(beta)*, and$convertFromMarkdownString(and any CommonMark parser) read the asterisks back as text, so a save and reload lost the format.The export now writes that letter or digit as a numeric character reference, which is punctuation to the run and the same character once imported:
**Hello,**world,x*(beta)*. The inline Markdown of a block is built as pieces (tags, the first and last character of each text node, everything else as written), and each run of tags is checked against the characters actually next to it; a run is changed only where it fails CommonMark's flanking rules both as CommonMark classes characters (Unicode P and S are punctuation) and as parsers that read UTF-16 units do (they disagree on emoji). Then the letter or digit on the failing side is written as a reference and the runs next to it are checked again, over the whole block (so a chain of one-character texts of any length works). Where this package's import, which counts only ASCII punctuation, would see a written reference next to a character it takes for a letter (€), that character is written as a reference too. Control characters are never written as references (€reads as€). Markdown that every reading accepts as written stays byte for byte as it was.Closes #9660
Review follow-up (second commit)
The first version modelled the tags per text node: it changed exports that already read back (
*(beta)***€**), missed runs that close and reopen a tag (****), stopped after 64 one-character texts and could write C1 controls as references. The second commit replaces it with the check on the actual runs described above; details and numbers in its message.Test plan
unit/EmphasisFlanking.test.ts: the six cases of the issue plus inline code and a shared run, each with its exact Markdown, read back by this package's import and by a CommonMark parser (micromark, through @lexical/mdast); controls that must export byte for byte as main does (including*(beta)***€**,**a.***€*,*—***.**, an emoji run parsers disagree on, a C1 control); and two properties over 1,500 random paragraphs each: through this package's import (ASCII punctuation; bold, italic, strikethrough, highlight, code) and through micromark (Unicode punctuation; bold, italic, strikethrough, code).Before
On main (c363dd9):
Main's Markdown for the first seven loses the format on import;
*€*5and**a,***b*import back here (this package's import counts only ASCII punctuation and is lenient on the shared run) but not in a CommonMark parser (markdown-it renders them as*€*5and**a,**<em>b</em>).After
A corpus of 6,000 random paragraphs (ASCII and Unicode punctuation, emoji,
*,_, a C1 control; bold, italic, code), compared with main's export and read by commonmark.js, markdown-it, micromark and marked: 939 exports differ, none where main's export read back in all four parsers and this package's import; 0 parser regressions; 847 to 867 fixed per parser; this package's import fixed in 531 and broken in 1 (*—\_***Bc:…, where main's export fails all four parsers and this package's import misreads the run after the escaped\_). The playground's Markdown e2e specs were not run.