diff --git a/README.md b/README.md index 5f75895..ec2fe99 100644 --- a/README.md +++ b/README.md @@ -186,6 +186,28 @@ const buffer = await writeXlsx({ }) ``` +A row entry may be a value or a cell object, so styling one cell does not +mean naming its position again in a parallel map: + +```ts +await writeXlsx({ + sheets: [ + { + name: "Report", + rows: [ + [{ value: "Region", style: { font: { bold: true } } }, "Revenue"], + ["EU", { value: 12500, style: { numFmt: "$#,##0.00" } }], + ["Total", { formula: "SUM(B2:B2)" }], + ], + }, + ], +}) +``` + +Anything a cell carries works there — `style`, `formula`, `richText`, +`hyperlink`, `checkbox`. `cells` still takes a `"row,col"` map, and wins +where both describe the same position. + Features: cell styles, auto column widths, merged cells, freeze/split panes, auto-filter (with per-column value filters — ``; custom/dynamic/colour criteria are not emitted), data validation, hyperlinks, images (PNG/JPEG/GIF/SVG/WebP), comments, tables, conditional formatting (all 15 rule types, with their dxf styles), named ranges, print settings, page breaks, sheet protection, workbook protection, rich text, shared/array/dynamic formulas, sparklines, textboxes, background images, number formats, hidden sheets, Excel 2024 native checkboxes, HTML/Markdown/JSON/TSV export, template engine. ### Auto Column Width diff --git a/src/_inline-cells.ts b/src/_inline-cells.ts new file mode 100644 index 0000000..241848d --- /dev/null +++ b/src/_inline-cells.ts @@ -0,0 +1,146 @@ +// ── Cell objects written inline in `rows` ─────────────────────────── +// +// `WriteSheet.rows` is the grid and `WriteSheet.cells` the per-cell +// detail, keyed `"row,col"`. Styling one cell therefore meant naming its +// position twice — once in the row, once in the map — and keeping the +// two in step by hand. +// +// The streaming writer never had that split: `addRow` has taken +// `{ value, style, formula }` inline since it existed. `writeOdsStream` +// takes the shape too, though only the `value` and `formula` of it — +// per-cell styles are the buffered ODS writer's alone. +// +// So the two XLSX writers disagreed about what a row entry may be, and +// the buffered one did not refuse the shape it did not accept — +// `resolveRows` read a cell object as a value, and the cell came out +// **empty**. Value, style and formula all gone, no error. See #433. +// +// Rather than teach every consumer of `rows` about a second shape — the +// two writers, the auto-width measurer, the pivot source collector, the +// table extent — an inline cell is lifted into `cells` once, before any +// of them runs. `rows` stays a grid of values, `cells` stays the one +// place per-cell detail lives, and an explicit `cells` entry still wins +// over an inline one at the same position. + +import type { Cell, CellValue, WriteSheet } from "./_types" +import { isHyperlinkValue } from "./xlsx/hyperlink" + +/** + * A cell written where a value goes: `{ value, style }`, `{ formula }`, + * or any other part of a {@link Cell}. + */ +export type InlineCell = Partial + +/** + * Whether a row entry is a cell object rather than a value. + * + * `Date` is the only object a `CellValue` can be, and a + * `HyperlinkValue` — `{ text, hyperlink }`, both strings — is the object + * the `data[]` path already accepts in a value position. Everything else + * is a cell object: not because the shape was inspected, but because + * nothing else was ever a legal entry, so the alternative to reading it + * as one is dropping it. + */ +export function isInlineCell(v: unknown): v is InlineCell { + return ( + typeof v === "object" && + v !== null && + !(v instanceof Date) && + !Array.isArray(v) && + !isHyperlinkValue(v) + ) +} + +/** + * The value of a row entry, whichever shape it arrived in. + * + * A total function rather than a cast: a consumer that calls it is + * correct on a sheet that went through {@link splitInlineCells} and on + * one that did not, so the compiler is being told something true rather + * than being overruled. + */ +export function toCellValue(v: CellValue | InlineCell): CellValue { + return isInlineCell(v) ? (v.value ?? null) : v +} + +/** + * {@link toCellValue} over a grid, without copying one that is already + * all values — which is every grid a caller wrote before #433, and most + * of them since. The scan is one `typeof` per cell; a 100,000 × 12 sheet + * that a CSV or JSON writer is about to walk anyway is not worth + * duplicating to satisfy a type. + */ +export function toCellValues(rows: Array>): CellValue[][] { + for (const row of rows) { + for (const v of row) { + if (isInlineCell(v)) return rows.map((r) => r.map(toCellValue)) + } + } + return rows as CellValue[][] +} + +/** + * Lift any inline cell objects out of `sheet.rows` into `sheet.cells`. + * + * Returns the sheet **unchanged** when there are none, which is the + * usual case — the scan is one `typeof` per cell and allocates nothing + * until it finds something. A sheet that does carry them is copied + * shallowly; the caller's arrays and map are never mutated. + */ +export function splitInlineCells(sheet: WriteSheet): WriteSheet { + const rows = sheet.rows + if (!rows) return sheet + + let found = false + for (const row of rows) { + for (const v of row) { + if (isInlineCell(v)) { + found = true + break + } + } + if (found) break + } + if (!found) return sheet + + const plainRows: CellValue[][] = [] + const lifted = new Map>() + + for (let r = 0; r < rows.length; r++) { + const row = rows[r]! + const plain: CellValue[] = new Array(row.length) + for (let c = 0; c < row.length; c++) { + const v = row[c] + if (isInlineCell(v)) { + lifted.set(`${r},${c}`, v) + // The value stays in the grid too, so everything that reads only + // `rows` — auto-width, a pivot's source range, a table's extent — + // sees the cell rather than a hole. + plain[c] = v.value ?? null + } else { + plain[c] = v as CellValue + } + } + plainRows[r] = plain + } + + // The caller's own `cells` is applied second, so where both describe a + // position the explicit map wins — the same precedence `cells` already + // has over `rows`. + if (sheet.cells) { + for (const [key, cell] of sheet.cells) lifted.set(key, cell) + } + + return { ...sheet, rows: plainRows, cells: lifted } +} + +/** {@link splitInlineCells} over a workbook's sheets. */ +export function splitInlineCellsInSheets(sheets: WriteSheet[]): WriteSheet[] { + let changed = false + const out = sheets.map((s) => { + const next = splitInlineCells(s) + if (next !== s) changed = true + return next + }) + return changed ? out : sheets +} diff --git a/src/_types.ts b/src/_types.ts index e1d4132..4b6405a 100644 --- a/src/_types.ts +++ b/src/_types.ts @@ -1685,8 +1685,17 @@ export interface WriteOptions { export interface WriteSheet { name: string columns?: ColumnDef[] - /** Raw row data (array of arrays) */ - rows?: CellValue[][] + /** + * Raw row data (array of arrays). + * + * An entry is a {@link CellValue}, or a cell object — `{ value, style }`, + * `{ formula }`, anything a {@link Cell} carries — written where the + * value goes. The streaming writers have taken that shape since they + * existed; the buffered ones now do too, so styling one cell no longer + * means naming its position again in {@link cells} (#433). Where both + * describe a position, {@link cells} wins. + */ + rows?: Array>> /** * Object data (array of objects — uses column keys). A value may be a scalar * {@link CellValue} or a rich {@link HyperlinkValue} for inline clickable links. diff --git a/src/defter.ts b/src/defter.ts index 0f1fbbd..883199d 100644 --- a/src/defter.ts +++ b/src/defter.ts @@ -37,6 +37,7 @@ import { writeXml } from "./xml/data-writer" import { fromHtml } from "./export/html-import" import { toHtml } from "./export/html" import { toMarkdown } from "./export/markdown" +import { toCellValues } from "./_inline-cells" // ── Format Detection ──────────────────────────────────────────────── @@ -217,7 +218,10 @@ export async function write( if (!sheet) { throw new UnsupportedFormatError(`${format} needs a sheet to write, and the workbook has none.`) } - const rows = sheet.rows ?? [] + // These formats carry values and nothing else, so an inline cell object + // reduces to its value here rather than going through the `cells` split + // the two spreadsheet writers do. See #433. + const rows = toCellValues(sheet.rows ?? []) const encoder = new TextEncoder() switch (format) { diff --git a/src/ods/writer.ts b/src/ods/writer.ts index 6c2d86c..7f64fce 100644 --- a/src/ods/writer.ts +++ b/src/ods/writer.ts @@ -19,6 +19,7 @@ import { validateSheetNames } from "../_validate" import { unwrapCellValue } from "../xlsx/hyperlink" import { xmlDocument, xmlElement, xmlSelfClose, xmlEscape as escapeXmlText } from "../xml/writer" import { replaceA1Ranges, toRanges } from "../cell-utils" +import { splitInlineCellsInSheets, toCellValues } from "../_inline-cells" const encoder = /* @__PURE__ */ new TextEncoder() @@ -1230,7 +1231,7 @@ function writeContentXml(options: WriteOptions): string { // Resolve rows from rows or data let rows: CellValue[][] = [] if (sheet.rows) { - rows = sheet.rows + rows = toCellValues(sheet.rows) } else if (sheet.data && sheet.columns) { // Generate header row + data rows from objects const keys = sheet.columns.map((c) => c.key ?? c.header ?? "") @@ -1518,6 +1519,11 @@ export function writeManifestXml(): string { * Returns a Uint8Array containing the ZIP archive. */ export async function writeOds(options: WriteOptions): Promise { + // A cell object written inline in `rows` becomes a `cells` entry before + // anything reads the grid — the same normalisation `writeXlsx` does, in + // one implementation. See #433 and `src/_inline-cells.ts`. + options = { ...options, sheets: splitInlineCellsInSheets(options.sheets) } + // Same rules as XLSX: LibreOffice enforces Excel's sheet-name limits // for interoperability. See #364. validateSheetNames(options.sheets) diff --git a/src/xlsx/worksheet-writer.ts b/src/xlsx/worksheet-writer.ts index 207c232..059734a 100644 --- a/src/xlsx/worksheet-writer.ts +++ b/src/xlsx/worksheet-writer.ts @@ -31,6 +31,7 @@ import { calculateColumnWidth } from "./auto-width" import { DYNAMIC_ARRAY_CM } from "./metadata" import { hashSheetPassword } from "./password" import { validateColumnIndex } from "../_validate" +import { toCellValue } from "../_inline-cells" // ── Hyperlink Relationship ──────────────────────────────────────── @@ -829,7 +830,11 @@ function resolveRows(sheet: WriteSheet): Array> { for (const row of sheet.rows) { const resolvedRow: Array = [] for (let c = 0; c < row.length; c++) { - const value = row[c] + // `writeXlsx` lifts an inline cell object into `cells` before this + // runs, so the entry is a value by then. Reading it through + // `toCellValue` keeps `resolveRows` correct for a caller that + // reached it another way, rather than emitting `[object Object]`. + const value = toCellValue(row[c]!) const style = sheet.columns ? columnCellStyle(sheet.columns[c]) : undefined resolvedRow.push(style ? { value, style } : { value }) } diff --git a/src/xlsx/writer.ts b/src/xlsx/writer.ts index a72f7d7..4a8aeaa 100644 --- a/src/xlsx/writer.ts +++ b/src/xlsx/writer.ts @@ -9,6 +9,7 @@ import type { WriteSheet, } from "../_types" import { ZipWriter } from "../zip/writer" +import { splitInlineCellsInSheets, toCellValue } from "../_inline-cells" import { writeContentTypes } from "./content-types-writer" import { FPB_PART_PATH, writeFeaturePropertyBagXml } from "./feature-property-bag" import { METADATA_PART_PATH, writeMetadataXml } from "./metadata" @@ -72,7 +73,11 @@ function effectiveProperties(options: WriteOptions): WorkbookProperties | undefi * Returns a Uint8Array containing the ZIP archive. */ export async function writeXlsx(options: WriteOptions): Promise { - const { sheets, defaultFont, dateSystem, namedRanges, activeSheet, workbookProtection } = options + // A cell object written inline in `rows` becomes a `cells` entry before + // anything reads the grid, so every consumer below still sees values. + // See #433 and `src/_inline-cells.ts`. + const sheets = splitInlineCellsInSheets(options.sheets) + const { defaultFont, dateSystem, namedRanges, activeSheet, workbookProtection } = options // Before any bytes are produced, so a rejected workbook leaves no // half-written output. See #364. @@ -595,7 +600,9 @@ export async function writeXlsx(options: WriteOptions): Promise { */ function collectSourceRows(sheet: WriteSheet): CellValue[][] { if (sheet.rows && sheet.rows.length > 0) { - return sheet.rows.map((row) => [...row]) + // A pivot sources values; `writeXlsx` has already lifted any inline + // cell objects, and `toCellValue` keeps this correct on its own. + return sheet.rows.map((row) => row.map(toCellValue)) } if (sheet.data && sheet.data.length > 0 && sheet.columns && sheet.columns.length > 0) { const out: CellValue[][] = [] diff --git a/test/coverage-roundtrip-pivot.test.ts b/test/coverage-roundtrip-pivot.test.ts index 5cb2454..03e5cd6 100644 --- a/test/coverage-roundtrip-pivot.test.ts +++ b/test/coverage-roundtrip-pivot.test.ts @@ -6,7 +6,7 @@ import { cloneChart } from "../src/xlsx/chart-clone" import { ZipReader } from "../src/zip/reader" import { ZipWriter } from "../src/zip/writer" import { EncryptedFileError } from "../src/errors" -import type { Chart, SheetChart, WritePivotTable, WriteSheet } from "../src/_types" +import type { CellValue, Chart, SheetChart, WritePivotTable, WriteSheet } from "../src/_types" const encoder = new TextEncoder() const decoder = new TextDecoder("utf-8") @@ -39,16 +39,18 @@ async function withParts( return out.build() } -const SALES: WriteSheet = { - name: "Data", - rows: [ - ["Region", "Product", "Quarter", "Revenue"], - ["EU", "Widget", "Q1", 100], - ["US", "Widget", "Q1", 200], - ["EU", "Gadget", "Q2", 50], - ["US", "Gadget", "Q2", 75], - ], -} +// Declared as its own grid rather than read back off `SALES` — a +// `WriteSheet` row may now hold a cell object as well as a value (#433), +// and `resolvePivotSource` takes values. +const SALES_ROWS: CellValue[][] = [ + ["Region", "Product", "Quarter", "Revenue"], + ["EU", "Widget", "Q1", 100], + ["US", "Widget", "Q1", 200], + ["EU", "Gadget", "Q2", 50], + ["US", "Gadget", "Q2", 75], +] + +const SALES: WriteSheet = { name: "Data", rows: SALES_ROWS } // ═══════════════════════════════════════════════════════════════════════ // pivot-writer — axis placement @@ -171,7 +173,7 @@ describe("pivot data fields", () => { rows: ["Region"], values: [{ field: "Revenue", function: fn }], } - const source = resolvePivotSource(pivot, "Data", SALES.rows!) + const source = resolvePivotSource(pivot, "Data", SALES_ROWS) const { pivotTableXml } = writePivotTable(pivot, source, 0) expect(pivotTableXml).toContain(`name="${label}"`) expect(pivotTableXml).toContain(`subtotal="${fn}"`) @@ -228,7 +230,7 @@ describe("pivot cache fields", () => { columns: ["Region"], values: [{ field: "Revenue" }], } - const source = resolvePivotSource(pivot, "Data", SALES.rows!) + const source = resolvePivotSource(pivot, "Data", SALES_ROWS) const { pivotTableXml } = writePivotTable(pivot, source, 0) expect(pivotTableXml).toContain(" { rows: ["Region"], values: [{ field: "Revenue" }], } - const source = resolvePivotSource(pivot, "Data", SALES.rows!) + const source = resolvePivotSource(pivot, "Data", SALES_ROWS) expect(() => writePivotTable(pivot, source, 0)).toThrow(/A1-style reference/) }) diff --git a/test/inline-cells.test.ts b/test/inline-cells.test.ts new file mode 100644 index 0000000..f0666c0 --- /dev/null +++ b/test/inline-cells.test.ts @@ -0,0 +1,166 @@ +// A cell object written where a value goes — `rows: [[{ value, style }]]`. +// +// Before #433 the buffered writers read it as a value and emitted an +// *empty* cell: the style, the formula and the value all gone, with no +// error. `XlsxStreamWriter.addRow` had accepted the shape since it +// existed, so the two halves of the library disagreed about what a row +// may hold. + +import { describe, expect, it } from "vitest" +import { writeXlsx } from "../src/xlsx/writer" +import { openXlsx, saveXlsx } from "../src/xlsx/roundtrip" +import { writeOds } from "../src/ods/writer" +import { readOds } from "../src/ods/reader" +import { readXlsx } from "../src/xlsx/reader" +import { ZipReader } from "../src/zip/reader" +import { write } from "../src/defter" +import type { WriteSheet } from "../src/_types" + +async function part(buf: Uint8Array, path: string): Promise { + const zip = new ZipReader(buf) + return new TextDecoder().decode(await zip.extract(path)) +} + +const inlineSheet: WriteSheet = { + name: "S", + rows: [ + ["plain", { value: "wrapped", style: { alignment: { wrapText: true } } }], + [{ value: 1234.5, style: { numFmt: "#,##0.00" } }, { formula: "A2*2" }], + ], +} + +/** The same sheet said the old way, for the writers to agree with. */ +const mapSheet: WriteSheet = { + name: "S", + rows: [ + ["plain", "wrapped"], + [1234.5, null], + ], + cells: new Map([ + ["0,1", { value: "wrapped", style: { alignment: { wrapText: true } } }], + ["1,0", { value: 1234.5, style: { numFmt: "#,##0.00" } }], + ["1,1", { formula: "A2*2" }], + ]), +} + +describe("cell objects written inline in rows", () => { + it("keeps the value that used to be dropped (xlsx)", async () => { + const buf = await writeXlsx({ sheets: [inlineSheet] }) + const wb = await readXlsx(buf, { readStyles: true }) + const rows = wb.sheets[0]!.rows + + expect(rows[0]![1]).toBe("wrapped") + expect(rows[1]![0]).toBe(1234.5) + }) + + it("keeps the value that used to be dropped (ods)", async () => { + const buf = await writeOds({ sheets: [inlineSheet] }) + const wb = await readOds(buf) + + expect(wb.sheets[0]!.rows[0]![1]).toBe("wrapped") + expect(wb.sheets[0]!.rows[1]![0]).toBe(1234.5) + }) + + it("carries the style, not just the value", async () => { + const buf = await writeXlsx({ sheets: [inlineSheet] }) + const styles = await part(buf, "xl/styles.xml") + const sheet = await part(buf, "xl/worksheets/sheet1.xml") + + // A wrap alignment and a number format both had to be registered. + expect(styles).toContain('applyAlignment="true"') + expect(styles).toContain('wrapText="true"') + expect(styles).toContain("#,##0.00") + // …and reach the cells that asked for them. + expect(sheet).toMatch(/]*s="[1-9]/) + expect(sheet).toMatch(/]*s="[1-9]/) + }) + + it("carries a formula", async () => { + const buf = await writeXlsx({ sheets: [inlineSheet] }) + expect(await part(buf, "xl/worksheets/sheet1.xml")).toContain("A2*2") + }) + + it("is the same document as the cells map spelling", async () => { + const inline = await part( + await writeXlsx({ sheets: [inlineSheet] }), + "xl/worksheets/sheet1.xml", + ) + const viaMap = await part(await writeXlsx({ sheets: [mapSheet] }), "xl/worksheets/sheet1.xml") + expect(inline).toBe(viaMap) + + const odsInline = await part(await writeOds({ sheets: [inlineSheet] }), "content.xml") + const odsMap = await part(await writeOds({ sheets: [mapSheet] }), "content.xml") + expect(odsInline).toBe(odsMap) + }) + + it("lets an explicit cells entry win over the inline one", async () => { + const buf = await writeXlsx({ + sheets: [ + { + name: "S", + rows: [[{ value: "inline", style: { font: { bold: true } } }]], + cells: new Map([["0,0", { value: "explicit" }]]), + }, + ], + }) + const wb = await readXlsx(buf) + expect(wb.sheets[0]!.rows[0]![0]).toBe("explicit") + }) + + it("does not mistake a Date for a cell object", async () => { + const when = new Date(Date.UTC(2020, 0, 15)) + const buf = await writeXlsx({ sheets: [{ name: "S", rows: [[when]] }] }) + const wb = await readXlsx(buf) + expect(wb.sheets[0]!.rows[0]![0]).toBeInstanceOf(Date) + }) + + it("does not mistake a hyperlink value for a cell object", async () => { + const buf = await writeXlsx({ + sheets: [ + { + name: "S", + columns: [{ key: "a", header: "A" }], + data: [{ a: { text: "hucre", hyperlink: "https://example.com" } }], + }, + ], + }) + const wb = await readXlsx(buf) + expect(wb.sheets[0]!.rows[1]![0]).toBe("hucre") + }) + + it("reaches the round-trip writer too", async () => { + const base = await writeXlsx({ sheets: [{ name: "S", rows: [["a"]] }] }) + const wb = await openXlsx(base) + wb.sheets[0]!.rows = [[{ value: "b", style: { font: { bold: true } } }]] as never + const out = await saveXlsx(wb, {}) + const back = await readXlsx(out) + expect(back.sheets[0]!.rows[0]![0]).toBe("b") + }) + + it("reduces to the value for the formats that carry only values", async () => { + const csv = await write({ sheets: [inlineSheet], format: "csv" }) + const text = new TextDecoder().decode(csv as Uint8Array) + expect(text).toContain("wrapped") + expect(text).toContain("1234.5") + expect(text).not.toContain("object Object") + }) + + it("leaves a sheet of plain values exactly as it was", async () => { + // The scan must not copy a grid it found nothing in — the same array + // instance is what proves it. + const rows = [["a", 1]] + const sheet: WriteSheet = { name: "S", rows } + await writeXlsx({ sheets: [sheet] }) + expect(sheet.rows).toBe(rows) + expect(sheet.cells).toBeUndefined() + }) + + it("does not mutate the caller's sheet when it does split", async () => { + const rows = [[{ value: "x", style: { font: { bold: true } } }]] + const sheet: WriteSheet = { name: "S", rows } + await writeXlsx({ sheets: [sheet] }) + expect(sheet.rows).toBe(rows) + expect(sheet.rows![0]![0]).toEqual({ value: "x", style: { font: { bold: true } } }) + expect(sheet.cells).toBeUndefined() + }) +})