Customizable heading styles

[K-Kumar-01]

Rework of #116.
Cherry picked the commits @DemoMacro

Summary

This PR introduces customizable heading styles (H1-H6) for DOCX documents, allowing users to configure font family, font size, bold formatting, paragraph spacing, and outline levels. This is a rework of PR #116 with cherry-picked commits.

Changes:

• 6 files changed
• +183 additions / -119 deletions

Overview

This feature adds significant flexibility to heading customization by:

1. Adding a new heading configuration object to document options
2. Implementing XML generation for dynamic heading styles
3. Adding security measures (XML escaping)
4. Refactoring repetitive code into reusable functions
5. Providing comprehensive TypeScript definitions

Detailed Review

✅ Strengths

1. Security Improvements (src/utils/xml-escape.js)

• Excellent addition of XML escaping utility to prevent injection attacks
• Properly escapes all dangerous XML characters: &, <, >, ", '
• Used correctly in font name attributes at src/schemas/styles.js:10-11

2. Code Quality Refactoring (src/schemas/styles.js)

• Replaced 120+ lines of repetitive XML with ~60 lines of reusable code
• Dynamic heading generation using Object.entries() + map() approach
• Proper sorting to maintain consistent heading order
• Much more maintainable than the previous hardcoded approach

3. Type Safety (index.d.ts)

• Comprehensive TypeScript definitions for all heading configuration options
• Well-structured interfaces: HeadingConfig, HeadingStyle, HeadingSpacing
• Proper optional typing with ? modifiers

4. Validation & Safety (src/schemas/styles.js:32)

• Outline level validation: Math.max(0, Math.min(5, heading.outlineLevel || 0))
• Font size validation: checks for > 0 to prevent invalid values
• Proper undefined checks for optional properties

5. Documentation (README.md)

• Clear documentation of all new configuration options
• Includes data types and default values
• Good integration with existing documentation structure

⚠️ Issues & Concerns

1. Critical: Undefined Spacing Before (src/schemas/styles.js:29)

spacingXml = `<w:spacing w:before="${heading.spacing.before}" ${spacingAfterXml} />`;

Problem: If heading.spacing exists but heading.spacing.before is undefined, this will output w:before="undefined" in the XML, which is invalid.

Fix Needed:

if (heading.spacing) {
  const spacingBeforeXml = heading.spacing.before !== undefined
    ? `w:before="${heading.spacing.before}"`
    : '';
  spacingAfterXml = heading.spacing.after !== undefined
    ? `w:after="${heading.spacing.after}"`
    : '';
  spacingXml = spacingBeforeXml || spacingAfterXml
    ? `<w:spacing ${spacingBeforeXml} ${spacingAfterXml} />`
    : '';
}

2. Potential Issue: String Comparison (src/schemas/styles.js:35)

const additionalPropsXml = headingNumber >= 3 ? '<w:semiHidden /><w:unhideWhenUsed />' : '';

Problem: headingNumber is extracted as a string from 'Heading1', so headingNumber >= 3 is a string comparison, which works but is not semantically correct.

Suggestion:

const headingNumber = parseInt(headingId.replace('Heading', ''), 10);

3. Empty XML Elements (src/schemas/styles.js:47-52)

When optional properties are not set, empty paragraph properties (<w:pPr>) or run properties (<w:rPr>) sections are still generated. While this doesn't break DOCX files, it adds unnecessary bloat.

Suggestion: Conditionally render these sections only if they contain content.

4. Missing Font Validation

The escapeXml function is used for fonts, but there's no validation that the font string is reasonable (e.g., not excessively long, no control characters).

Suggestion: Add basic validation before escaping:

const fontXml = heading.font && heading.font !== defaultFont
  ? `<w:rFonts w:ascii="${escapeXml(heading.font.trim().substring(0, 100))}" ...`

5. Inconsistent Merging Strategy (src/schemas/styles.js:68)

const config = { ...defaultHeadingOptions, ...headingConfig };

This shallow merge means if a user provides { heading1: { fontSize: 50 } }, it will completely override defaultHeadingOptions.heading1 rather than merging properties. Users lose default values for bold, spacing, keepLines, etc.

Critical Fix Needed:

const config = Object.fromEntries(
  Object.entries(defaultHeadingOptions).map(([key, defaultValue]) => [
    key,
    headingConfig?.[key]
      ? { ...defaultValue, ...headingConfig[key] }
      : defaultValue
  ])
);

📋 Minor Observations

1. Commit History: Clean commit messages with good descriptions
2. Attribution: Proper co-authorship maintained from original PR
3. Constants: Good practice extracting defaultHeadingOptions to constants file
4. Consistency: Follows existing code style and patterns

🧪 Testing Recommendations

The PR should include tests for:

1. ✅ Default heading styles work without configuration
2. ✅ Custom heading styles override defaults correctly
3. ✅ Partial configuration merges with defaults (currently broken - see Issue Margin and Padding Support #5)
4. ✅ XML injection attempts are escaped properly
5. ✅ Edge cases: undefined, null, empty strings, negative numbers
6. ✅ Outline level clamping (values < 0 or > 5)
7. ✅ Font size validation (zero, negative, undefined)

🔒 Security Assessment

Good:

• XML escaping implemented for user-provided font names
• Outline level validation prevents out-of-bounds values
• Font size validation prevents negative/zero values

Consider:

• Input length limits for font names
• Validation of numeric inputs (spacing, fontSize) for reasonable ranges

Recommendations

Must Fix (Blocking)

1. Fix undefined spacing.before issue - This will cause invalid XML
2. Fix shallow merge issue - This breaks the expected API behavior

Should Fix (High Priority)

3. Convert headingNumber string to integer for proper numeric comparison
4. Add deep merge for heading configurations

Nice to Have

5. Add length validation for font names
6. Optimize empty XML element generation
7. Add unit tests for edge cases

Verdict

Status: ⚠️ CHANGES REQUESTED

This PR adds valuable functionality and demonstrates good code quality practices (refactoring, security, TypeScript support). However, there are two critical bugs that need to be fixed before merging:

1. The undefined spacing.before issue will produce invalid XML
2. The shallow merge will cause unexpected behavior when users provide partial configurations

Once these issues are resolved, this will be an excellent addition to the library.

Files Changed

• README.md - Documentation (+11)
• index.d.ts - TypeScript definitions (+25)
• src/constants.js - Default heading configurations (+59)
• src/docx-document.js - Integration (+8/-1)
• src/schemas/styles.js - Core implementation (+72/-118)
• src/utils/xml-escape.js - Security utility (+8)

---

Reviewed by: Claude Code
Review Date: 2025-10-06

[nicolasiscoding]

@DemoMacro thank you for your contributions they are truly appreciated! We added you to the contirbutors list and will be merging and deploying shortly.

@K-Kumar-01 I took this as an opportunity to start with the unit tests and will integrate this into a CI. I added a few more test cases

[nicolasiscoding]

Need to change expected.toBeDefined to be explicit and check the zip file