diff --git a/packages/app-cli/tests/HtmlToMd.ts b/packages/app-cli/tests/HtmlToMd.ts index 01f71ac739..fa7da34a7d 100644 --- a/packages/app-cli/tests/HtmlToMd.ts +++ b/packages/app-cli/tests/HtmlToMd.ts @@ -2,6 +2,7 @@ import shim from '@joplin/lib/shim'; import * as os from 'os'; import { readFile } from 'fs/promises'; import { filename } from '@joplin/lib/path-utils'; +import { dirname } from 'path'; import HtmlToMd, { ParseOptions } from '@joplin/lib/HtmlToMd'; import MdToHtml from '@joplin/renderer/MdToHtml'; @@ -130,6 +131,31 @@ describe('HtmlToMd', () => { expect(collapsedResult).toContain('\n\n'); }); + it('should not re-read the rows of a table for every row', async () => { + // Under domino, each access to `table.rows` walks the whole table, so reading it once per row made + // converting large tables quadratic: https://github.com/laurent22/joplin/issues/16894 + const resolveFrom = (moduleName: string, fromPath: string) => require.resolve(moduleName, { paths: [dirname(fromPath)] }); + const domino: { createDocument: (html: string)=> Document } = require(resolveFrom('@mixmark-io/domino', resolveFrom('@joplin/turndown', require.resolve('@joplin/lib/HtmlToMd')))); + + const rowCount = 100; + const rows = Array.from({ length: rowCount }, (_, i) => `${i}bc`).join(''); + const root = domino.createDocument(`
${rows}
ABC
`).querySelector('div'); + // Turndown converts a clone of the element it's given, which wouldn't have the counter below + Object.defineProperty(root, 'cloneNode', { value: () => root }); + + const table = root.querySelector('table'); + const getRows = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(table), 'rows').get; + let rowsReadCount = 0; + Object.defineProperty(table, 'rows', { get: () => { + rowsReadCount++; + return getRows.call(table); + } }); + + const markdown = new HtmlToMd().parse(root); + expect(markdown.split('\n')).toHaveLength(rowCount + 2); + expect(rowsReadCount).toBeLessThan(rowCount); + }); + // Covers how newlines are carried between children when joining them, which was rewritten to fix // https://github.com/laurent22/joplin/issues/16895 it.each([ diff --git a/packages/tools/cspell/dictionary4.txt b/packages/tools/cspell/dictionary4.txt index e1032747b5..92a2c30a3c 100644 --- a/packages/tools/cspell/dictionary4.txt +++ b/packages/tools/cspell/dictionary4.txt @@ -375,4 +375,5 @@ twodo Gemma untruncated instr +mixmark Hrvatski diff --git a/packages/turndown-plugin-gfm/src/tables.js b/packages/turndown-plugin-gfm/src/tables.js index a116a9bc83..9cc08fd6ad 100644 --- a/packages/turndown-plugin-gfm/src/tables.js +++ b/packages/turndown-plugin-gfm/src/tables.js @@ -29,8 +29,11 @@ function getColumnAlignment(table, columnIndex) { var align = ''; - for (var i = 0; i < table.rows.length; ++i) { - var row = table.rows[i]; + // Read `rows` once: under domino each access re-walks the whole table, which made converting large tables + // quadratic: https://github.com/laurent22/joplin/issues/16894 + var rows = table.rows; + for (var i = 0; i < rows.length; ++i) { + var row = rows[i]; if (columnIndex < row.childNodes.length) { var cellAlignment = getAlignment(row.childNodes[columnIndex]); ++votes[cellAlignment]; @@ -371,8 +374,9 @@ function handleColSpan(content, node, emptyChar) { function tableColCount(node) { let maxColCount = 0; - for (let i = 0; i < node.rows.length; i++) { - const row = node.rows[i] + const rows = node.rows; + for (let i = 0; i < rows.length; i++) { + const row = rows[i] const colCount = row.childNodes.length if (colCount > maxColCount) maxColCount = colCount }