Repository navigation
M3X-1480: Fix tables redlining and refactor the package - #8
Conversation
sirenevenkii
left a comment
There was a problem hiding this comment.
didn't looked at the actual logic yet.
sirenevenkii
left a comment
There was a problem hiding this comment.
I didn't check all the code, but I made some sanity checks using AI. I think we're good. great job overall 👍
one thing that worth looking at is performance. I did some testing with AI and it seems like there are cases where it behaves suboptimal. this PR is big enough and I think the goal is achieved, but we need to have a look at it separately.
Performance results (AI response as is)
The PR's speed on tables depends on how heavily they were edited: it's 3–4× slower on large, lightly edited or unchanged tables and 3–7× faster on heavily edited ones. Content without tables runs at the same speed as on develop. The slow path scales quadratically with row count.
| Workload | Size | develop | PR | PR vs develop |
|---|---|---|---|---|
| Text, 5% of words edited | 5 / 51 / 505 KB | 0.66 / 21.3 / 1999 ms | 0.75 / 21.0 / 1878 ms | same (0.94–1.13×) |
| Text, identical | 505 KB | ~0 ms | 4.9 ms | scan overhead only |
| Table 10×5, light edits | 4 KB | 0.46 ms | 1.7 ms | 3.7× slower |
| Table 50×6, 10% cells + a row added/deleted | 21 KB | 7.3 ms | 24.7 ms | 3.4× slower |
| Table 200×6, 10% cells + a row added/deleted | 82 KB | 96 ms | 301 ms | 3.1× slower |
| Table 200×6, cell ids on rows, 10% cells | 92 KB | 78 ms | 294 ms | 3.8× slower |
| Table 200×6, identical | 82 KB | ~0 ms | 295 ms | develop returns at once |
| Table 200×6, 60% cells edited | 82 KB | 182 ms | 30 ms | 6× faster |
| Table 300×8, long cells, 60% edited | 403 KB | 1483 ms | 218 ms | 6.8× faster |
| 20 sections, each with a 10×4 table, all edited | 66 KB | 56 ms | 52 ms | same |
Scaling with row count, for a table with 6 columns where 5% of cells changed:
| Rows | 50 | 100 | 200 | 400 | 800 |
|---|---|---|---|---|---|
| develop | 7 ms | 22 ms | 110 ms | 368 ms | 1.8 s |
| PR | 23 ms | 83 ms | 295 ms | 1.16 s | 4.5 s |
| PR, identical table | 25 ms | 78 ms | 289 ms | 1.12 s | 4.3 s |
Ticket
Tables: tables are now compared as whole structures. Each
old/newtable pair becomes one merged table, whole rows and columns are marked as added or deleted, merged cells are handled, and each cell content is diffed throughdiffCoreword level diff.Refactoring: the single
js/htmldiff.jsis split into modules.src/core/holds the existing word level diff (tokens, matching, operations, rendering) andsrc/tables/holds the new table code, while the public API(export = diff)stays the same.TypeScript: the library and tests are now written in TS, built from
src/intojs/with generated.d.tsfiles (sojs/is no longer committed). Tests run throughmocha + ts-node, and publishing does a clean build plus the CLI build.Rich text tables
rich-text-tables.mov
Step list table
step-list.mov
Document section table
document-section.mov
Caution
Still need to clean up client and small css update and for document sections need to add a
html-diff-idto cells so algo understands which is actually the real primary item ref as multiple item refs are involved in many section.