diff --git a/docs/PARITY.md b/docs/PARITY.md index 6b21f59..fd3f7d9 100644 --- a/docs/PARITY.md +++ b/docs/PARITY.md @@ -317,6 +317,15 @@ cannot be right. `value` still holds the error token, so spotting an error by its value is unaffected, and a _hard-coded_ error cell — one with no formula — still reports `"error"`. +An error result and a text result are told apart. Both hold a string in +`formulaResult`, and a formula can return the text `"#DIV/0!"` as well as +the error, so the token alone cannot say which it is. A cached `t="e"` +result now reads with `formulaResultType: "error"`, and the writer emits +`t="e"` for it rather than `t="str"`; before #599 every error result was +written back as text. `writeXlsxStream` has no way to say a streamed +formula's result is an error, so it still writes string results as +`t="str"`. + ### ISO-8601 date cells `ST_CellType` (§18.18.11) has seven members and the reader's switch had diff --git a/src/_types.ts b/src/_types.ts index 8d9591a..69ac05b 100644 --- a/src/_types.ts +++ b/src/_types.ts @@ -217,6 +217,19 @@ export interface Cell { checkbox?: boolean formula?: string formulaResult?: CellValue + /** + * `"error"` when the cached {@link formulaResult} is an error token + * (`t="e"`) rather than text (`t="str"`). Both hold a string, and + * `#DIV/0!` is a perfectly good text result too, so the value alone + * cannot tell them apart (ECMA-376 Part 1, §18.18.11 ST_CellType). + * See #599. + * + * The reader sets it only on a formula cell whose cached result is an + * error; it is absent otherwise. The writer honours it only when + * `formulaResult` is a string — an error token is always text — and + * writes any other `formulaResult` as it would without it. + */ + formulaResultType?: "error" /** Formula type: "shared" | "array". Undefined means normal formula. */ formulaType?: "shared" | "array" /** Shared formula index (si attribute) */ diff --git a/src/sheet-ops.ts b/src/sheet-ops.ts index 7858fbf..3b722c3 100644 --- a/src/sheet-ops.ts +++ b/src/sheet-ops.ts @@ -890,6 +890,7 @@ function cloneCell(cell: Cell): Cell { if (cell.checkbox !== undefined) result.checkbox = cell.checkbox if (cell.formula !== undefined) result.formula = cell.formula if (cell.formulaResult !== undefined) result.formulaResult = cell.formulaResult + if (cell.formulaResultType !== undefined) result.formulaResultType = cell.formulaResultType // The formula's *shape*, not just its text. Dropping these turned a // shared-formula slave cell — `{ formula: "", formulaType: "shared", // formulaSharedIndex: 3 }` — into a plain `{ formula: "" }`, which the diff --git a/src/worker.ts b/src/worker.ts index 4c75dd4..f8b981f 100644 --- a/src/worker.ts +++ b/src/worker.ts @@ -27,6 +27,8 @@ export interface SerializedCell { checkbox?: Cell["checkbox"] formula?: Cell["formula"] formulaResult?: SerializedCellValue + /** Whether that result is an error token rather than text (#599). */ + formulaResultType?: Cell["formulaResultType"] /** * The formula's shape, not just its text. Without these a shared-formula * slave cell — `{ formula: "", formulaType: "shared", si }` — arrives as @@ -155,6 +157,7 @@ function serializeCell(cell: Cell): SerializedCell { if (cell.formulaResult !== undefined) { out.formulaResult = serializeCellValue(cell.formulaResult) } + if (cell.formulaResultType !== undefined) out.formulaResultType = cell.formulaResultType if (cell.formulaType !== undefined) out.formulaType = cell.formulaType if (cell.formulaSharedIndex !== undefined) out.formulaSharedIndex = cell.formulaSharedIndex if (cell.formulaRef !== undefined) out.formulaRef = cell.formulaRef @@ -346,6 +349,7 @@ function deserializeCell(sc: SerializedCell): Cell { if (sc.formulaResult !== undefined) { cell.formulaResult = deserializeCellValue(sc.formulaResult) } + if (sc.formulaResultType !== undefined) cell.formulaResultType = sc.formulaResultType if (sc.formulaType !== undefined) cell.formulaType = sc.formulaType if (sc.formulaSharedIndex !== undefined) cell.formulaSharedIndex = sc.formulaSharedIndex if (sc.formulaRef !== undefined) cell.formulaRef = sc.formulaRef diff --git a/src/xlsx/worksheet-writer.ts b/src/xlsx/worksheet-writer.ts index 381cc64..8e24cdd 100644 --- a/src/xlsx/worksheet-writer.ts +++ b/src/xlsx/worksheet-writer.ts @@ -215,6 +215,7 @@ export interface ResolvedCell { checkbox?: boolean formula?: string formulaResult?: CellValue + formulaResultType?: "error" formulaType?: "shared" | "array" formulaSharedIndex?: number formulaRef?: string @@ -851,6 +852,7 @@ function resolveRows(sheet: WriteSheet): Array> { checkbox: cellOverride.checkbox ?? existing?.checkbox, formula: cellOverride.formula ?? existing?.formula, formulaResult: cellOverride.formulaResult ?? existing?.formulaResult, + formulaResultType: cellOverride.formulaResultType ?? existing?.formulaResultType, formulaType: cellOverride.formulaType ?? existing?.formulaType, formulaSharedIndex: cellOverride.formulaSharedIndex ?? existing?.formulaSharedIndex, formulaRef: cellOverride.formulaRef ?? existing?.formulaRef, @@ -920,6 +922,7 @@ export function serializeCell( style, formula, formulaResult, + formulaResultType, formulaType, formulaSharedIndex, formulaRef, @@ -1010,7 +1013,11 @@ export function serializeCell( // and booleans need their `t`. if (formulaResult !== undefined && formulaResult !== null) { if (typeof formulaResult === "string") { - cellAttrs["t"] = "str" + // An error result is a string too, and written as `t="str"` it + // became a formula that returned that *text* (#599). Which one + // it is comes from `formulaResultType`, never from the token: a + // formula can return the text "#DIV/0!" as well. + cellAttrs["t"] = formulaResultType === "error" ? "e" : "str" children.push(xmlElement("v", undefined, xmlEscape(formulaResult))) } else if (typeof formulaResult === "boolean") { cellAttrs["t"] = "b" diff --git a/src/xlsx/worksheet.ts b/src/xlsx/worksheet.ts index c208e23..0b84a33 100644 --- a/src/xlsx/worksheet.ts +++ b/src/xlsx/worksheet.ts @@ -1853,6 +1853,7 @@ function processCell( let cellType: Cell["type"] = "empty" let formula: string | undefined let formulaResult: CellValue | undefined + let formulaResultType: Cell["formulaResultType"] let richText: RichTextRun[] | undefined // Handle formula (including shared formula slave cells with no text) @@ -1935,9 +1936,22 @@ function processCell( // `value` still holds the error token either way, so spotting an // error by its value is unaffected; a *hard-coded* error cell, // which carries no formula, still reports `"error"`. See #497. + // + // The cached result is still a plain string, and so is the result + // of a formula that returned the *text* "#DIV/0!" (`t="str"`), so + // `formulaResultType` is what keeps the two apart — on the way in + // and, through the writer, on the way back out. See #599. + // + // `!== undefined`, not truthiness: a cell a shared formula covers + // has `formula: ""`, and its cached error is no less an error. Read + // as a bare error it lost the result, and the writer then emitted + // the shared formula with no ``. value = valueText - cellType = formula ? "formula" : "error" - if (formula) formulaResult = value + cellType = formula !== undefined ? "formula" : "error" + if (formula !== undefined) { + formulaResult = value + formulaResultType = "error" + } break } case "d": { @@ -2042,6 +2056,9 @@ function processCell( if (formulaResult !== undefined) { cell.formulaResult = formulaResult } + if (formulaResultType !== undefined) { + cell.formulaResultType = formulaResultType + } // Store formula type metadata if (formulaType === "shared") { cell.formulaType = "shared" diff --git a/test/clone-sheet-coverage.test.ts b/test/clone-sheet-coverage.test.ts index a03b04a..66c6306 100644 --- a/test/clone-sheet-coverage.test.ts +++ b/test/clone-sheet-coverage.test.ts @@ -30,7 +30,10 @@ const FULL_CELL: Required = { style: { font: { bold: true }, numFmt: "0.00" }, checkbox: true, formula: "", - formulaResult: 42, + // A string, because `formulaResultType: "error"` only means anything + // on one — an error token is text. See #599. + formulaResult: "#DIV/0!", + formulaResultType: "error", formulaType: "shared", formulaSharedIndex: 3, formulaRef: "A1:A9", diff --git a/test/formula-result-types.test.ts b/test/formula-result-types.test.ts index cb1dd9e..af18f22 100644 --- a/test/formula-result-types.test.ts +++ b/test/formula-result-types.test.ts @@ -102,8 +102,107 @@ describe("the round trip that was losing them", () => { // An `` with no `` is what anything that does not recalculate // reads as an empty cell. expect(sheetXml).not.toMatch(/[^<]*<\/f><\/c>/) - expect(sheetXml).toContain("xy") - expect(sheetXml).toContain("#DIV/0!") + expect(sheetXml).toContain('"x" & "y"xy') + // Not just the token: written as text, `t="str"`, it is a formula + // that returned the string "#DIV/0!" — a different cell. See #599. + expect(sheetXml).toContain('1/0#DIV/0!') + }) +}) + +// ═══════════════════════════════════════════════════════════════════════ +// #599 — `formulaResult` holds an error token as a plain string, so a +// formula whose cached result is `#DIV/0!` (`t="e"`) and a formula that +// returned the *text* "#DIV/0!" (`t="str"`) read back identically, and +// the writer turned the first into the second. ST_CellType (ECMA-376 +// Part 1, §18.18.11) keeps them apart; `formulaResultType` now does too. +// ═══════════════════════════════════════════════════════════════════════ + +const ERROR_RESULT = '1/0#DIV/0!' +const TEXT_LIKE_ERROR = '"#DIV/0!"#DIV/0!' +const HARD_CODED_ERROR = '#N/A' + +async function sheetXmlOf(data: Uint8Array): Promise { + return dec.decode(await new ZipReader(data).extract("xl/worksheets/sheet1.xml")) +} + +describe("an error result is told apart from a text result", () => { + it('reads `formulaResultType: "error"` only on the error', async () => { + const cells = await cellsOf(`${ERROR_RESULT}${TEXT_LIKE_ERROR}${HARD_CODED_ERROR}`) + + expect(cells.get("0,0")).toMatchObject({ + type: "formula", + formula: "1/0", + formulaResult: "#DIV/0!", + formulaResultType: "error", + }) + expect(cells.get("0,1")?.formulaResult).toBe("#DIV/0!") + expect(cells.get("0,1")).not.toHaveProperty("formulaResultType") + // No formula, so no formula result to qualify: `type` says it. + expect(cells.get("0,2")).toEqual({ value: "#N/A", type: "error" }) + }) + + it("is absent for every other kind of cached result", async () => { + const cells = await cellsOf(`${NUMBER}${TEXT}${BOOLEAN}`) + + for (const key of ["0,0", "0,1", "0,3"]) { + expect(cells.get(key)).not.toHaveProperty("formulaResultType") + } + }) + + it('readXlsx -> writeXlsx keeps `t="e"` and `t="str"` apart', async () => { + const first = await readXlsx( + await withCells(`${ERROR_RESULT}${TEXT_LIKE_ERROR}${HARD_CODED_ERROR}`), + ) + const rewritten = await writeXlsx({ + sheets: first.sheets.map((s) => ({ name: s.name, rows: s.rows, cells: s.cells })), + }) + const sheetXml = await sheetXmlOf(rewritten) + + expect(sheetXml).toContain('1/0#DIV/0!') + expect(sheetXml).toContain('"#DIV/0!"#DIV/0!') + expect(sheetXml).toContain('#N/A') + + const second = (await readXlsx(rewritten)).sheets[0]!.cells! + expect(second.get("0,0")?.formulaResultType).toBe("error") + expect(second.get("0,1")).not.toHaveProperty("formulaResultType") + }) + + it('writes an authored error result as `t="e"`', async () => { + const out = await writeXlsx({ + sheets: [ + { + name: "S", + cells: new Map([ + ["0,0", { formula: "1/0", formulaResult: "#DIV/0!", formulaResultType: "error" }], + ["0,1", { formula: '"#DIV/0!"', formulaResult: "#DIV/0!" }], + ]), + }, + ], + }) + const sheetXml = await sheetXmlOf(out) + + expect(sheetXml).toContain('1/0#DIV/0!') + // The token alone is not taken as a signal: without the field, a + // string result is text, as it always was. + expect(sheetXml).toContain('"#DIV/0!"#DIV/0!') + }) + + it("leaves a non-string result as it was", async () => { + // An error token is text, so `t="e"` with `42` is not a cell + // anything could read. The field qualifies a string result only, as + // its doc comment says. + const out = await writeXlsx({ + sheets: [ + { + name: "S", + cells: new Map([ + ["0,0", { formula: "B1*2", formulaResult: 42, formulaResultType: "error" }], + ]), + }, + ], + }) + + expect(await sheetXmlOf(out)).toContain('B1*242') }) }) @@ -146,3 +245,33 @@ describe("the value stays where callers look for it", () => { expect(wb.sheets[0]!.rows[0]).toEqual([24, "xy", "#DIV/0!", true]) }) }) + +describe("a shared formula's error result", () => { + // The cells a shared formula covers carry no formula text of their own + // — `` — so the reader gives them `formula: ""`. + // That is still a formula, and its cached error is still an error. + const MASTER = '1/0#DIV/0!' + const SLAVE = '#DIV/0!' + + it('reads `formulaResultType: "error"` on the covered cell too', async () => { + const cells = await cellsOf(`${MASTER}${SLAVE}`) + + expect(cells.get("0,1")).toMatchObject({ + formula: "", + formulaType: "shared", + formulaResult: "#DIV/0!", + formulaResultType: "error", + }) + }) + + it('keeps the covered cell\'s cached result and `t="e"` through a rewrite', async () => { + const first = await readXlsx(await withCells(`${MASTER}${SLAVE}`)) + const rewritten = await writeXlsx({ + sheets: first.sheets.map((s) => ({ name: s.name, rows: s.rows, cells: s.cells })), + }) + + expect(await sheetXmlOf(rewritten)).toContain( + '#DIV/0!', + ) + }) +})