Skip to content

Commit 8de58f9

Browse files
authored
Fix an issue where overwriting a non-computed cell caused the Value of the formula cell is not computed error (#1569)
* Add unit tests reproducing the bug * Fix the error * Improve test utils log message for columnIndexToSheet function * Add jsdoc comments to Operations.ts file
1 parent 48647e5 commit 8de58f9

File tree

7 files changed

+117
-15
lines changed

7 files changed

+117
-15
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Fixed an issue where overwriting a non-computed cell caused the `Value of the formula cell is not computed` error. [#1194](https://github.com/handsontable/hyperformula/issues/1194)
13+
1014
## [3.1.0] - 2025-10-14
1115

1216
### Changed

src/Operations.ts

Lines changed: 70 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ import {
2222
ParsingErrorVertex,
2323
SheetMapping,
2424
SparseStrategy,
25-
ValueCellVertex
25+
ValueCellVertex,
2626
} from './DependencyGraph'
2727
import { FormulaVertex } from './DependencyGraph/FormulaCellVertex'
28-
import { RawAndParsedValue } from './DependencyGraph/ValueCellVertex'
28+
import { RawAndParsedValue, ValueCellVertexValue } from './DependencyGraph/ValueCellVertex'
2929
import { AddColumnsTransformer } from './dependencyTransformers/AddColumnsTransformer'
3030
import { AddRowsTransformer } from './dependencyTransformers/AddRowsTransformer'
3131
import { CleanOutOfScopeDependenciesTransformer } from './dependencyTransformers/CleanOutOfScopeDependenciesTransformer'
@@ -459,8 +459,6 @@ export class Operations {
459459

460460
/**
461461
* Restores a single cell.
462-
* @param {SimpleCellAddress} address
463-
* @param {ClipboardCell} clipboardCell
464462
*/
465463
public restoreCell(address: SimpleCellAddress, clipboardCell: ClipboardCell): void {
466464
switch (clipboardCell.type) {
@@ -570,7 +568,6 @@ export class Operations {
570568

571569
this.setFormulaToCell(address, size, parserResult)
572570
} catch (error) {
573-
574571
if (!(error as Error).message) {
575572
throw error
576573
}
@@ -598,45 +595,66 @@ export class Operations {
598595
}
599596
}
600597

598+
/**
599+
* Sets cell content to an instance of parsing error.
600+
* Creates a ParsingErrorVertex and updates the dependency graph and column search index.
601+
*/
601602
public setParsingErrorToCell(rawInput: string, errors: ParsingError[], address: SimpleCellAddress) {
602-
const oldValue = this.dependencyGraph.getCellValue(address)
603+
this.removeCellValueFromColumnSearch(address)
604+
603605
const vertex = new ParsingErrorVertex(errors, rawInput)
604606
const arrayChanges = this.dependencyGraph.setParsingErrorToCell(address, vertex)
605-
this.columnSearch.remove(getRawValue(oldValue), address)
607+
606608
this.columnSearch.applyChanges(arrayChanges.getChanges())
607609
this.changes.addAll(arrayChanges)
608610
this.changes.addChange(vertex.getCellValue(), address)
609611
}
610612

613+
/**
614+
* Sets cell content to a formula.
615+
* Creates a FormulaCellVertex and updates the dependency graph and column search index.
616+
*/
611617
public setFormulaToCell(address: SimpleCellAddress, size: ArraySize, {
612618
ast,
613619
hasVolatileFunction,
614620
hasStructuralChangeFunction,
615621
dependencies
616622
}: ParsingResult) {
617-
const oldValue = this.dependencyGraph.getCellValue(address)
623+
this.removeCellValueFromColumnSearch(address)
624+
618625
const arrayChanges = this.dependencyGraph.setFormulaToCell(address, ast, absolutizeDependencies(dependencies, address), size, hasVolatileFunction, hasStructuralChangeFunction)
619-
this.columnSearch.remove(getRawValue(oldValue), address)
626+
620627
this.columnSearch.applyChanges(arrayChanges.getChanges())
621628
this.changes.addAll(arrayChanges)
622629
}
623630

631+
/**
632+
* Sets cell content to a value.
633+
* Creates a ValueCellVertex and updates the dependency graph and column search index.
634+
*/
624635
public setValueToCell(value: RawAndParsedValue, address: SimpleCellAddress) {
625-
const oldValue = this.dependencyGraph.getCellValue(address)
636+
this.changeCellValueInColumnSearch(address, value.parsedValue)
637+
626638
const arrayChanges = this.dependencyGraph.setValueToCell(address, value)
627-
this.columnSearch.change(getRawValue(oldValue), getRawValue(value.parsedValue), address)
639+
628640
this.columnSearch.applyChanges(arrayChanges.getChanges().filter(change => !equalSimpleCellAddress(change.address, address)))
629641
this.changes.addAll(arrayChanges)
630642
this.changes.addChange(value.parsedValue, address)
631643
}
632644

645+
/**
646+
* Sets cell content to an empty value.
647+
* Creates an EmptyCellVertex and updates the dependency graph and column search index.
648+
*/
633649
public setCellEmpty(address: SimpleCellAddress) {
634650
if (this.dependencyGraph.isArrayInternalCell(address)) {
635651
return
636652
}
637-
const oldValue = this.dependencyGraph.getCellValue(address)
653+
654+
this.removeCellValueFromColumnSearch(address)
655+
638656
const arrayChanges = this.dependencyGraph.setCellEmpty(address)
639-
this.columnSearch.remove(getRawValue(oldValue), address)
657+
640658
this.columnSearch.applyChanges(arrayChanges.getChanges())
641659
this.changes.addAll(arrayChanges)
642660
this.changes.addChange(EmptyValue, address)
@@ -923,6 +941,45 @@ export class Operations {
923941
}
924942
return this.dependencyGraph.fetchCellOrCreateEmpty(expression.address).vertex
925943
}
944+
945+
/**
946+
* Removes a cell value from the columnSearch index.
947+
* Ignores the non-computed formula vertices.
948+
*/
949+
private removeCellValueFromColumnSearch(address: SimpleCellAddress): void {
950+
if (this.isNotComputed(address)) {
951+
return
952+
}
953+
954+
const oldValue = this.dependencyGraph.getCellValue(address)
955+
this.columnSearch.remove(getRawValue(oldValue), address)
956+
}
957+
958+
/**
959+
* Changes a cell value in the columnSearch index.
960+
* Ignores the non-computed formula vertices.
961+
*/
962+
private changeCellValueInColumnSearch(address: SimpleCellAddress, newValue: ValueCellVertexValue): void {
963+
if (this.isNotComputed(address)) {
964+
return
965+
}
966+
967+
const oldValue = this.dependencyGraph.getCellValue(address)
968+
this.columnSearch.change(getRawValue(oldValue), getRawValue(newValue), address)
969+
}
970+
971+
/**
972+
* Checks if the FormulaCellVertex or ArrayVertex at the given address is not computed.
973+
*/
974+
private isNotComputed(address: SimpleCellAddress): boolean {
975+
const vertex = this.dependencyGraph.getCell(address)
976+
977+
if (!vertex) {
978+
return false
979+
}
980+
981+
return 'isComputed' in vertex && !vertex.isComputed()
982+
}
926983
}
927984

928985
export function normalizeRemovedIndexes(indexes: ColumnRowIndex[]): ColumnRowIndex[] {

test/unit/column-index.spec.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ import {ColumnIndex} from '../../src/Lookup/ColumnIndex'
1414
import {NamedExpressions} from '../../src/NamedExpressions'
1515
import {ColumnsSpan, RowsSpan} from '../../src/Span'
1616
import {Statistics} from '../../src/statistics'
17-
import {adr, expectColumnIndexToMatchSheet} from './testUtils'
17+
import {adr, detailedError, expectColumnIndexToMatchSheet} from './testUtils'
18+
import { ErrorMessage } from '../../src/error-message'
1819

1920
function buildEmptyIndex(transformingService: LazilyTransformingAstService, config: Config, statistics: Statistics): ColumnIndex {
2021
const functionRegistry = new FunctionRegistry(config)
@@ -600,6 +601,8 @@ describe('Arrays', () => {
600601

601602
engine.setCellContents(adr('D1'), [['foo']])
602603

604+
expect(engine.getCellValue(adr('C1'))).toEqualError(detailedError(ErrorType.SPILL, ErrorMessage.NoSpaceForArrayResult))
605+
603606
expectColumnIndexToMatchSheet([
604607
[1, 2, null, 'foo']
605608
], engine)

test/unit/computation-suspension.spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,4 +206,22 @@ describe('Evaluation suspension', () => {
206206
expect(changes).toContainEqual(new ExportedCellChange(adr('B1'), 4))
207207
expect(changes).toContainEqual(new ExportedCellChange(adr('C1'), 5))
208208
})
209+
210+
it('allows to update cell content of a not computed formula cell (#1194)', () => {
211+
const hf = HyperFormula.buildFromArray([], {
212+
licenseKey: 'gpl-v3'
213+
})
214+
215+
hf.suspendEvaluation()
216+
hf.setCellContents(adr('A1'), '=42')
217+
hf.setCellContents(adr('A1'), 42)
218+
hf.setCellContents(adr('A1'), '=42')
219+
hf.setCellContents(adr('A1'), null)
220+
hf.setCellContents(adr('A1'), '=42')
221+
hf.setCellContents(adr('A1'), '==')
222+
hf.setCellContents(adr('A1'), '=42')
223+
hf.setCellContents(adr('A1'), '=42')
224+
hf.resumeEvaluation()
225+
expect(hf.getCellValue(adr('A1'))).toBe(42)
226+
})
209227
})

test/unit/rebuild.spec.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,14 @@ describe('Rebuilding engine', () => {
5353

5454
expect(rebuildEngineSpy).toHaveBeenCalled()
5555
})
56+
57+
it('doesn\'t throw after adding named expression (#1194)', () => {
58+
const hf = HyperFormula.buildFromArray([['=42']], {
59+
licenseKey: 'gpl-v3'
60+
})
61+
62+
63+
hf.addNamedExpression('ABC', '=Sheet1!$A$1')
64+
expect(() => hf.rebuildAndRecalculate()).not.toThrow()
65+
})
5666
})

test/unit/testUtils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ export function columnIndexToSheet(columnIndex: ColumnIndex, width: number, heig
213213
for (const row of index) {
214214
result[row] = result[row] ?? []
215215
if (result[row][col] !== undefined) {
216-
throw new Error('ColumnIndex ambiguity.')
216+
throw new Error(`ColumnIndex ambiguity. Expected ${JSON.stringify(result[row][col])}, but found ${JSON.stringify(value)} at row ${JSON.stringify(row)}, column ${JSON.stringify(col)}`)
217217
}
218218
result[row][col] = value
219219
}

test/unit/update-config.spec.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,4 +112,14 @@ describe('update config', () => {
112112
engine.updateConfig({ useColumnIndex: false, functionArgSeparator: ',', undoLimit: 20 })
113113
expect(rebuildEngineSpy).not.toHaveBeenCalled()
114114
})
115+
116+
it('doesn\'t throw after adding named expression (#1194)', () => {
117+
const hf = HyperFormula.buildFromArray([['=42']], {
118+
licenseKey: 'gpl-v3'
119+
})
120+
121+
122+
hf.addNamedExpression('ABC', '=Sheet1!$A$1')
123+
expect(() => hf.updateConfig({ maxRows: 101 })).not.toThrow()
124+
})
115125
})

0 commit comments

Comments
 (0)