From 10a5c5d1d1ab597ddb7d898f534908b1ed9a3136 Mon Sep 17 00:00:00 2001 From: Pieter Sheth-Voss Date: Wed, 6 May 2026 23:37:12 -0400 Subject: [PATCH 1/5] Fix: pin uuid to ^9.0.1 to keep browserify build working (#64) uuid@8 is npm-deprecated and uuid@14 (current latest) drops the main field plus ships ES2021 (??=) syntax that browserify@16 cannot parse. uuid@9.0.1 is the highest version with a main field and CJS output that browserify@16 accepts. Stage: git add package.json package-lock.json --- package-lock.json | 27 +++++++++------------------ package.json | 2 +- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/package-lock.json b/package-lock.json index 58490005c..51fcab5ad 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,7 +17,7 @@ "saxes": "^5.0.1", "tmp": "^0.2.0", "unzipper": "^0.10.11", - "uuid": "^8.3.0" + "uuid": "^9.0.1" }, "devDependencies": { "@babel/cli": "^7.10.5", @@ -12911,22 +12911,6 @@ } } }, - "node_modules/puppeteer/node_modules/typescript": { - "version": "5.9.3", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.9.3.tgz", - "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", - "dev": true, - "license": "Apache-2.0", - "optional": true, - "peer": true, - "bin": { - "tsc": "bin/tsc", - "tsserver": "bin/tsserver" - }, - "engines": { - "node": ">=14.17" - } - }, "node_modules/puppeteer/node_modules/yargs": { "version": "17.7.1", "resolved": "https://registry.npmjs.org/yargs/-/yargs-17.7.1.tgz", @@ -15785,7 +15769,14 @@ } }, "node_modules/uuid": { - "version": "8.3.2", + "version": "9.0.1", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-9.0.1.tgz", + "integrity": "sha512-b+1eJOlsR9K8HJpow9Ok3fiWOWSIcIzXodvv0rQjVoOVNpWMpxf1wZNpt4y9h10odCNrqnYp1OBzRktckBe3sA==", + "deprecated": "uuid@10 and below is no longer supported. For ESM codebases, update to uuid@latest. For CommonJS codebases, use uuid@11 (but be aware this version will likely be deprecated in 2028).", + "funding": [ + "https://github.com/sponsors/broofa", + "https://github.com/sponsors/ctavan" + ], "license": "MIT", "bin": { "uuid": "dist/bin/uuid" diff --git a/package.json b/package.json index ba749545a..a3e23ddb9 100644 --- a/package.json +++ b/package.json @@ -111,7 +111,7 @@ "saxes": "^5.0.1", "tmp": "^0.2.0", "unzipper": "^0.10.11", - "uuid": "^8.3.0" + "uuid": "^9.0.1" }, "devDependencies": { "@babel/cli": "^7.10.5", From 41fa3aec3bbde71d870b8ea14247d2f447c28739 Mon Sep 17 00:00:00 2001 From: Pieter Sheth-Voss Date: Wed, 6 May 2026 23:37:25 -0400 Subject: [PATCH 2/5] Add AGENTS.md with hard rules for AI-generated PRs (#65) * Fix: pin uuid to ^9.0.1 to keep browserify build working uuid@8 is npm-deprecated and uuid@14 (current latest) drops the main field plus ships ES2021 (??=) syntax that browserify@16 cannot parse. uuid@9.0.1 is the highest version with a main field and CJS output that browserify@16 accepts. Stage: git add package.json package-lock.json * Add AGENTS.md with hard rules for AI-generated PRs Captures recurring problems from AI-authored PRs: drive-by formatter sweeps, synthetic-model tests instead of real round-trip, missing file lists, .prettierrc edits across overlapping PRs, and unverified XLSX serialization changes. README and CONTRIBUTING point to AGENTS.md. PR template adds a Files-changed section and a checklist mirroring the rules. Stage: git add AGENTS.md README.md CONTRIBUTING.md .github/PULL_REQUEST_TEMPLATE.md --- .github/PULL_REQUEST_TEMPLATE.md | 21 ++++++- AGENTS.md | 96 ++++++++++++++++++++++++++++++++ CONTRIBUTING.md | 2 + README.md | 10 ++-- 4 files changed, 122 insertions(+), 7 deletions(-) create mode 100644 AGENTS.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 469fc0d20..d8540a289 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,13 +1,28 @@ - + + + ## Summary +## Files changed + + + ## Test plan - + ## Related to source code (for typings update) - + + +## Checklist + +- [ ] Only the lines required for the fix/feature are changed (no formatter sweeps, no drive-by refactors) +- [ ] Tests use real fixture round-trip (`wb.xlsx.load` / `writeBuffer`), not synthetic model objects, where possible +- [ ] Checked open PRs (`gh pr list --state open`) for conflicting changes to the same files +- [ ] (If serialization is touched) Output verified to open in Excel, or `soffice --headless` round-trip clean +- [ ] (If `--no-verify` was used to bypass the pre-commit hook) Reason explained above +- [ ] (If a major dep was bumped) Runtime smoke test described above diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..9fead244e --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,96 @@ +# AGENTS.md + +Instructions for AI coding agents (Claude Code, Cursor, Codex, Copilot Workspace, etc.) opening pull requests against this repository. + +Human contributors: see [CONTRIBUTING.md](CONTRIBUTING.md). The rules below also apply to any human submitting AI-generated code. + +If you are an AI agent, read this file in full before making changes. The patterns below come from real PRs we have had to reject or rework. + +--- + +## Hard rules + +### 1. Surgical scope + +- Touch ONLY the lines required for the fix or feature. +- Do NOT extract constants, rename variables, collapse `Promise.all`, rewrite `reduce` to `for`, or "clean up" surrounding code, even if your linter or style tool suggests it. +- If the pre-commit hook (prettier ↔ eslint conflict) fights you, commit with `--no-verify` and call this out in the PR description. Do NOT reshape unrelated code to satisfy the hook. + +### 2. No formatter sweeps + +- Do NOT run `npx prettier --write .`, `eslint --fix` on whole files, or any tool that rewrites files you did not otherwise change. +- Do NOT modify `.prettierrc`, `.eslintrc`, or other config files in a feature/bugfix PR. If a config change is needed, open a separate config-only PR. + +### 3. PR description must be complete + +- List EVERY file changed in the PR description, including configs, lockfiles, and incidental edits. +- If you used `--no-verify`, say so and explain why. +- If your change depends on or conflicts with another open PR, link it. + +### 4. Tests must use real fixtures, not synthetic models + +- For XLSX read/write fixes, your test MUST round-trip through `wb.xlsx.load(...)` and/or `wb.xlsx.writeBuffer(...)` against a real or minimal fixture file. +- Do NOT build hand-rolled model objects and call internal serializers (e.g. `XLSX.reconcile`, individual xforms) directly. Synthetic-model tests can pass while the real load/write path still breaks. +- Place fixture files under `spec/integration/data/` following existing naming. + +### 5. Cross-check open PRs before submitting + +- Before opening a PR, list the currently open PRs in this repo and verify your change does not touch the same files or overlap in scope. The maintainer has had three concurrent PRs all editing `.prettierrc` because none of them checked. +- Run: `gh pr list --state open --limit 50 --json number,title,files` + +### 6. XLSX serialization changes must be Excel-verified + +If your change touches anything that writes XLSX (xforms, sheet/workbook serialization, pivot tables, charts, comments, conditional formatting): + +- Verify the output file actually opens in Excel without a "Repaired Records" warning, OR +- Round-trip with LibreOffice headless and inspect the bytes: + ```bash + soffice --headless --convert-to xlsx /tmp/your-output.xlsx --outdir /tmp/roundtrip + unzip -p /tmp/roundtrip/your-output.xlsx xl/.xml | head + ``` +- Unit tests do NOT catch Excel's repair warnings. This step is mandatory for serialization-touching PRs. + +### 7. Dependency bumps require runtime smoke tests + +- Major-version dep bumps (e.g. `fast-csv`, `unzipper`, `archiver`) MUST be smoke-tested against the runtime paths that use them, not just `npm test`. +- Streaming reader changes: load a real `.xlsx`. CSV writer changes: write and re-parse real CSV. Document the smoke test in the PR. + +### 8. One concern per PR + +- Bug fix + sibling bugs of the same shape in the same file: ONE PR. Good. +- Bug fix + unrelated cleanup + dep bump: THREE PRs. Required. +- If you find yourself touching `index.d.ts`, `README.md`, `package-lock.json`, etc. incidentally, stop and revert those edits unless they are the actual subject of the PR. + +--- + +## Per-PR checklist (also in `.github/pull_request_template.md`) + +Before opening the PR, verify: + +- [ ] Only the lines required for the fix are changed +- [ ] No prettier/eslint sweeps on unrelated files +- [ ] Every file change is listed in the PR description below +- [ ] Tests use real fixture round-trip (`wb.xlsx.load` / `writeBuffer`), not synthetic models +- [ ] `gh pr list --state open` checked for conflicting PRs +- [ ] (If serialization) Output file opens in Excel or `soffice --headless` without warnings +- [ ] (If `--no-verify` used) Reason noted in PR description +- [ ] (If dep bump) Runtime smoke test described in PR + +--- + +## Repository conventions + +- Code style: enforced by ESLint + Prettier (the conflict above is real — live with it on a per-PR basis) +- Tests: Mocha, in `spec/unit/`, `spec/integration/`, `spec/end-to-end/` +- Run unit tests fast: `npm run test:unit` (skips the build step) +- Full suite: `npm test` (build + unit + integration + e2e + jasmine) +- Node target: see `engines` in `package.json` +- Browserify is in the build chain. Do NOT introduce dependencies that ship ES2021+ syntax (`??=`, `?.()`) in their CJS output, or that publish `exports`-only packages without a `main` field. Both break the browser bundle. + +--- + +## What this fork is + +This is a curated Protobi fork of [exceljs/exceljs](https://github.com/exceljs/exceljs). We selectively adopt upstream features for production use. We are NOT a general-purpose alternative fork. See [CONTRIBUTING.md](CONTRIBUTING.md) for what we accept. + +If your change is a generic improvement that has not been merged upstream, consider opening it against [exceljs/exceljs](https://github.com/exceljs/exceljs) first. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0ba3f8965..8a953aad5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,6 +2,8 @@ Thank you for your interest in contributing! +> **AI agents and humans submitting AI-generated code:** read [AGENTS.md](AGENTS.md) first. It has hard rules about PR scope, formatter sweeps, real-fixture testing, and Excel-verification for serialization changes. PRs that ignore those rules get closed regardless of the underlying fix quality. + ## About This Fork This is a **curated fork** of ExcelJS maintained by Protobi. We selectively adopt features from the upstream repository that we need for our production systems. diff --git a/README.md b/README.md index 1c8e80eab..406aa0984 100644 --- a/README.md +++ b/README.md @@ -145,11 +145,13 @@ npm install exceljs Contributions are very welcome! It helps me know what features are desired or what bugs are causing the most pain. -I have just one request; If you submit a pull request for a bugfix, please add a unit-test or integration-test (in the spec folder) that catches the problem. - Even a PR that just has a failing test is fine - I can analyse what the test is doing and fix the code from that. +**Before opening a PR, read [CONTRIBUTING.md](CONTRIBUTING.md).** -Note: Please try to avoid modifying the package version in a PR. -Versions are updated on release and any change will most likely result in merge collisions. +**If you are an AI agent (Claude Code, Cursor, Codex, Copilot Workspace, etc.) — or a human submitting AI-generated code — read [AGENTS.md](AGENTS.md) in full first.** It contains hard rules about scope, formatter sweeps, real-fixture testing, and Excel-verification for serialization changes. AI-generated PRs that ignore these get closed. + +For humans: if you submit a pull request for a bugfix, please add a unit-test or integration-test (in the `spec/` folder) that catches the problem. Even a PR that just has a failing test is fine — I can analyse what the test is doing and fix the code from that. + +Note: Please try to avoid modifying the package version in a PR. Versions are updated on release and any change will most likely result in merge collisions. To be clear, all contributions added to this library will be included in the library's MIT licence. From 3f0bfdc21ef87653e86158137cc05486cf94d398 Mon Sep 17 00:00:00 2001 From: Pieter Sheth-Voss Date: Wed, 6 May 2026 23:37:31 -0400 Subject: [PATCH 3/5] Cherry-pick #50 + add regression tests for richText shared-string dedup (#66) * fix: use XML hash for streaming shared strings * Add regression tests for richText shared-string deduplication Covers the bug fixed in the cherry-picked commit: richText objects previously collapsed to hash key '[object Object]', deduping every richText cell into a single shared-string entry. Tests verify both correct deduplication and distinction by formatting. --------- Co-authored-by: Gavin Kline --- lib/utils/shared-strings.js | 11 +++++++-- spec/unit/utils/shared-strings.spec.js | 32 ++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/lib/utils/shared-strings.js b/lib/utils/shared-strings.js index c3e46af06..070f3e1f2 100644 --- a/lib/utils/shared-strings.js +++ b/lib/utils/shared-strings.js @@ -1,3 +1,5 @@ +const SharedStringXform = require('../xlsx/xform/strings/shared-string-xform'); + class SharedStrings { constructor() { this._values = []; @@ -21,10 +23,15 @@ class SharedStrings { return this._values[index]; } + get sharedStringXform() { + return this._sharedStringXform || (this._sharedStringXform = new SharedStringXform()); + } + add(value) { - let index = this._hash[value]; + const hashKey = value && value.richText ? this.sharedStringXform.toXml(value) : value; + let index = this._hash[hashKey]; if (index === undefined) { - index = this._hash[value] = this._values.length; + index = this._hash[hashKey] = this._values.length; this._values.push(value); } this._totalRefs++; diff --git a/spec/unit/utils/shared-strings.spec.js b/spec/unit/utils/shared-strings.spec.js index bb9c5bd9f..4f153b776 100644 --- a/spec/unit/utils/shared-strings.spec.js +++ b/spec/unit/utils/shared-strings.spec.js @@ -25,4 +25,36 @@ describe('SharedStrings', () => { expect(ss.getString(iXml)).to.equal('value'); expect(ss.getString(iAmpersand)).to.equal('&'); }); + + it('Deduplicates richText values by their XML representation', () => { + // regression: previously, richText objects all collapsed to the + // hash key "[object Object]", deduping every richText cell into one entry + const ss = new SharedStrings(); + + const a = {richText: [{text: 'Hello'}]}; + const b = {richText: [{text: 'Hello'}]}; + const c = {richText: [{text: 'Goodbye'}]}; + + const iA = ss.add(a); + const iB = ss.add(b); + const iC = ss.add(c); + + expect(iA).to.equal(iB); + expect(iC).to.not.equal(iA); + expect(ss.count).to.equal(2); + expect(ss.totalRefs).to.equal(3); + }); + + it('Distinguishes richText entries that differ only in formatting', () => { + const ss = new SharedStrings(); + + const plain = {richText: [{text: 'Hello'}]}; + const bold = {richText: [{font: {bold: true}, text: 'Hello'}]}; + + const iPlain = ss.add(plain); + const iBold = ss.add(bold); + + expect(iPlain).to.not.equal(iBold); + expect(ss.count).to.equal(2); + }); }); From 46db092b8a08a498c008f98f69404bb97c9985c2 Mon Sep 17 00:00:00 2001 From: Pieter Sheth-Voss Date: Wed, 6 May 2026 23:51:55 -0400 Subject: [PATCH 4/5] Release 4.4.0-protobi.10: Fix richText shared-string dedup + uuid pin Fixes critical bug in WorkbookWriter streaming with useSharedStrings: every richText cell collapsed into a single shared-string entry (richText objects coerced to '[object Object]' as hash key, deduping all of them). Cherry-picked from #50 by @gwkline; adds regression tests covering richText deduplication and formatting-aware distinction. Addresses upstream exceljs/exceljs#2267. Also pins uuid to ^9.0.1 to keep browserify build working (uuid@14 dropped the main field; uuid@11 ships ES2021 syntax that browserify@16 cannot parse). Adds AGENTS.md with hard rules for AI-generated PRs. Files changed: - lib/utils/shared-strings.js: hash richText by rendered XML - spec/unit/utils/shared-strings.spec.js: 2 new regression tests - AGENTS.md: new (hard rules for AI agents) - README.md, CONTRIBUTING.md: pointers to AGENTS.md, release notes - .github/PULL_REQUEST_TEMPLATE.md: per-PR checklist - FORK.md: release notes - package.json, package-lock.json: version bump, uuid 8 -> 9 Tests: 886 unit tests passing (4 in SharedStrings, up from 2). PRs: #64 (uuid), #65 (AGENTS.md), #66 (richText fix) --- FORK.md | 29 +++++++++++++++++++++++++++++ README.md | 7 +++++++ package-lock.json | 4 ++-- package.json | 2 +- 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/FORK.md b/FORK.md index 216edd925..e53d4ffee 100644 --- a/FORK.md +++ b/FORK.md @@ -28,6 +28,35 @@ This AI-assisted workflow enables rapid response to community issues while maint ## Fork Release History +### 4.4.0-protobi.10 (2026-05-06) + +**Bug Fix: richText shared-string deduplication in streaming writer** ([#66](https://github.com/protobi/exceljs/pull/66), cherry-picked from [#50](https://github.com/protobi/exceljs/pull/50) by @gwkline) + +When using `WorkbookWriter` with `useSharedStrings` enabled, every cell containing richText collapsed into a single shared-string entry, corrupting the strings table. Root cause: `SharedStrings.add()` used the value directly as a hash key; richText objects coerced to the string `"[object Object]"`, deduping all of them into one entry. + +**Fix:** Hash richText values by their rendered XML representation (via `SharedStringXform.toXml()`). Plain strings unchanged. + +**Tests:** Adds two regression tests (`spec/unit/utils/shared-strings.spec.js`): +1. Equal richText values share one entry; different ones get separate entries +2. richText values that differ only in formatting are NOT deduped + +**Upstream context:** This bug was first reported as [exceljs/exceljs#2267](https://github.com/exceljs/exceljs/issues/2267) (May 2023). An alternative fix was opened upstream as [exceljs/exceljs#2588](https://github.com/exceljs/exceljs/pull/2588) (Nov 2023) but has been stale since Feb 2024. + +**Build Fix: Pin uuid to ^9.0.1** ([#64](https://github.com/protobi/exceljs/pull/64)) + +`npm install` had drifted to `uuid@14.0.0`, which broke `npm run build`: +- uuid@14 dropped the `main` field in favor of `exports`-only — browserify@16 doesn't honor `exports` +- uuid@11 (npm's recommended CJS target) ships ES2021 syntax (`??=`) browserify@16 can't parse +- uuid@9.0.1 is the highest version that keeps both `main` and pre-ES2021 CJS output + +**Documentation: AGENTS.md for AI-generated PRs** ([#65](https://github.com/protobi/exceljs/pull/65)) + +Adds `AGENTS.md` at repo root with hard rules for AI coding agents (Claude Code, Cursor, Codex, etc.): surgical scope, no formatter sweeps, complete PR descriptions, real-fixture testing, cross-check open PRs, Excel-verification for serialization changes. Auto-discovered by AI tools; referenced from README.md, CONTRIBUTING.md, and the PR template. + +**Tests:** All 886 unit tests passing (4 in `SharedStrings`, up from 2). + +--- + ### 4.4.0-protobi.9 (2026-02-01) **New Feature: Pivot Table & Chart Round-Trip Preservation** ([#41](https://github.com/cjnoname/excelts/issues/41)) diff --git a/README.md b/README.md index 406aa0984..432b408f3 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,13 @@ See [FORK.md Release History](FORK.md#fork-release-history) for details. ## Fork Release Notes +**4.4.0-protobi.10** (May 2026) - Streaming richText Bug Fix + Build Fix +- **Fix richText shared-string deduplication in streaming writer** - Cherry-picked from PR #50 by @gwkline. Previously, every richText cell collapsed into a single shared-string entry because objects coerced to `"[object Object]"` as a hash key. Now richText values are hashed by their rendered XML representation. Addresses upstream [exceljs/exceljs#2267](https://github.com/exceljs/exceljs/issues/2267). +- Adds regression tests covering richText deduplication and formatting-aware distinction +- Pin uuid to ^9.0.1 to keep browserify build working (uuid@14 dropped the `main` field; uuid@11 ships ES2021 syntax browserify can't parse) +- Adds `AGENTS.md` with hard rules for AI-generated PRs +- All 886 unit tests passing + **4.4.0-protobi.9** (February 2026) - Pivot Table & Chart Round-Trip Preservation + Critical Bug Fixes - **Round-trip preservation for pivot tables and charts** - Read Excel files with existing pivot tables and charts, write them back without corruption - Hybrid preservation approach: stores raw XML while extracting minimal metadata for structural integrity diff --git a/package-lock.json b/package-lock.json index 51fcab5ad..f8fcbb81d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@protobi/exceljs", - "version": "4.4.0-protobi.9", + "version": "4.4.0-protobi.10", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@protobi/exceljs", - "version": "4.4.0-protobi.9", + "version": "4.4.0-protobi.10", "license": "MIT", "dependencies": { "archiver": "^7.0.1", diff --git a/package.json b/package.json index a3e23ddb9..da5de1b5a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@protobi/exceljs", - "version": "4.4.0-protobi.9", + "version": "4.4.0-protobi.10", "description": "Excel Workbook Manager - Temporary fork with pivot table enhancements and bug fixes pending upstream merge", "private": false, "license": "MIT", From 453da5f78a987cf14b924e6d0f63cc53771767bc Mon Sep 17 00:00:00 2001 From: Bob Senoff Date: Sat, 1 Aug 2026 10:55:02 -0500 Subject: [PATCH 5/5] Streaming reader: preserve a formula cell's cached ERROR token (t="e") MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The streaming WorkbookReader dropped the cached error result on a FORMULA cell. A cell like `1/0#DIV/0!` carries both a formula and a cached error result, but the formula branch only special-cased t="str"; every other type (including t="e") fell through to `parseFloat(c.v.text)`, so `parseFloat('#DIV/0!')` -> NaN and the token was lost. The bare-error `case 'e'` further down only covers non-formula error cells. Fix: add a `c.t === 'e'` case in the formula branch that mirrors the non-streaming `Workbook.load()` shape — `cellValue.result = {error: c.v.text}` — matching the bare-error case below it. Ordering is str -> e -> numeric fallback. Test (spec/integration/issues): writes a formula cell with a cached error result via the public `writeBuffer`, reads it back BOTH ways, and asserts the streaming WorkbookReader now matches the non-streaming oracle (`result:{error:'#DIV/0!'}`). Reverting the fix reds the streaming arm (result becomes NaN); the oracle arm stays green. Reader-only change — no serialization path touched. Committed with --no-verify per AGENTS.md (avoid prettier/eslint reshaping unrelated code); only the two intended files are staged. --- lib/stream/xlsx/worksheet-reader.js | 6 ++ ...s214-streaming-formula-error-token.spec.js | 62 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 spec/integration/issues/issue-xls214-streaming-formula-error-token.spec.js diff --git a/lib/stream/xlsx/worksheet-reader.js b/lib/stream/xlsx/worksheet-reader.js index 8cecd5c17..be39880f6 100644 --- a/lib/stream/xlsx/worksheet-reader.js +++ b/lib/stream/xlsx/worksheet-reader.js @@ -294,6 +294,12 @@ class WorksheetReader extends EventEmitter { if (c.v) { if (c.t === 'str') { cellValue.result = utils.xmlDecode(c.v.text); + } else if (c.t === 'e') { + // A cached ERROR result on a formula cell (t="e", e.g. =1/0 → #DIV/0!). + // Mirror the non-streaming Workbook.load() shape (result:{error}) and the + // bare-error case below — the parseFloat fallback would turn '#DIV/0!' into + // NaN and drop the token entirely. + cellValue.result = {error: c.v.text}; } else { cellValue.result = parseFloat(c.v.text); } diff --git a/spec/integration/issues/issue-xls214-streaming-formula-error-token.spec.js b/spec/integration/issues/issue-xls214-streaming-formula-error-token.spec.js new file mode 100644 index 000000000..a50b8d34d --- /dev/null +++ b/spec/integration/issues/issue-xls214-streaming-formula-error-token.spec.js @@ -0,0 +1,62 @@ +const {Readable} = require('stream'); + +const ExcelJS = verquire('exceljs'); + +// XLS-214 — the streaming WorkbookReader dropped the cached ERROR token on a FORMULA cell. +// +// A cell like `1/0#DIV/0!` carries a formula AND a cached error +// result. The streaming reader's formula branch only special-cased t="str"; everything else +// (including t="e") fell through to `parseFloat(c.v.text)`, so `parseFloat('#DIV/0!')` → NaN and +// the token was LOST. The bare-error `case 'e'` lower down only covers non-formula error cells. +// +// The oracle is the non-streaming `Workbook.load()`, which returns `result:{error:'#DIV/0!'}` for +// the same cell. This test writes such a cell with the (non-streaming) writer, reads it back BOTH +// ways, and asserts the streaming reader now matches the oracle. Revert the fork fix (the t="e" +// case in worksheet-reader.js) and the streaming assertion goes RED (result becomes NaN). +describe('github issues: XLS-214 streaming reader preserves a formula cell cached error token', () => { + let buffer; + let oracleValue; + let streamedValue; + + before(async () => { + const wb = new ExcelJS.Workbook(); + const ws = wb.addWorksheet('S'); + ws.getCell('A1').value = {formula: '1/0', result: {error: '#DIV/0!'}}; + buffer = await wb.xlsx.writeBuffer(); + + // Oracle: the non-streaming loader. + const oracleWb = new ExcelJS.Workbook(); + await oracleWb.xlsx.load(buffer); + oracleValue = oracleWb.getWorksheet('S').getCell('A1').value; + + // Subject: the streaming WorkbookReader path. + await new Promise((resolve, reject) => { + const reader = new ExcelJS.stream.xlsx.WorkbookReader(Readable.from(buffer), { + worksheets: 'emit', + sharedStrings: 'cache', + styles: 'cache', + hyperlinks: 'ignore', + entries: 'ignore', + }); + reader.on('worksheet', worksheet => + worksheet.on('row', row => { + if (row.number === 1) { + streamedValue = row.getCell(1).value; + } + }) + ); + reader.on('end', resolve); + reader.on('error', reject); + reader.read(); + }); + }); + + it('the non-streaming oracle returns result:{error} (the target shape)', () => { + expect(oracleValue).to.deep.equal({formula: '1/0', result: {error: '#DIV/0!'}}); + }); + + it('the streaming reader preserves the error token (not NaN) — matches the oracle', () => { + expect(streamedValue).to.deep.equal(oracleValue); + expect(streamedValue.result).to.deep.equal({error: '#DIV/0!'}); + }); +});