Skip to content

refactor(painter): delete dead hash helpers after blockLookup removal #2829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tupizz wants to merge 1 commit into
base: tadeu/sd-2563-11-delete-blocklookup
Choose a base branch
from
+3 −369
Open

refactor(painter): delete dead hash helpers after blockLookup removal #2829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
197 changes: 2 additions & 195 deletions packages/layout-engine/painters/dom/src/paragraph-hash-utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down
165 changes: 1 addition & 164 deletions packages/layout-engine/painters/dom/src/paragraph-hash-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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<string, string> => {
return prop in run && typeof (run as Record<string, unknown>)[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<string, number> => {
return prop in run && typeof (run as Record<string, unknown>)[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<string, boolean> => {
return prop in run && typeof (run as Record<string, unknown>)[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 '';
};
10 changes: 0 additions & 10 deletions packages/layout-engine/painters/dom/src/renderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading