fix(xlsx): keep a formula's error result apart from a text result

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

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) */

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

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

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<Array<ResolvedCell | null>> {
         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"

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 `<v>`.
       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"

test/clone-sheet-coverage.test.ts

@@ -30,7 +30,10 @@ const FULL_CELL: Required<Cell> = {
   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",

test/formula-result-types.test.ts

@@ -102,8 +102,107 @@ describe("the round trip that was losing them", () => {
     // An `<f>` with no `<v>` is what anything that does not recalculate
     // reads as an empty cell.
     expect(sheetXml).not.toMatch(/<f>[^<]*<\/f><\/c>/)
-    expect(sheetXml).toContain("xy")
-    expect(sheetXml).toContain("#DIV/0!")
+    expect(sheetXml).toContain('<c r="B1" t="str"><f>"x" &amp; "y"</f><v>xy</v></c>')
+    // 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('<c r="C1" t="e"><f>1/0</f><v>#DIV/0!</v></c>')
+  })
+})
+
+// ═══════════════════════════════════════════════════════════════════════
+// #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 = '<c r="A1" t="e"><f>1/0</f><v>#DIV/0!</v></c>'
+const TEXT_LIKE_ERROR = '<c r="B1" t="str"><f>"#DIV/0!"</f><v>#DIV/0!</v></c>'
+const HARD_CODED_ERROR = '<c r="C1" t="e"><v>#N/A</v></c>'
+
+async function sheetXmlOf(data: Uint8Array): Promise<string> {
+  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('<c r="A1" t="e"><f>1/0</f><v>#DIV/0!</v></c>')
+    expect(sheetXml).toContain('<c r="B1" t="str"><f>"#DIV/0!"</f><v>#DIV/0!</v></c>')
+    expect(sheetXml).toContain('<c r="C1" t="e"><v>#N/A</v></c>')
+
+    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('<c r="A1" t="e"><f>1/0</f><v>#DIV/0!</v></c>')
+    // The token alone is not taken as a signal: without the field, a
+    // string result is text, as it always was.
+    expect(sheetXml).toContain('<c r="B1" t="str"><f>"#DIV/0!"</f><v>#DIV/0!</v></c>')
+  })
+
+  it("leaves a non-string result as it was", async () => {
+    // An error token is text, so `t="e"` with `<v>42</v>` 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('<c r="A1"><f>B1*2</f><v>42</v></c>')
   })
 })
 
@@ -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
+  // — `<f t="shared" si="0"/>` — so the reader gives them `formula: ""`.
+  // That is still a formula, and its cached error is still an error.
+  const MASTER = '<c r="A1" t="e"><f t="shared" ref="A1:B1" si="0">1/0</f><v>#DIV/0!</v></c>'
+  const SLAVE = '<c r="B1" t="e"><f t="shared" si="0"/><v>#DIV/0!</v></c>'
+
+  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(
+      '<c r="B1" t="e"><f t="shared" si="0"/><v>#DIV/0!</v></c>',
+    )
+  })
+})