Skip to content

Fix #2966: convert formula result to Date when numFmt is date-like#55

Open
senoff wants to merge 1 commit intoprotobi:masterfrom
senoff:fix-2966-formula-result-date
Open

Fix #2966: convert formula result to Date when numFmt is date-like#55
senoff wants to merge 1 commit intoprotobi:masterfrom
senoff:fix-2966-formula-result-date

Conversation

@senoff
Copy link
Copy Markdown

@senoff senoff commented May 1, 2026

Bug

Tracked at exceljs#2966.

When a cell holds a formula whose cached result is a numeric Excel date
serial and the cell's numFmt is a date format (e.g. m/d/yyyy), the
reader is supposed to materialize cell.value.result as a JS Date.

It does in the common case (<v>46143</v> with no t attribute), but
not when an off-spec writer marks the formula cell with t=\"str\"
and still puts a numeric serial inside <v>. After PR exceljs#2956's guard
in excelToDate, the reader stopped throwing Invalid Date for that
shape, but the value was left as the raw string "46143.20833333333"

  • still wrong for any consumer that expects a Date.

The streaming WorkbookReader had the same gap and never applied
date conversion to formula results at all.

Fix

lib/xlsx/xform/sheet/cell-xform.js (reconcile, Formula branch) and
lib/stream/xlsx/worksheet-reader.js (formula cell handling) both now:

  1. Detect numFmt is date-like (utils.isDateFmt).
  2. If result is a string but parses as a finite number that round-trips
    exactly, coerce to that number first.
  3. Pass through utils.excelToDate, which already handles plain numbers
    and (post-Fix the return value from dateToExcel() when it's passed a non-numeric value. exceljs/exceljs#2956) returns non-numeric input unchanged.

Genuine display strings like \"27/08/2025 19:33:34\" cannot be
reliably parsed as an Excel serial, so they fall through unchanged - we
never fabricate an Invalid Date.

Tests

spec/integration/issues/issue-2966-formula-result-date-numfmt.spec.js
covers four shapes:

  • numeric <v>, no t attribute (regression baseline)
  • numeric <v> with t=\"str\" (the actual bug)
  • non-numeric display string with t=\"str\" (must not become Invalid Date)
  • streaming reader: numeric serial with t=\"str\" becomes a Date

Fixtures are built programmatically: a workbook is written with the
real m/d/yyyy style, then the <c> element is rewritten in-place via
JSZip so the malformed shapes can be exercised without checking binary
fixtures into the repo.

npm run test:unit, npm run test:integration, and
npm run test:end-to-end all pass.

Notes

Two unrelated multi-line function calls in the touched files were
collapsed to single-line form to satisfy the prettier+eslint hook
(prettier wants trailingComma: \"all\", eslint enforces
comma-dangle.functions: \"never\"). Behavior is unchanged.

…like

When a cell's formula result is cached as t="str" with a numeric Excel
date serial in <v> (some non-Excel writers do this), the reader left
the result as a string instead of converting it to a JS Date the way
it does for plain numeric formula results. After PR exceljs#2956's guard,
this no longer produced an Invalid Date, but the value was still wrong
for downstream consumers that expect a Date.

The standard (non-streaming) reconcile path and the streaming reader
both now coerce string-typed formula results that look like a finite
number before passing through excelToDate. Genuine non-numeric display
strings (e.g., "27/08/2025 19:33:34") fall through unchanged so we
never fabricate an Invalid Date.

Adds spec/integration/issues/issue-2966-formula-result-date-numfmt
covering: numeric serial with no t, numeric serial with t="str", a
non-numeric display string, and the streaming reader.

Two unrelated multi-line function calls in the touched files were
collapsed to single-line form to satisfy the prettier+eslint hook
(prettier wants a trailing comma, eslint forbids it on functions).
Behavior is unchanged.

Refs: exceljs#2966

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.

1 participant