From f72ccf38c0b1b781e52f68ea955be8008a05865b Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Wed, 15 Apr 2026 17:26:29 -0300 Subject: [PATCH] refactor(painter): delete dead hash helpers after blockLookup removal --- .../dom/src/paragraph-hash-utils.test.ts | 197 +----------------- .../painters/dom/src/paragraph-hash-utils.ts | 165 +-------------- .../painters/dom/src/renderer.ts | 10 - 3 files changed, 3 insertions(+), 369 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/paragraph-hash-utils.test.ts b/packages/layout-engine/painters/dom/src/paragraph-hash-utils.test.ts index 4ef72e7592..f1d9fcd66c 100644 --- a/packages/layout-engine/painters/dom/src/paragraph-hash-utils.test.ts +++ b/packages/layout-engine/painters/dom/src/paragraph-hash-utils.test.ts @@ -1,199 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { - getRunStringProp, - getRunNumberProp, - getRunBooleanProp, - getRunUnderlineStyle, - getRunUnderlineColor, - hashParagraphBorders, -} from './paragraph-hash-utils.js'; -import type { Run, TextRun, ParagraphBorders } from '@superdoc/contracts'; - -describe('paragraph-hash-utils', () => { - describe('getRunStringProp', () => { - it('returns string value when property exists', () => { - const run: TextRun = { text: 'hello', fontFamily: 'Arial', fontSize: 12, color: '#FF0000' }; - expect(getRunStringProp(run, 'color')).toBe('#FF0000'); - }); - - it('returns empty string when property does not exist', () => { - const run: TextRun = { text: 'hello', fontFamily: 'Arial', fontSize: 12 }; - expect(getRunStringProp(run, 'color')).toBe(''); - }); - - it('returns empty string when property is not a string', () => { - const run: TextRun = { text: 'hello', fontFamily: 'Arial', fontSize: 12, bold: true }; - expect(getRunStringProp(run, 'bold')).toBe(''); - }); - }); - - describe('getRunNumberProp', () => { - it('returns number value when property exists', () => { - const run: TextRun = { text: 'hello', fontFamily: 'Arial', fontSize: 12 }; - expect(getRunNumberProp(run, 'fontSize')).toBe(12); - }); - - it('returns 0 when property does not exist', () => { - const run: TextRun = { text: 'hello', fontFamily: 'Arial', fontSize: 12 }; - expect(getRunNumberProp(run, 'letterSpacing')).toBe(0); - }); - }); - - describe('getRunBooleanProp', () => { - it('returns boolean value when property exists', () => { - const run: TextRun = { text: 'hello', fontFamily: 'Arial', fontSize: 12, bold: true }; - expect(getRunBooleanProp(run, 'bold')).toBe(true); - }); - - it('returns false when property does not exist', () => { - const run: TextRun = { text: 'hello', fontFamily: 'Arial', fontSize: 12 }; - expect(getRunBooleanProp(run, 'bold')).toBe(false); - }); - }); - - describe('getRunUnderlineStyle', () => { - it('returns underline style when present', () => { - const run: TextRun = { - text: 'hello', - fontFamily: 'Arial', - fontSize: 12, - underline: { style: 'dashed' }, - }; - expect(getRunUnderlineStyle(run)).toBe('dashed'); - }); - - it('returns underline style with color present', () => { - const run: TextRun = { - text: 'hello', - fontFamily: 'Arial', - fontSize: 12, - underline: { style: 'wavy', color: '#00FF00' }, - }; - expect(getRunUnderlineStyle(run)).toBe('wavy'); - }); - - it('returns empty string when underline has no style', () => { - const run: TextRun = { - text: 'hello', - fontFamily: 'Arial', - fontSize: 12, - underline: { color: '#00FF00' }, - }; - expect(getRunUnderlineStyle(run)).toBe(''); - }); - - it('returns empty string when underline is not present', () => { - const run: TextRun = { text: 'hello', fontFamily: 'Arial', fontSize: 12 }; - expect(getRunUnderlineStyle(run)).toBe(''); - }); - - it('returns empty string for non-text runs without underline', () => { - const run: Run = { kind: 'lineBreak' }; - expect(getRunUnderlineStyle(run)).toBe(''); - }); - - it('returns "single" when underline is boolean true', () => { - const run = { text: 'hello', fontFamily: 'Arial', fontSize: 12, underline: true } as Run; - expect(getRunUnderlineStyle(run)).toBe('single'); - }); - - it('returns empty string when underline is boolean false', () => { - const run = { text: 'hello', fontFamily: 'Arial', fontSize: 12, underline: false } as Run; - expect(getRunUnderlineStyle(run)).toBe(''); - }); - }); - - describe('getRunUnderlineColor', () => { - it('returns underline color when present', () => { - const run: TextRun = { - text: 'hello', - fontFamily: 'Arial', - fontSize: 12, - underline: { color: '#FF0000' }, - }; - expect(getRunUnderlineColor(run)).toBe('#FF0000'); - }); - - it('returns underline color with style present', () => { - const run: TextRun = { - text: 'hello', - fontFamily: 'Arial', - fontSize: 12, - underline: { style: 'double', color: '#0000FF' }, - }; - expect(getRunUnderlineColor(run)).toBe('#0000FF'); - }); - - it('returns empty string when underline has no color', () => { - const run: TextRun = { - text: 'hello', - fontFamily: 'Arial', - fontSize: 12, - underline: { style: 'single' }, - }; - expect(getRunUnderlineColor(run)).toBe(''); - }); - - it('returns empty string when underline is not present', () => { - const run: TextRun = { text: 'hello', fontFamily: 'Arial', fontSize: 12 }; - expect(getRunUnderlineColor(run)).toBe(''); - }); - - it('returns empty string for non-text runs without underline', () => { - const run: Run = { kind: 'lineBreak' }; - expect(getRunUnderlineColor(run)).toBe(''); - }); - }); - - describe('underline hashing for table runs (regression test)', () => { - it('correctly extracts underline properties for cache invalidation', () => { - // This test verifies that underline style and color can be correctly extracted - // for table cell run hashing. Previously, getRunStringProp(run, 'underline') - // was used which always returned '' because underline is an object, not a string. - const runWithUnderline: TextRun = { - text: 'table cell text', - fontFamily: 'Arial', - fontSize: 12, - underline: { style: 'dashed', color: '#00FF00' }, - }; - - const runWithoutUnderline: TextRun = { - text: 'table cell text', - fontFamily: 'Arial', - fontSize: 12, - }; - - // Both style and color should be extracted for the run with underline - expect(getRunUnderlineStyle(runWithUnderline)).toBe('dashed'); - expect(getRunUnderlineColor(runWithUnderline)).toBe('#00FF00'); - - // Empty strings for the run without underline - expect(getRunUnderlineStyle(runWithoutUnderline)).toBe(''); - expect(getRunUnderlineColor(runWithoutUnderline)).toBe(''); - - // Verify that changing underline properties produces different hash inputs - const runWithDifferentStyle: TextRun = { - text: 'table cell text', - fontFamily: 'Arial', - fontSize: 12, - underline: { style: 'wavy', color: '#00FF00' }, - }; - - const runWithDifferentColor: TextRun = { - text: 'table cell text', - fontFamily: 'Arial', - fontSize: 12, - underline: { style: 'dashed', color: '#FF0000' }, - }; - - // Different style should produce different hash input - expect(getRunUnderlineStyle(runWithDifferentStyle)).not.toBe(getRunUnderlineStyle(runWithUnderline)); - - // Different color should produce different hash input - expect(getRunUnderlineColor(runWithDifferentColor)).not.toBe(getRunUnderlineColor(runWithUnderline)); - }); - }); -}); +import { hashParagraphBorders } from './paragraph-hash-utils.js'; +import type { ParagraphBorders } from '@superdoc/contracts'; describe('hashParagraphBorders', () => { it('includes between border in hash', () => { diff --git a/packages/layout-engine/painters/dom/src/paragraph-hash-utils.ts b/packages/layout-engine/painters/dom/src/paragraph-hash-utils.ts index 55870ed7e9..2e25729557 100644 --- a/packages/layout-engine/painters/dom/src/paragraph-hash-utils.ts +++ b/packages/layout-engine/painters/dom/src/paragraph-hash-utils.ts @@ -1,12 +1,4 @@ -import type { - Run, - ParagraphBorders, - ParagraphBorder, - TableBorders, - TableBorderValue, - CellBorders, - BorderSpec, -} from '@superdoc/contracts'; +import type { ParagraphBorders, ParagraphBorder } from '@superdoc/contracts'; /** * Hash helpers are duplicated from layout-bridge to avoid a circular dependency @@ -32,158 +24,3 @@ export const hashParagraphBorders = (borders: ParagraphBorders): string => { if (borders.between) parts.push(`bw:[${hashParagraphBorder(borders.between)}]`); return parts.join(';'); }; - -const isNoneBorder = (value: TableBorderValue): value is { none: true } => { - return typeof value === 'object' && value !== null && 'none' in value && (value as { none: true }).none === true; -}; - -const isBorderSpec = (value: unknown): value is BorderSpec => { - return typeof value === 'object' && value !== null && !('none' in value); -}; - -export const hashBorderSpec = (border: BorderSpec): string => { - const parts: string[] = []; - if (border.style !== undefined) parts.push(`s:${border.style}`); - if (border.width !== undefined) parts.push(`w:${border.width}`); - if (border.color !== undefined) parts.push(`c:${border.color}`); - if (border.space !== undefined) parts.push(`sp:${border.space}`); - return parts.join(','); -}; - -export const hashTableBorderValue = (borderValue: TableBorderValue | undefined): string => { - if (borderValue === undefined) return ''; - if (borderValue === null) return 'null'; - if (isNoneBorder(borderValue)) return 'none'; - if (isBorderSpec(borderValue)) { - return hashBorderSpec(borderValue); - } - return ''; -}; - -export const hashTableBorders = (borders: TableBorders | undefined): string => { - if (!borders) return ''; - const parts: string[] = []; - if (borders.top !== undefined) parts.push(`t:[${hashTableBorderValue(borders.top)}]`); - if (borders.right !== undefined) parts.push(`r:[${hashTableBorderValue(borders.right)}]`); - if (borders.bottom !== undefined) parts.push(`b:[${hashTableBorderValue(borders.bottom)}]`); - if (borders.left !== undefined) parts.push(`l:[${hashTableBorderValue(borders.left)}]`); - if (borders.insideH !== undefined) parts.push(`ih:[${hashTableBorderValue(borders.insideH)}]`); - if (borders.insideV !== undefined) parts.push(`iv:[${hashTableBorderValue(borders.insideV)}]`); - return parts.join(';'); -}; - -export const hashCellBorders = (borders: CellBorders | undefined): string => { - if (!borders) return ''; - const parts: string[] = []; - if (borders.top) parts.push(`t:[${hashBorderSpec(borders.top)}]`); - if (borders.right) parts.push(`r:[${hashBorderSpec(borders.right)}]`); - if (borders.bottom) parts.push(`b:[${hashBorderSpec(borders.bottom)}]`); - if (borders.left) parts.push(`l:[${hashBorderSpec(borders.left)}]`); - return parts.join(';'); -}; - -/** - * Type guard to check if a run has a string property. - * - * @param run - The run to check - * @param prop - The property name to check - * @returns True if the run has the property and it's a string - */ -export const hasStringProp = (run: Run, prop: string): run is Run & Record => { - return prop in run && typeof (run as Record)[prop] === 'string'; -}; - -/** - * Type guard to check if a run has a number property. - * - * @param run - The run to check - * @param prop - The property name to check - * @returns True if the run has the property and it's a number - */ -export const hasNumberProp = (run: Run, prop: string): run is Run & Record => { - return prop in run && typeof (run as Record)[prop] === 'number'; -}; - -/** - * Type guard to check if a run has a boolean property. - * - * @param run - The run to check - * @param prop - The property name to check - * @returns True if the run has the property and it's a boolean - */ -export const hasBooleanProp = (run: Run, prop: string): run is Run & Record => { - return prop in run && typeof (run as Record)[prop] === 'boolean'; -}; - -/** - * Safely gets a string property from a run, with type narrowing. - * - * @param run - The run to get the property from - * @param prop - The property name - * @returns The string value or empty string if not present - */ -export const getRunStringProp = (run: Run, prop: string): string => { - if (hasStringProp(run, prop)) { - return run[prop]; - } - return ''; -}; - -/** - * Safely gets a number property from a run, with type narrowing. - * - * @param run - The run to get the property from - * @param prop - The property name - * @returns The number value or 0 if not present - */ -export const getRunNumberProp = (run: Run, prop: string): number => { - if (hasNumberProp(run, prop)) { - return run[prop]; - } - return 0; -}; - -/** - * Safely gets a boolean property from a run, with type narrowing. - * - * @param run - The run to get the property from - * @param prop - The property name - * @returns The boolean value or false if not present - */ -export const getRunBooleanProp = (run: Run, prop: string): boolean => { - if (hasBooleanProp(run, prop)) { - return run[prop]; - } - return false; -}; - -/** - * Safely gets the underline style from a run. - * Handles the object-shaped underline property { style?, color? }. - * - * @param run - The run to get the underline style from - * @returns The underline style or empty string if not present - */ -export const getRunUnderlineStyle = (run: Run): string => { - if ('underline' in run && typeof run.underline === 'boolean') { - return run.underline ? 'single' : ''; - } - if ('underline' in run && run.underline && typeof run.underline === 'object') { - return (run.underline as { style?: string }).style ?? ''; - } - return ''; -}; - -/** - * Safely gets the underline color from a run. - * Handles the object-shaped underline property { style?, color? }. - * - * @param run - The run to get the underline color from - * @returns The underline color or empty string if not present - */ -export const getRunUnderlineColor = (run: Run): string => { - if ('underline' in run && run.underline && typeof run.underline === 'object') { - return (run.underline as { color?: string }).color ?? ''; - } - return ''; -}; diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index e9ea32ac6b..b21f42d2f8 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -71,16 +71,6 @@ import { getPresetShapeSvg } from '@superdoc/preset-geometry'; import { encodeTooltip, sanitizeHref } from '@superdoc/url-validation'; import { DOM_CLASS_NAMES } from './constants.js'; import { createChartElement as renderChartToElement } from './chart-renderer.js'; -import { - getRunBooleanProp, - getRunNumberProp, - getRunStringProp, - getRunUnderlineColor, - getRunUnderlineStyle, - hashCellBorders, - hashParagraphBorders, - hashTableBorders, -} from './paragraph-hash-utils.js'; import { assertFragmentPmPositions, assertPmPositions } from './pm-position-validation.js'; import { createRulerElement, ensureRulerStyles, generateRulerDefinitionFromPx } from './ruler/index.js'; import {