Full document, spreadsheet, slideshow, and diagram tooling
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

fix(sheets): runtime cross-sheet refs return actual values, not #REF! (#725)

Closes #725

scott 88873ee1 b4511b56

+146 -4
+1
CHANGELOG.md
··· 8 8 ## [Unreleased] 9 9 10 10 ### Fixed 11 + - Sheets: cross-sheet formula references (e.g. `='Sheet 1'!F11`) now resolve to actual values instead of returning `#REF!` (v0.62.16, #725). The formula engine in `src/sheets/formulas.ts` has always accepted a third `crossSheetResolver` argument and `tests/cross-sheet.test.ts` had 21 passing unit tests proving the parser works — but `evaluateFormula()` in `src/sheets/cell-computation.ts` (the one entry point every in-app cell flows through) never passed one. Every runtime cross-sheet formula hit the `#REF!` default branch. Built a CrossSheetResolver that walks `getYSheets()`, reads the named sheet's `cells` Y.Map, and recurses into formula cells with a sheet-scoped local resolver so nested references resolve in the correct sheet's context. 5 jsdom regression tests in `tests/sheets-cross-sheet-runtime.test.ts` build a real Y.Doc and pin: plain cross-ref, SUM over cross-sheet range, arithmetic, `#REF!` for missing sheet, and cross-sheet → formula-cell recursion. Caught while building a Homelab Budget workbook with Sheet 1 data + Sheet 2 summary. (#725) 11 12 - Docs: version-history preview now renders readable HTML instead of raw ProseMirror schema XML (v0.62.11, #719). Before: clicking a version entry dumped `<paragraph indent="0"><heading level="1">Title</heading><bulletList>…` into the preview pane — unreadable, and "Restore this version" was effectively blind. Added `renderYjsFragmentAsHtml()` in `src/version-panel.ts` that walks the decrypted Yjs XmlFragment and maps each node name to a sensible HTML tag (heading→h1..h6, paragraph→p, bulletList→ul, listItem→li, table/tableRow/tableCell/tableHeader→table/tr/td/th, codeBlock→pre/code, image→img, etc.), with HTML-escaped text and a `<div>` fallback for unknown node types. 11 regression tests in `tests/version-panel-render-preview.test.ts`. (#719) 12 13 - Sheets: Share button click now opens the in-app share dialog (v0.62.12, #720). The share-dialog markup shipped in `src/sheets/index.html` and the reusable `initShareDialog()` helper existed in `src/lib/share-dialog.ts`, but sheets' `main.ts` never called it — so clicking the Share button was a silent no-op. Threaded `keyString` through `session-bootstrap.ts`'s return value and added `initShareDialog({ docId, docType: 'sheets', keyString })` in sheets' init path. Share now opens a dialog with a live-generated link, mode selector, and expiry picker — same as docs. (#720) 13 14 - Docs: markdown source view + Export Markdown produce GFM tables instead of raw HTML (v0.62.9, #716). TipTap v3 emits `<table><colgroup><col>...<tbody><tr><th><p>header</p></th>...</tbody></table>` — header in `<tbody>` (no `<thead>`), cells wrapped in `<p>`. turndown-plugin-gfm refused to convert that shape so the entire `<table>` fell through to raw HTML in .md exports and the MD source view. Added `normalizeTipTapTables()` in `src/docs/markdown-export.ts` that pre-processes the HTML before turndown: drops `<colgroup>`, unwraps single-`<p>` cells, and promotes the first `<tr>` to `<thead>` when it has `<th>` cells. Pure string-based so it runs in both browser and Node. 3 regression tests in `tests/markdown-export.test.ts` cover the TipTap v3 shape. (#716)
+1 -1
package.json
··· 1 1 { 2 2 "name": "tools", 3 - "version": "0.62.15", 3 + "version": "0.62.16", 4 4 "private": true, 5 5 "type": "module", 6 6 "main": "electron/main.js",
+49 -3
src/sheets/cell-computation.ts
··· 3 3 import { createSpillState, clearSpillMaps, registerSpill, isSpillSource, isSpillTarget } from './spill-tracking.js'; 4 4 import type { SpillState } from './spill-tracking.js'; 5 5 import { isSparklineResult } from './sparkline.js'; 6 - import type { CellData, CellStore, RecalcCellData } from './types.js'; 7 - import { getCellData, getCells, getActiveSheet } from './core-state.js'; 6 + import type { CellData, CellStore, CellValue, CrossSheetResolver, RecalcCellData } from './types.js'; 7 + import { getCellData, getCells, getActiveSheet, getYSheets } from './core-state.js'; 8 + 9 + /** 10 + * Cross-sheet resolver built on top of the live Y.Doc-backed sheets map. 11 + * 12 + * #725 — previously `evaluateFormula` called `evaluate(formula, sameSheetResolver)` 13 + * without the 3rd cross-sheet argument, so any formula like `='Sheet 2'!A1` fell 14 + * through to the formula engine's default #REF! branch. This resolver walks 15 + * getYSheets() (the top-level Y.Map<sheetName, Y.Map>) and reads the named 16 + * sheet's `cells` sub-map, returning the referenced cell's value (or recursively 17 + * evaluating its formula). Sheet names with spaces like `'Sheet 1'` are 18 + * supported because the formula parser strips the surrounding quotes before 19 + * reaching us. 20 + */ 21 + const crossSheetResolver: CrossSheetResolver = { 22 + sheetExists(sheetName: string): boolean { 23 + const ySheets = getYSheets(); 24 + if (!ySheets) return false; 25 + return ySheets.has(sheetName); 26 + }, 27 + getSheetCellValue(sheetName: string, cellRef: string): CellValue | '' { 28 + const ySheets = getYSheets(); 29 + if (!ySheets || !ySheets.has(sheetName)) return ''; 30 + const sheet = ySheets.get(sheetName); 31 + if (!sheet) return ''; 32 + const cells = sheet.get('cells') as import('yjs').Map<import('yjs').Map<unknown>> | undefined; 33 + if (!cells) return ''; 34 + const yCell = cells.get(cellRef); 35 + if (!yCell) return ''; 36 + const f = (yCell.get('f') as string) || ''; 37 + if (f) { 38 + // Evaluate the formula in the target sheet's context. For simplicity, 39 + // recurse via evaluateFormula which uses the ACTIVE sheet's cells — 40 + // that's wrong when the target sheet isn't active, so substitute a 41 + // one-shot resolver that looks up cells only in the named sheet. 42 + const localRef = (ref: string): CellValue | '' => { 43 + const rc = cells.get(ref); 44 + if (!rc) return ''; 45 + const rf = (rc.get('f') as string) || ''; 46 + if (rf) return evaluate(rf, localRef, crossSheetResolver) as CellValue | ''; 47 + return ((rc.get('v') as CellValue) ?? '') as CellValue | ''; 48 + }; 49 + return evaluate(f, localRef, crossSheetResolver) as CellValue | ''; 50 + } 51 + return ((yCell.get('v') as CellValue) ?? '') as CellValue | ''; 52 + }, 53 + }; 8 54 9 55 export const evalCache = new Map<string, unknown>(); 10 56 const _spillState: SpillState = createSpillState(); ··· 56 102 if (!data) return ''; 57 103 if (data.f) return evaluateFormula(data.f) as string; 58 104 return (data.v ?? '') as string; 59 - }); 105 + }, crossSheetResolver); 60 106 evalCache.set(formula, result); 61 107 return result; 62 108 }
+95
tests/sheets-cross-sheet-runtime.test.ts
··· 1 + /** 2 + * @vitest-environment jsdom 3 + * 4 + * Regression test for #725: cross-sheet formula references returned #REF! 5 + * in the live app because `evaluateFormula()` in cell-computation.ts never 6 + * passed a cross-sheet resolver to `evaluate()`. The unit tests in 7 + * tests/cross-sheet.test.ts pass because they construct their own resolver, 8 + * but in-app formulas never got one. 9 + * 10 + * This test uses a real Yjs doc shaped exactly like the live app (top-level 11 + * ySheets map with named sub-maps, each containing a 'cells' sub-map of 12 + * cellId → {v, f}) and confirms the resolver walks sheets correctly. 13 + */ 14 + import { describe, it, expect, beforeEach } from 'vitest'; 15 + import * as Y from 'yjs'; 16 + import { initCoreState, setCellData, getCellData } from '../src/sheets/core-state.js'; 17 + import { evaluateFormula, clearEvalCache } from '../src/sheets/cell-computation.js'; 18 + 19 + function makeCell(ydoc: Y.Doc, v: unknown = '', f = ''): Y.Map<unknown> { 20 + const cell = new Y.Map(); 21 + ydoc.transact(() => { 22 + cell.set('v', v); 23 + cell.set('f', f); 24 + }); 25 + return cell; 26 + } 27 + 28 + describe('#725 — cross-sheet refs in live cell-computation path', () => { 29 + let ydoc: Y.Doc; 30 + let ySheets: Y.Map<Y.Map<unknown>>; 31 + 32 + beforeEach(() => { 33 + ydoc = new Y.Doc(); 34 + ySheets = ydoc.getMap('sheets') as Y.Map<Y.Map<unknown>>; 35 + initCoreState(ySheets); 36 + clearEvalCache(); 37 + 38 + // Build Sheet 1 with some data 39 + const sheet1 = new Y.Map(); 40 + const cells1 = new Y.Map(); 41 + ydoc.transact(() => { 42 + sheet1.set('name', 'Sheet 1'); 43 + sheet1.set('cells', cells1); 44 + cells1.set('A1', makeCell(ydoc, 10)); 45 + cells1.set('A2', makeCell(ydoc, 20)); 46 + cells1.set('A3', makeCell(ydoc, 30)); 47 + cells1.set('F11', makeCell(ydoc, 2270)); 48 + cells1.set('G11', makeCell(ydoc, 3250)); 49 + ySheets.set('Sheet 1', sheet1); 50 + }); 51 + 52 + // Build Sheet 2 empty (summary lives here; see below) 53 + const sheet2 = new Y.Map(); 54 + const cells2 = new Y.Map(); 55 + ydoc.transact(() => { 56 + sheet2.set('name', 'Sheet 2'); 57 + sheet2.set('cells', cells2); 58 + ySheets.set('Sheet 2', sheet2); 59 + }); 60 + }); 61 + 62 + it("resolves ='Sheet 1'!F11 to the numeric value (not #REF!)", () => { 63 + const v = evaluateFormula("'Sheet 1'!F11"); 64 + expect(v).toBe(2270); 65 + }); 66 + 67 + it("resolves SUM over a cross-sheet range", () => { 68 + const v = evaluateFormula("SUM('Sheet 1'!A1:A3)"); 69 + expect(v).toBe(60); 70 + }); 71 + 72 + it("cross-sheet division works (percent calc pattern)", () => { 73 + // Typical summary-sheet formula: =F11/G11 but pulling from Sheet 1 74 + const v = evaluateFormula("'Sheet 1'!F11/'Sheet 1'!G11"); 75 + // 2270/3250 ≈ 0.6985 76 + expect(typeof v).toBe('number'); 77 + expect(Math.abs((v as number) - 0.6984615384615385)).toBeLessThan(1e-9); 78 + }); 79 + 80 + it("returns #REF! for a non-existent sheet name (not a crash)", () => { 81 + const v = evaluateFormula("'Nonexistent'!A1"); 82 + expect(v).toBe('#REF!'); 83 + }); 84 + 85 + it("cross-sheet ref to a formula cell evaluates that formula (not the raw f string)", () => { 86 + // Add a cell on Sheet 1 that's itself a formula: G1 = A1 * 2 87 + const sheet1 = ySheets.get('Sheet 1')!; 88 + const cells1 = sheet1.get('cells') as Y.Map<Y.Map<unknown>>; 89 + cells1.set('G1', makeCell(ydoc, '', 'A1*2')); 90 + clearEvalCache(); 91 + 92 + const v = evaluateFormula("'Sheet 1'!G1"); 93 + expect(v).toBe(20); // A1=10, *2 94 + }); 95 + });