Skip to content

Commit 7840290

Browse files
committed
Different approach - no transformer
1 parent 01cd268 commit 7840290

File tree

6 files changed

+34
-145
lines changed

6 files changed

+34
-145
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
99

1010
### Fixed
1111

12-
- Fixed an issue where cells where not recalculated after adding new sheet. [#1116](https://github.com/handsontable/hyperformula/issues/1116)
12+
- Fixed an issue where cells were not recalculated after adding new sheet. [#1116](https://github.com/handsontable/hyperformula/issues/1116)
1313

1414
## [3.1.0] - 2025-10-14
1515

src/DependencyGraph/DependencyGraph.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import {SimpleRangeValue} from '../SimpleRangeValue'
2424
import {LazilyTransformingAstService} from '../LazilyTransformingAstService'
2525
import {Maybe} from '../Maybe'
2626
import {NamedExpressions} from '../NamedExpressions'
27-
import {Ast, collectDependencies, NamedExpressionDependency} from '../parser'
27+
import {Ast, AstNodeType, collectDependencies, NamedExpressionDependency} from '../parser'
2828
import {ColumnsSpan, RowsSpan, Span} from '../Span'
2929
import {Statistics, StatType} from '../statistics'
3030
import {
@@ -308,6 +308,10 @@ export class DependencyGraph {
308308
})
309309
}
310310

311+
public addSheet(sheetName: string): void {
312+
// TODO
313+
}
314+
311315
public clearSheet(sheetId: number) {
312316
const arrays: Set<ArrayVertex> = new Set()
313317
for (const [address, vertex] of this.addressMapping.sheetEntries(sheetId)) {
@@ -523,6 +527,14 @@ export class DependencyGraph {
523527
}
524528
}
525529

530+
public* formulaNodes(): IterableIterator<FormulaVertex> {
531+
for (const vertex of this.graph.getNodes()) {
532+
if (vertex instanceof FormulaVertex) {
533+
yield vertex
534+
}
535+
}
536+
}
537+
526538
public* entriesFromRowsSpan(rowsSpan: RowsSpan): IterableIterator<[SimpleCellAddress, CellVertex]> {
527539
yield* this.addressMapping.entriesFromRowsSpan(rowsSpan)
528540
}

src/HyperFormula.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2635,7 +2635,7 @@ export class HyperFormula implements TypedEmitter {
26352635
validateArgToType(sheetName, 'string', 'sheetName')
26362636
}
26372637
const addedSheetName = this._crudOperations.addSheet(sheetName)
2638-
this.recomputeIfDependencyGraphNeedsIt() //?
2638+
this.recomputeIfDependencyGraphNeedsIt()
26392639
this._emitter.emit(Events.SheetAdded, addedSheetName)
26402640
return addedSheetName
26412641
}

src/Operations.ts

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import { FormulaVertex } from './DependencyGraph/FormulaCellVertex'
2828
import { RawAndParsedValue } from './DependencyGraph/ValueCellVertex'
2929
import { AddColumnsTransformer } from './dependencyTransformers/AddColumnsTransformer'
3030
import { AddRowsTransformer } from './dependencyTransformers/AddRowsTransformer'
31-
import { AddSheetTransformer } from './dependencyTransformers/AddSheetTransformer'
3231
import { CleanOutOfScopeDependenciesTransformer } from './dependencyTransformers/CleanOutOfScopeDependenciesTransformer'
3332
import { MoveCellsTransformer } from './dependencyTransformers/MoveCellsTransformer'
3433
import { RemoveColumnsTransformer } from './dependencyTransformers/RemoveColumnsTransformer'
@@ -52,7 +51,7 @@ import {
5251
NamedExpressions
5352
} from './NamedExpressions'
5453
import { NamedExpressionDependency, ParserWithCaching, ParsingErrorType, RelativeDependency } from './parser'
55-
import { ParsingError } from './parser/Ast'
54+
import { AstNodeType, ParsingError } from './parser/Ast'
5655
import { ParsingResult } from './parser/ParserWithCaching'
5756
import { findBoundaries, Sheet } from './Sheet'
5857
import { ColumnsSpan, RowsSpan } from './Span'
@@ -251,38 +250,27 @@ export class Operations {
251250
const sheet: Sheet = []
252251
this.dependencyGraph.addressMapping.autoAddSheet(sheetId, findBoundaries(sheet))
253252
const addedSheetName = this.sheetMapping.fetchDisplayName(sheetId)
253+
const quotedSheetName = `'${addedSheetName.replace(/'/g, "''")}'`
254254

255-
// Rebuild formulas that reference the newly added sheet
256-
let version = 0
255+
// TODO: move this code to dependencyGraph.addSheet()
257256
this.stats.measure(StatType.TRANSFORM_ASTS, () => {
258-
const transformation = new AddSheetTransformer(sheetId, addedSheetName)
259-
transformation.performEagerTransformations(this.dependencyGraph, this.parser)
260-
261-
// TODO: move this logic to the transformer
262-
// Rebuild non-array formulas that were transformed
263-
for (const vertex of this.dependencyGraph.graph.getNodes()) {
264-
if (vertex instanceof FormulaCellVertex) {
265-
const originalAst = vertex.getFormula(this.lazilyTransformingAstService)
266-
const address = vertex.getAddress(this.lazilyTransformingAstService)
267-
const [transformedAst] = transformation.transformSingleAst(originalAst, address)
268-
269-
// Check if the AST actually changed
270-
const originalHash = this.parser.computeHashFromAst(originalAst)
271-
const transformedHash = this.parser.computeHashFromAst(transformedAst)
272-
273-
if (originalHash !== transformedHash) {
274-
// The formula was transformed (ERROR_WITH_RAW_INPUT -> valid reference)
275-
// We need to rebuild it with proper dependencies using setFormulaToCellFromCache
276-
this.parser.rememberNewAst(transformedAst)
277-
this.setFormulaToCellFromCache(transformedHash, address)
278-
}
257+
for (const node of this.dependencyGraph.formulaNodes()) {
258+
const ast = node.getFormula(this.dependencyGraph.lazilyTransformingAstService)
259+
if (ast.type === AstNodeType.ERROR_WITH_RAW_INPUT && this.containsSheetName(ast.rawInput, addedSheetName, quotedSheetName)) {
260+
const address = node.getAddress(this.dependencyGraph.lazilyTransformingAstService)
261+
this.setCellContent(address, `=${ast.rawInput}`) // or maybe take just the inner part of this function body
279262
}
280263
}
281-
282-
version = this.lazilyTransformingAstService.addTransformation(transformation)
283264
})
284265

285-
return { addedSheetName, version }
266+
return { addedSheetName, version: 0 } // TODO: calculate version
267+
}
268+
269+
private containsSheetName(rawInput: string, unquotedSheetNameLowercase: string, quotedSheetNameLowercase: string): boolean {
270+
const lowerInput = rawInput.toLowerCase()
271+
272+
return lowerInput.includes(`${unquotedSheetNameLowercase}!`)
273+
|| lowerInput.includes(`${quotedSheetNameLowercase}!`)
286274
}
287275

288276
public renameSheet(sheetId: number, newName: string) {

src/dependencyTransformers/AddSheetTransformer.ts

Lines changed: 0 additions & 110 deletions
This file was deleted.

test/unit/cruds/adding-sheet.spec.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,9 @@ describe('add sheet to engine', () => {
117117
expect(engine.getCellValue(adr('A1', engine.getSheetId(table1Name)))).toEqualError(detailedError(ErrorType.REF))
118118

119119
engine.addSheet(table2Name)
120+
121+
expect(engine.getCellValue(adr('A1', engine.getSheetId(table1Name)))).toBeNull()
122+
120123
engine.setCellContents(adr('A1', engine.getSheetId(table2Name)), 10)
121124

122125
expect(engine.getCellValue(adr('A1', engine.getSheetId(table1Name)))).toBe(10)
@@ -130,7 +133,6 @@ describe('add sheet to engine', () => {
130133
engine.batch(() => {
131134
engine.addSheet(table1Name)
132135
engine.setCellContents(adr('A1', engine.getSheetId(table1Name)), `='${table2Name}'!A1`)
133-
134136
engine.addSheet(table2Name)
135137
engine.setCellContents(adr('A1', engine.getSheetId(table2Name)), 10)
136138
})
@@ -144,13 +146,10 @@ describe('add sheet to engine', () => {
144146
const table2Name = 'table2'
145147

146148
engine.suspendEvaluation()
147-
148149
engine.addSheet(table1Name)
149150
engine.setCellContents(adr('A1', engine.getSheetId(table1Name)), `='${table2Name}'!A1`)
150-
151151
engine.addSheet(table2Name)
152152
engine.setCellContents(adr('A1', engine.getSheetId(table2Name)), 10)
153-
154153
engine.resumeEvaluation()
155154

156155
expect(engine.getCellValue(adr('A1', engine.getSheetId(table1Name)))).toBe(10)

0 commit comments

Comments
 (0)