Skip to content

HF-161: document OFFSET function limitations#1666

Open
marcin-kordas-hoc wants to merge 5 commits into
developfrom
feature/hf-161-offset-limitations
Open

HF-161: document OFFSET function limitations#1666
marcin-kordas-hoc wants to merge 5 commits into
developfrom
feature/hf-161-offset-limitations

Conversation

@marcin-kordas-hoc
Copy link
Copy Markdown
Collaborator

@marcin-kordas-hoc marcin-kordas-hoc commented Apr 29, 2026

Summary

Adds a single canonical ### OFFSET function sub-section under ## Nuances of the implemented functions in docs/guide/known-limitations.md. Documents all six behavioral limits of the OFFSET function in HyperFormula, each backed either by an existing test in unit/parser/offset-translation.spec.ts or by a runtime check captured before this PR was opened.

Removes the now-superseded one-row OFFSET entry from docs/guide/list-of-differences.md (per Kuba's decision in the 2026-04-21 meeting: "można wtedy to stąd też usunąć. Żeby wszystko było tam jednak"). The HF-vs-Excel/Sheets behavioral differences for OFFSET are not lost — they remain documented in full in known-limitations.md under the new sub-section, which is the canonical place for parse-time restrictions per the 04-21 decision to consolidate.

Note on PR routing: this PR replaces #1662 which was opened from a fork branch. Same content, now from upstream branch — CI will have full access (no fork-PR DEPLOY_TOKEN issue). Closing #1662 in favor of this one.

Linked

Limits documented

  1. First argument must be a single-cell reference (passing a range = parser error stored as cell value)
  2. Row/column/height/width arguments must be static integer literals (parser error otherwise)
  3. Height and width must be bare positive integer literals — NUMBER AST nodes only (unary +, parens, non-integers, values <1 all rejected at parse time)
  4. Out-of-sheet target → #REF! error stored at parse time (not evaluation time), with the message Resulting reference is out of the sheet
  5. getCellFormula returns the resolved reference, not the original =OFFSET(...)
  6. Architectural rationale: OFFSET is rewritten at parse time into a plain cell reference, so introspection via getCellFormula shows the resolved reference rather than the call

Runtime verification

All six limits were verified against this branch's HEAD before publishing:

A. OFFSET in registered names: false  (correct — OFFSET is parse-time, not registered)
B. getCellFormula recovers: "=B1"     (rewritten reference, NOT "=OFFSET(A1, 0, 1)")
C. Out-of-sheet value: { value: "#REF!", message: "Resulting reference is out of the sheet." }

Tests covering all six limits live in test/hyperformula-tests/unit/parser/offset-translation.spec.ts (24 tests, lines 13–206 in the private repo). Run via npm run test:jest -- --testPathPattern="offset-translation".

Test plan

  • CI green on handsontable/hyperformula
  • Netlify deploy preview: /guide/known-limitations — verify the new ### OFFSET function sub-section renders, including the four embedded js code blocks
  • Netlify deploy preview: /guide/list-of-differences — verify the table is intact and the OFFSET row is gone

Notes

  • This is docs-only — no CHANGELOG entry per project convention.
  • The internal ErrorMessage.OutOfSheet string is intentionally NOT quoted verbatim in the docs; the bullet describes the behavior instead, so future internal-string refactors don't break the docs.
  • Post-Codex review (2026-05-14): wording clarified to distinguish parser-error-as-cell-value vs API exception, and to specify that height/width accept only bare NUMBER literals (unary + etc. rejected).

@qunabu
Copy link
Copy Markdown

qunabu commented Apr 29, 2026

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 29, 2026

Deploy Preview for hyperformula-docs ready!

Name Link
🔨 Latest commit 8fb75e9
🔍 Latest deploy log https://app.netlify.com/projects/hyperformula-docs/deploys/6a05586b8416110008e95daa
😎 Deploy Preview https://deploy-preview-1666--hyperformula-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 29, 2026

Performance comparison of head (8fb75e9) vs base (d213a57)

                                     testName |   base |   head |  change
-------------------------------------------------------------------------
                                      Sheet A | 515.56 | 497.66 |  -3.47%
                                      Sheet B | 177.44 | 158.26 | -10.81%
                                      Sheet T | 151.63 | 135.16 | -10.86%
                                Column ranges | 534.73 | 520.89 |  -2.59%
Sheet A:  change value, add/remove row/column |  15.52 |  17.18 | +10.70%
 Sheet B: change value, add/remove row/column |  130.1 | 153.87 | +18.27%
                   Column ranges - add column | 151.78 | 163.85 |  +7.95%
                Column ranges - without batch | 462.77 | 493.42 |  +6.62%
                        Column ranges - batch | 122.74 | 123.45 |  +0.58%

@marcin-kordas-hoc marcin-kordas-hoc force-pushed the feature/hf-161-offset-limitations branch from dac40ae to 563642b Compare April 29, 2026 09:49
Per Q2 principle (describe runtime-observable behavior, not internal
strings): replace verbatim copies of internal parser error messages
with behavioral descriptions. Future internal-string refactors will
no longer silently invalidate these doc comments.
- Replace 'Raises a parsing error' wording with explicit 'cell stores
  a parser error' — the JS API does not throw an exception
- Specify 'bare positive integer literals' instead of 'positive integers'
  to reflect that the parser accepts only NUMBER AST nodes (so `+1` and
  parenthesised expressions are also rejected, not just non-integers)
- Reframe the out-of-sheet bullet to make the parse-time-vs-evaluation
  distinction explicit (matches Excel value but not detection timing)
@netlify
Copy link
Copy Markdown

netlify Bot commented May 14, 2026

Deploy Preview for hyperformula-dev-docs ready!

Name Link
🔨 Latest commit 8fb75e9
🔍 Latest deploy log https://app.netlify.com/projects/hyperformula-dev-docs/deploys/6a05586b74e1d60008529777
😎 Deploy Preview https://deploy-preview-1666--hyperformula-dev-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@marcin-kordas-hoc marcin-kordas-hoc requested a review from sequba May 15, 2026 07:16
@codecov
Copy link
Copy Markdown

codecov Bot commented May 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.16%. Comparing base (d213a57) to head (8fb75e9).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1666   +/-   ##
========================================
  Coverage    97.16%   97.16%           
========================================
  Files          175      175           
  Lines        15319    15319           
  Branches      3287     3287           
========================================
  Hits         14884    14884           
  Misses         435      435           
🚀 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.

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