Skip to content

HF-270: Exporter honours evaluateNullToZero for bare empty cells - #1738

Open
marcin-kordas-hoc wants to merge 1 commit into
developfrom
spike/hf270-null-to-zero
Open

HF-270: Exporter honours evaluateNullToZero for bare empty cells#1738
marcin-kordas-hoc wants to merge 1 commit into
developfrom
spike/hf270-null-to-zero

Conversation

@marcin-kordas-hoc

@marcin-kordas-hoc marcin-kordas-hoc commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Fixes HF-270: the evaluateNullToZero config option is honoured by the interpreter (a formula reading an empty cell already coerces empty → 0) but was ignored by the Exporter, which always returned null for a bare empty cell read directly — so with the flag on, getSheetValues over [[null, '=A1']] gave [[null, 0]].

What changed

The Exporter's EmptyValue branch returns 0 when config.evaluateNullToZero is true, null otherwise. Default behaviour (flag off/absent) is byte-identical. ISBLANK still reports true for the bare empty cell either way. The CHANGELOG entry names the caveat: code relying on null from getCellValue to distinguish truly-empty from computed-zero under this flag should use ISBLANK instead.

Implemented independently from the task description (not taken from the fork referenced there).

Verification

Paired tests: hyperformula-tests@spike/hf270-null-to-zero (22 assertions across getCellValue / getSheetValues / ExportedCellChange / ISBLANK / default-off). tsc --noEmit clean, eslint --quiet clean.

🤖 Generated with Claude Code


Note

Medium Risk
Behavior change for export APIs when evaluateNullToZero is enabled may break code that relied on null to detect empty cells; default-off path is unchanged.

Overview
Fixes HF-270: when evaluateNullToZero is enabled, the interpreter already treats empty cells as 0 inside formulas, but Exporter.exportValue always mapped EmptyValue to null, so direct reads via getCellValue, getSheetValues, getAllSheetsValues, and ExportedCellChange stayed inconsistent (e.g. [[null, '=A1']][[null, 0]]).

The EmptyValue branch now returns 0 when config.evaluateNullToZero is true, otherwise null. With the flag off or absent, export behaviour is unchanged. ISBLANK still treats bare empty cells as blank either way.

CHANGELOG documents the caveat: callers that used null from export APIs to distinguish truly empty cells from computed zero under this flag should use ISBLANK instead.

Reviewed by Cursor Bugbot for commit 10e48b2. Bugbot is set up for automated code reviews on this repo. Configure here.

With evaluateNullToZero: true the interpreter already coerced empty -> 0
inside formulas, but the Exporter returned null for a bare empty cell read
directly - getSheetValues over [[null, '=A1']] gave [[null, 0]]. The
EmptyValue branch now returns 0 when the flag is on; default behaviour is
byte-identical, ISBLANK still reports true. CHANGELOG names the caveat:
code using null to distinguish truly-empty under this flag needs ISBLANK.

Implemented by a prep-ship lane (task HF-270), loop green after 1
iteration; verified: authored specs 22/22, eslint clean, tsc clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PdHPZAjciZFWqGa19Yf7it
@qunabu

qunabu commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
hyperformula-docs 10e48b2 Commit Preview URL

Branch Preview URL
Aug 14 2026, 12:36 PM

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown

Performance comparison of head (10e48b2) vs base (61ead73)

                                     testName |   base |    head | change
-------------------------------------------------------------------------
                                      Sheet A | 492.25 |  497.94 | +1.16%
                                      Sheet B | 155.65 |   159.7 | +2.60%
                                      Sheet T | 140.21 |  141.63 | +1.01%
                                Column ranges | 467.36 |  477.88 | +2.25%
                                Sorted lookup |  14129 | 14601.1 | +3.34%
Sheet A:  change value, add/remove row/column |     16 |   15.89 | -0.69%
 Sheet B: change value, add/remove row/column | 142.39 |   131.7 | -7.51%
                   Column ranges - add column | 153.98 |  145.25 | -5.67%
                Column ranges - without batch | 469.87 |  448.41 | -4.57%
                        Column ranges - batch | 120.01 |  114.45 | -4.63%

@marcin-kordas-hoc
marcin-kordas-hoc marked this pull request as ready for review August 18, 2026 05:48
@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.32%. Comparing base (61ead73) to head (10e48b2).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1738      +/-   ##
===========================================
+ Coverage    97.31%   97.32%   +0.01%     
===========================================
  Files          195      195              
  Lines        15719    15719              
  Branches      3455     3385      -70     
===========================================
+ Hits         15297    15299       +2     
- Misses         414      420       +6     
+ Partials         8        0       -8     
Files with missing lines Coverage Δ
src/Exporter.ts 89.33% <100.00%> (+2.66%) ⬆️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@marcin-kordas-hoc

Copy link
Copy Markdown
Collaborator Author

Paired tests PR: handsontable/hyperformula-tests#39 — merge it BEFORE this one.

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.

2 participants