Skip to content

fix(xlsx): write rich text into the shared string table in shared mode - #613

Open
ronistrom wants to merge 1 commit into
productdevbook:mainfrom
ronistrom:fix/rich-text-shared-strings
Open

ronistrom wants to merge 1 commit into
productdevbook:mainfrom
ronistrom:fix/rich-text-shared-strings

Conversation

@ronistrom

@ronistrom ronistrom commented Oct 20, 2026 •

Copy link
Copy Markdown

Closes #602

What was wrong

With stringMode: "shared", which is the default, plain text goes into xl/sharedStrings.xml. A rich text cell was always written inline, whatever the mode:

await writeXlsx({
  stringMode: "shared",
  sheets: [{
    name: "S",
    rows: [["Plain"]],
    cells: new Map([["1,0", { richText: [{ text: "Bold", font: { bold: true } }, { text: " normal" }] }]]),
  }],
})
<!-- before: sheet1.xml -->
<row r="2"><c r="A2" t="inlineStr"><is><r><rPr><b/></rPr><t>Bold</t></r><r><t xml:space="preserve"> normal</t></r></is></c></row>
<!-- before: sharedStrings.xml -->
<sst … count="1" uniqueCount="1"><si><t>Plain</t></si></sst>

<!-- after: sheet1.xml -->
<row r="2"><c r="A2" t="s"><v>1</v></c></row>
<!-- after: sharedStrings.xml -->
<sst … count="2" uniqueCount="2"><si><t>Plain</t></si><si><r><rPr><b/></rPr><t>Bold</t></r><r><t xml:space="preserve"> normal</t></r></si></sst>

The old output is valid. But tools that only read the shared string table, such as template engines, find/replace and string indexing, never saw the rich cells. It also isn't what Excel writes. An <si> can hold the same <r> runs as an inline <is> (ECMA-376 Part 1 §18.4.8, §18.4.4, §18.3.1.53).

Why the tests didn't catch it

Several tests asserted the inline form for the default mode, so they pinned the behaviour: coverage-xlsx-writer.test.ts (t="inlineStr" on a styled rich cell), rich-text-write.test.ts (the mixed plain/rich sheet), one-cell-serializer.test.ts (<is> in the sheet XML), xml-space-preserve.test.ts (looked for the rich run's <t> in the sheet XML) and batch-fixes.test.ts. The round-trip tests passed either way, because the reader already turns both <is> and <si> runs into richText.

What landed

  • SharedStringsCollector gets addRich(runs). The dedup key is the serialized <r> XML (serializeRichTextRuns), kept in a map of its own. Two cells with equal runs share one entry. A plain string with the same text, or the same text with different formatting, gets its own entry.
  • writeSharedStringsXml writes a rich entry as <si> holding those runs.
  • serializeCell writes a rich cell as t="s" with the shared index when stringMode is "shared". The style index is kept. The comment there cites Rich text cells ignore stringMode: always inline strings, even with "shared" #602.
  • Inline mode is byte-identical: the SHA-256 of an inline-mode workbook with a styled rich cell is the same before and after.
  • saveXlsx always writes shared strings, so it now puts rich text in the table too. A rich cell read from an inline-only file moves into the table on save, which matches Excel.
  • The streaming writers take plain cell values and can't carry rich text, so nothing changes there. The reader is unchanged. docs/PARITY.md doesn't say where rich text is stored, so it is unchanged too.
  • The tests that pinned the inline form now either expect the shared form or pass stringMode: "inline" (or inlineStrings: true) when the test is about the inline form's runs.

Tests: a new test/rich-text-shared-strings.test.ts with seven tests. Five of them fail against main: shared <si> plus t="s", the style index, dedup apart from plain text, the shared string part being written when rich text is the only text, and saveXlsx. The other two passed already: the read-back round trip, and the exact inline output. The four updated assertions in the existing tests also fail against main, nine failures in total. bun run test passes on Node 24.22.0, with 241 files and 10,705 tests.

Checked in Excel

In Excel for Mac, with A1 as "Bold red then normal" and A2 as plain text, written by main and by this branch. Both files open without a repair prompt and look the same: the first run is bold and red, the rest is plain. In the after file the rich text is in sharedStrings.xml as an <si> with two <r> runs, and A1 is t="s". In the before file it is an inline <is>. So Excel reads the new form as it reads the old one.

Not verified

  • LibreOffice and Windows Excel. Not opened.
  • The tools this is for. Template engines and find/replace tools that only read the shared string table were not tried.

Summary by CodeRabbit

  • New Features
    • Rich text in XLSX files now supports shared-string storage by default.
    • Rich formatting, whitespace, styles, and duplicate text runs are preserved when exporting and re-importing spreadsheets.
    • Inline string storage remains available when explicitly selected.
    • Documentation clarifies how rich text follows the chosen string storage mode.

In the default stringMode "shared", plain text went to
xl/sharedStrings.xml but a rich text cell was always written inline as
t="inlineStr" with an <is> of runs. The file was valid, but tools that
only read the shared string table never saw the rich cells, and it is
not what Excel writes.

The shared strings collector now has addRich(runs), keyed by the
serialized <r> runs in a map of its own, so equal rich strings share one
entry and a plain string with the same text stays separate.
writeSharedStringsXml writes those entries as <si> holding the same <r>
runs as the inline form, and serializeCell writes the cell as t="s" when
stringMode is "shared". This also covers saveXlsx, which always uses
shared strings. Inline mode is byte-identical. The reader already
returned richText from <si> runs.

Tests that pinned t="inlineStr" for the default mode now either expect
the shared form or ask for inline mode explicitly.

Closes productdevbook#602
@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: aad351fb-4fa3-4101-8969-8c4f76adb0c4
📥 Commits

Reviewing files that changed from the base of the PR and between c991ad0 and cc2c9ca.

📒 Files selected for processing: 8
  • src/_types.ts
  • src/xlsx/worksheet-writer.ts
  • test/batch-fixes.test.ts
  • test/coverage-xlsx-writer.test.ts
  • test/one-cell-serializer.test.ts
  • test/rich-text-shared-strings.test.ts
  • test/rich-text-write.test.ts
  • test/xml-space-preserve.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 0 remain after this review.

📝 Walkthrough

Rich-text cells now use shared strings by default and retain inline output when inline mode is selected. The shared-string collector stores and deduplicates rich runs. Tests cover serialization, styles, whitespace preservation, and workbook round trips.

Changes: Rich-text shared-string serialization

Layer / File(s) Summary
Store and serialize rich shared strings
src/xlsx/worksheet-writer.ts, test/rich-text-shared-strings.test.ts
The collector registers rich runs and deduplicates entries by serialized run markup. Shared-string XML emits rich runs for rich entries and plain text for ordinary entries. Tests cover shared entries, deduplication, and workbooks containing only rich text.
Route rich-text cells by string mode
src/_types.ts, src/xlsx/worksheet-writer.ts, test/*rich-text*.test.ts, test/batch-fixes.test.ts, test/coverage-xlsx-writer.test.ts, test/one-cell-serializer.test.ts, test/xml-space-preserve.test.ts
Rich-text cells use shared strings unless inline mode is selected. Documentation describes the selected mode. Tests cover styles, inline output, whitespace preservation, and workbook round trips.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: productdevbook

Merge Risk: ⚪ Minimal · up to cc2c9

Rich-text storage follows the selected string mode, with no identified issue requiring a fix before merge.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 23.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 8 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: storing rich text in the shared string table for shared string mode.
Linked Issues check Passed Issue #602 requires rich text to follow stringMode, use shared <si> runs with t="s" in shared mode, retain inline output in inline mode, and deduplicate equal rich runs separately from plain str…
Out of Scope Changes check Passed The changes stay within issue #602. Source changes update shared-string collection, XML writing, worksheet serialization, and the related option documentation. Test changes update expectations and add…
Full details: Linked Issues check

Explanation

Issue #602 requires rich text to follow stringMode, use shared &lt;si&gt; runs with t="s" in shared mode, retain inline output in inline mode, and deduplicate equal rich runs separately from plain strings. SharedStringsCollector.addRich keys serialized runs independently, writeSharedStringsXml emits rich &lt;si&gt; entries, and serializeCell preserves the style index while using the shared index when inline strings are disabled. The added tests cover shared storage, deduplication, styling, inline output, read-back, and saveXlsx. The issue does not require streaming changes because those writers accept only plain values.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #602. Source changes update shared-string collection, XML writing, worksheet serialization, and the related option documentation. Test changes update expectations and add coverage for the requested shared and inline behavior. The saveXlsx coverage supports the same serialization requirement. No unrelated product behavior is demonstrated.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rich text cells ignore stringMode: always inline strings, even with "shared"

1 participant