perf(editor): stop serializing table markup that carries no information - #1487
Conversation
The chain prices a comment mostly on history_bytes, which is the serialized transaction size, so wrapper bytes are a direct cost to the author. A 46,620-byte post was rejected for insufficient RC needing more RC than the account's entire maximum, and most of that size was markup that renders identically without it. TipTap writes colspan="1" rowspan="1" on every cell whether or not it spans anything, and wraps every cell's content in a paragraph. On the two tables in that post: 940 cells carrying 22,560 bytes of span attributes and 6,580 bytes of paragraph wrappers. Spans of 1 are the default, so they are dropped. A cell whose only child is an unstyled paragraph with no nested block content is unwrapped, since it displays identically. Everything else is left alone: real spans, alignment on the paragraph, multi-block cells, lists and nested tables all keep their structure, which is the part that carries meaning. Measured on the real tables from that post: 44,288 bytes to 15,147, a 66% reduction, taking the whole transaction from 46,620 to 17,479 bytes. Empty cells now serialize as <td></td>. That is intentional and safe: parse-all-extensions-to-doc refills them with a paragraph on load, which is the guard added for the paste path.
|
Warning Review limit reached
Next review available in: 56 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by Qodoperf(editor): slim TipTap table HTML by removing default spans and wrappers
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69fea05998
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| !child.querySelector(CELL_BLOCK_CHILDREN) | ||
| ) { | ||
| cell.innerHTML = child.innerHTML; |
There was a problem hiding this comment.
When a table cell contains an inline image, such as <td><p>before <img ...> after</p></td>, this branch unwraps the paragraph because img is not in CELL_BLOCK_CHILDREN. The publish editor explicitly configures images as inline (use-publish-editor.ts:177-179), while the Ecency stylesheet only overrides Tailwind's block image display under .markdown-view p img (_markdown.scss:133-140). Removing the <p> therefore makes the image render as a block and splits the surrounding cell text across lines, so paragraphs containing images should retain their wrapper.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1767240. img now joins the list of content that requires its wrapper, so a cell containing an image keeps its paragraph and .markdown-view p img { display: inline-block } still applies.
Three specs cover it: before <img> after keeps the paragraph, an image-only cell keeps it too, and a cell that merely mentions the word "image" in text is still unwrapped.
Code Review by Qodo
1. Internal extensions module mocked
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Review follow-up. Both findings were correct, and the second one falsified this PR's central claim that nothing renders differently. Cells holding an inline image now keep their paragraph. `.markdown-view p img` sets display:inline-block to keep an image aligned with the text around it, so promoting the image to a bare cell child loses that and can break "before <img> after" across lines. `img` joins the list of content that requires its wrapper. Unwrapped cells were also losing the typography the `p` rule supplies, line-height 1.55 and text-rendering optimizeLegibility, so ordinary tables rendered tighter. Measured at a 700px column: cells went from 42px to 36px, both single-line and wrapped. The table rule now sets the same values on td and th, which restores an exact match and is a no-op for cells that kept a paragraph, since the values are identical. Parity is measured in a headless browser now rather than asserted.
1 participant
The chain prices a comment mostly on
history_bytes, which is the serialized transaction size, so wrapper bytes are a direct cost to the author. A 46,620-byte post was recently rejected for insufficient RC, needing more RC than the account's entire maximum, and most of that size was markup that renders identically without it.What is being removed
TipTap writes
colspan="1" rowspan="1"on every cell whether or not it spans anything, and wraps every cell's content in a paragraph:44 bytes of wrapper around 11 bytes of content. Across the 940 cells in that post's two tables: 22,560 bytes of span attributes and 6,580 bytes of paragraph wrappers.
A span of 1 is the default, so it is dropped. A cell whose only child is an unstyled paragraph with no nested block content is unwrapped, since it displays identically.
What is deliberately left alone
This is where the value is, so each case has a spec:
colspan="2",rowspan="3"<p style="text-align: right">in a cellMeasured
On the real tables from the post that failed:
One intentional consequence
Empty cells now serialize as
<td></td>rather than<td><p></p></td>. That is safe becauseparse-all-extensions-to-docrefills empty cells with a paragraph on load, which is the guard added in #1484 for the paste path. Covered by a spec here too.Scope
This is the low-risk half of the size problem: pure serialization cleanup with no representational change. The larger win, emitting GFM markdown for tables that can be expressed in it, is a follow-up and needs a capability check so nested tables and block-content cells keep using HTML.
Full suite green: 2707 tests / 281 files. Typecheck clean, no new lint.