Skip to content

fix(mdxish): preserve multi-paragraph table cells#1445

Merged
eaglethrost merged 5 commits into
nextfrom
dimas/rm-16217-editor-drops-n-new-lines-from-magic-block-table-cells-on
Apr 24, 2026
Merged

fix(mdxish): preserve multi-paragraph table cells#1445
eaglethrost merged 5 commits into
nextfrom
dimas/rm-16217-editor-drops-n-new-lines-from-magic-block-table-cells-on

Conversation

@eaglethrost
Copy link
Copy Markdown
Contributor

@eaglethrost eaglethrost commented Apr 23, 2026

🎫 Resolve RM-16217

🎯 What does this PR do?

Magic-block `[block:parameters]` tables (and actually JSX Tables as well) with cells containing paragraph breaks (`\n\n`) lose those breaks after a save through the MdxishEditor. A cell rendered as three paragraphs collapses into a single concatenated line (`para1para2para3`).

There are 2 issues causing this:

  1. The magic block table was getting serialised to GFM tables, which don't actually support multiline paragraphs in a table cell
  2. Even then, sometimes the newline / paragraph info got lost in the mdxish pipeline. This is because of a logic in the table transformer mdxish-tables.ts to flatten and unwrap paragraph nodes in a table cell, hence it'll become just an array of text nodes, which bunches them to one line

Fixes:

  1. tables-to-jsx: treat `>1` paragraph children in a cell as flow content, forcing JSX `` output instead of GFM
  2. jsx-to-mdast / mdxish-tables: only unwrap a paragraph child when it is the cell's sole paragraph. Multiple paragraphs stay intact. I think we still need to unwrap if there's only 1 paragraph as it might cause some visual issues
  3. 🧪 QA tips

    Paste the following into a doc with the MdxishEditor, save, and confirm the three paragraphs are preserved (not collapsed to `Line 1Line 2Line 3`):

    [block:parameters]
    {
      "data": {
        "h-0": "Item",
        "0-0": "Line 1 \n\nLine 2 \n\nLine 3"
      },
      "cols": 1,
      "rows": 1,
      "align": ["left"]
    }
    [/block]

    Also verify a JSX <Table> with blank-line-separated content in a cell round-trips cleanly:

    <Table align={["left"]}>
      <thead>
        <tr>
          <th>Item</th>
        </tr>
      </thead>
      <tbody>
        <tr>
          <td>
            Line 1
    
            Line 2
    
            Line 3
          </td>
        </tr>
      </tbody>
    </Table>
    • Magic-block table cell with `\n\n` preserves paragraphs after save
    • JSX <Table> cell with blank lines preserves paragraphs after save
    • Single-paragraph cells still render as plain GFM tables (no regression)

    📸 Screenshot or Loom

    BEFORE (Notice how the new lines get lost after the round-trip):

    Screen.Recording.2026-04-23.at.11.25.04.pm.mov

    AFTER (Serialising to JSX table and preserving newlines):

    Screen.Recording.2026-04-23.at.11.25.34.pm.mov

    NOTE that in the editor, the paragraph gaps are not exactly the same as in view mode. If we want to update that to be the exact same, we'd need to look into the editor side. In spite of that, the round-trip works and the view mode still renders with the correct paragraph gaps.

@eaglethrost eaglethrost changed the title fix(mdxish): preserve multi-paragraph table cells across round-trip (RM-16217) fix(mdxish): preserve multi-paragraph table cells Apr 23, 2026
const paragraphCount = cell.children.filter(c => c.type === 'paragraph').length;
const parsedChildren =
paragraphCount === 1
? cell.children.flatMap((parsedNode: Node) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This logic is kind of a duplicate to the one in mdxish-tables. I'm thinking of moving the table-related transformers under 1 subfolder under transformers/ so that it can host its own util file and share code.

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.

Yeah I agree with doing this. Also fine with putting an unwrapSoleParagraph (or whatever name makes sense) function into processor/utils. But we definitely don't want two duplicates that could drift if one gets updated without the other.

@eaglethrost
Copy link
Copy Markdown
Contributor Author

eaglethrost commented Apr 23, 2026

As I've alluded to in the note below the loom, even with this fix, the paragraphs in the table cell in the editor are only 1 line apart & different from the view mode. I don't think there's any more we can do in the engine though, and it would be a fix in the editor.

Unfortunately I think this part isn't a straightforward fix, one thing we could do is to add break tags to the tree to make sure the editor matches the view mode, but it will add it in the raw mode as well which I'm not sure if we want to do more of than we already do. Will think of that more.

Editor View
Screenshot 2026-04-23 at 11 44 13 pm Screenshot 2026-04-23 at 11 44 26 pm

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The as Table casts are to fix type errors

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.

This is because MDAST Table doesn't expect paragraphs in table cell children right?

Can we instead type the expected shape and use it here? I also guess we'd need to update a table transformer that is also casting unsafely.

We've got this types file that already holds transitional node shapes we could put a new Table & TableCell type in https://github.com/readmeio/markdown/blob/next/processor/transform/mdxish/types.ts

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah because the MDAST table cell children is typed as PhrasingContent[] which doesn't accept flow level nodes such as Paragraphs, Lists, etc.

Yes we can definitely extend the table types! I've had a thought about it, I think we should do it in a follow up so we can integrate the types more holistically and have some room for discussions as well. Will create one soon.

@@ -251,6 +257,7 @@ const mdxishTables = (): Transform => tree => {
// we let it get handled naturally
return EXIT;
}
return undefined;
Copy link
Copy Markdown
Contributor Author

@eaglethrost eaglethrost Apr 23, 2026

Choose a reason for hiding this comment

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

Lint error fix, shouldn't affect anything

Copy link
Copy Markdown
Contributor

@kevinports kevinports left a comment

Choose a reason for hiding this comment

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

Approving with a few comments that'd be good to address before merging.

As I've alluded to in the note below the loom, even with this fix, the paragraphs in the table cell in the editor are only 1 line apart & different from the view mode. I don't think there's any more we can do in the engine though, and it would be a fix in the editor.

Unfortunately I think this part isn't a straightforward fix, one thing we could do is to add break tags to the tree to make sure the editor matches the view mode, but it will add it in the raw mode as well which I'm not sure if we want to do more of than we already do. Will think of that more.

Yeah it'd definitely be nice to clean this up. Could it just be a CSS styling difference between view vs edit rendering?

const paragraphCount = cell.children.filter(c => c.type === 'paragraph').length;
const parsedChildren =
paragraphCount === 1
? cell.children.flatMap((parsedNode: Node) => {
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.

Yeah I agree with doing this. Also fine with putting an unwrapSoleParagraph (or whatever name makes sense) function into processor/utils. But we definitely don't want two duplicates that could drift if one gets updated without the other.

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.

This is because MDAST Table doesn't expect paragraphs in table cell children right?

Can we instead type the expected shape and use it here? I also guess we'd need to update a table transformer that is also casting unsafely.

We've got this types file that already holds transitional node shapes we could put a new Table & TableCell type in https://github.com/readmeio/markdown/blob/next/processor/transform/mdxish/types.ts

@eaglethrost eaglethrost merged commit c821b2d into next Apr 24, 2026
7 checks passed
@eaglethrost eaglethrost deleted the dimas/rm-16217-editor-drops-n-new-lines-from-magic-block-table-cells-on branch April 24, 2026 06:42
rafegoldberg pushed a commit that referenced this pull request Apr 24, 2026
## Version 14.1.1
### 🛠 Fixes & Updates

* DRY up some test helpers ([#1437](#1437)) ([f761248](f761248))
* **mdxish:** preserve multi-paragraph table cells ([#1445](#1445)) ([c821b2d](c821b2d))
* use magic block tokenizer when parsing JSX component children ([#1447](#1447)) ([51b22a9](51b22a9)), closes [#1429](#1429)

<!--SKIP CI-->
@rafegoldberg
Copy link
Copy Markdown
Contributor

This PR was released!

🚀 Changes included in v14.1.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants