Fix issue where cells where not recalculated after adding, removing and renaming sheets (handsontable#1570)

* Add unit test that reproduce the issue
* Add changelog entry
* Don't omit end address in RemoveSheetTransformer
* Add npm script for npm audit
* Add more unit tests
* Fix linter in tests
* Make SheetMapping store reserved sheet names
* Adjust graphComparator
* Adjust evaluator to handle not-added sheets correctly
* Adjust tests in compute-hash-from-tokens.spec.ts
* Adjust tests in parser.spec.ts
* Fix numeric aggregation plugin
* Make addSheet operation update the relevant dependency
* Adjust tests in named-expressions.spec.ts
* Adjust tests in mitting-events.spec.ts
* Add complex range test scenarios
* Remove vertices only if they are not referenced by existing sheets
* Adjust tests in removing-sheet.spec.ts
* Refactor: create SheetReferenceRegistrar
* Remove sheet from AddressMapping if nothing else depends on it
* Remove unused functions
* Remove unused constructor parameters
* Adjust range vertices after rename sheet
* Update sheet strategy in AddressMapping if placeholder exists
* Improve docs in SheetMapping
* Refactor NumericAggregationPlugin
* Rename function to more descriptive name
* Refactor addressRepresentationConverters
* Refactor Graph.ts
* Refactor SheetReferenceRegistrar.ts
* Refactor RangeMapping
* Remove RemoveSheetTransformem which is not needed anymore
* Rename FormulaCellVertex -> ScalarFormulaVertex and ArrayVertex -> ArrayFormulaVertex
* Refactor DependencyGraph
* Refactor Operations
* Refactor DependencyGraph.removeSheet
* Make sure renameSheet handles dependency graph correctly
* Refactor DependencyGraph.mergeSheets()
* Refactor DependencyGraph
* Hande undoAddSheet and redoAddSheet
* Hande undoRemoveSheet and redoRemoveSheet
* Add tests for undo/redo renameSheet()
* Handle undo for renameSheet()
* Apply suggestions from agent review
* Configure eslint to allow null assertions in test files
* Remove placeholder sheets when not needed enymore
* Apply suggestions after agentic code review of the test files
* Test the cleanup of placeholder sheets

Author: sequba

.eslintrc.js

@@ -128,7 +128,7 @@ module.exports = {
     'jest/no-standalone-expect': 'warn',
     'jest/no-test-prefixes': 'off',
     'jest/prefer-to-be': 'warn',
-    'jest/prefer-to-have-length': 'warn',
+    'jest/prefer-to-have-length': 'off',
   },
   overrides: [
     {
@@ -143,5 +143,11 @@ module.exports = {
         'sort-keys': ['error', 'asc'],
       }
     },
+    {
+      files: ['**/*.spec.ts'],
+      rules: {
+        '@typescript-eslint/no-non-null-assertion': 'off',
+      }
+    }
   ],
 }

.github/workflows/audit.yml

@@ -32,4 +32,4 @@ jobs:
 
       - name: Run audit
         run: |
-          npm audit --omit='dev'
+          npm run audit

CHANGELOG.md

@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ### Fixed
 
+- Fixed an issue where cells were not recalculated after adding, removing and renaming sheets. [#1116](https://github.com/handsontable/hyperformula/issues/1116)
 - 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)
 
 ## [3.1.0] - 2025-10-14

package.json

@@ -78,7 +78,7 @@
     "test": "npm-run-all lint test:unit test:browser test:compatibility",
     "test:unit": "cross-env NODE_ICU_DATA=node_modules/full-icu jest",
     "test:watch": "cross-env NODE_ICU_DATA=node_modules/full-icu jest --watch",
-    "test:tdd": "cross-env NODE_ICU_DATA=node_modules/full-icu jest --watch named-expressions",
+    "test:tdd": "cross-env NODE_ICU_DATA=node_modules/full-icu jest --watch adding-sheet",
     "test:coverage": "npm run test:unit -- --coverage",
     "test:logMemory": "cross-env NODE_ICU_DATA=node_modules/full-icu jest --runInBand --logHeapUsage",
     "test:unit.ci": "cross-env NODE_ICU_DATA=node_modules/full-icu node --expose-gc ./node_modules/jest/bin/jest --forceExit",
@@ -93,6 +93,7 @@
     "benchmark:compare-benchmarks": "npm run tsnode test/performance/compare-benchmarks.ts",
     "lint": "eslint . --ext .js,.ts",
     "lint:fix": "eslint . --ext .js,.ts --fix",
+    "audit": "npm audit --omit=dev",
     "clean": "rimraf coverage/ commonjs/ dist/ es/ languages/ lib/ typings/ test-jasmine/",
     "compile": "tsc",
     "check:licenses": "license-checker --production --excludePackages=\"[email protected]\" --onlyAllow=\"MIT; Apache-2.0; BSD-3-Clause; BSD-2-Clause; ISC; BSD; Unlicense\"",

src/BuildEngineFactory.ts

@@ -87,13 +87,17 @@ export class BuildEngineFactory {
           throw new SheetSizeLimitExceededError()
         }
         const sheetId = sheetMapping.addSheet(sheetName)
-        addressMapping.autoAddSheet(sheetId, boundaries)
+        addressMapping.addSheetAndSetStrategyBasedOnBoundaries(sheetId, boundaries, { throwIfSheetAlreadyExists: true })
       }
     }
 
-    const parser = new ParserWithCaching(config, functionRegistry, sheetMapping.get)
+    const parser = new ParserWithCaching(
+      config,
+      functionRegistry,
+      dependencyGraph.sheetReferenceRegistrar.ensureSheetRegistered.bind(dependencyGraph.sheetReferenceRegistrar)
+    )
     lazilyTransformingAstService.parser = parser
-    const unparser = new Unparser(config, buildLexerConfig(config), sheetMapping.fetchDisplayName, namedExpressions)
+    const unparser = new Unparser(config, sheetMapping, namedExpressions)
     const dateTimeHelper = new DateTimeHelper(config)
     const numberLiteralHelper = new NumberLiteralHelper(config)
     const arithmeticHelper = new ArithmeticHelper(config, dateTimeHelper, numberLiteralHelper)
@@ -106,7 +110,7 @@ export class BuildEngineFactory {
     const clipboardOperations = new ClipboardOperations(config, dependencyGraph, operations)
     const crudOperations = new CrudOperations(config, operations, undoRedo, clipboardOperations, dependencyGraph, columnSearch, parser, cellContentParser, lazilyTransformingAstService, namedExpressions)
 
-    const exporter = new Exporter(config, namedExpressions, sheetMapping.fetchDisplayName, lazilyTransformingAstService)
+    const exporter = new Exporter(config, namedExpressions, sheetMapping, lazilyTransformingAstService)
     const serialization = new Serialization(dependencyGraph, unparser, exporter)
 
     const interpreter = new Interpreter(config, dependencyGraph, columnSearch, stats, arithmeticHelper, functionRegistry, namedExpressions, serialization, arraySizePredictor, dateTimeHelper)

src/Cell.ts

@@ -3,8 +3,8 @@
  * Copyright (c) 2025 Handsoncode. All rights reserved.
  */
 
-import {ArrayVertex, CellVertex, FormulaCellVertex, ParsingErrorVertex, ValueCellVertex} from './DependencyGraph'
-import {FormulaVertex} from './DependencyGraph/FormulaCellVertex'
+import {ArrayFormulaVertex, CellVertex, ScalarFormulaVertex, ParsingErrorVertex, ValueCellVertex} from './DependencyGraph'
+import {FormulaVertex} from './DependencyGraph/FormulaVertex'
 import {ErrorMessage} from './error-message'
 import {
   EmptyValue,
@@ -59,14 +59,14 @@ export enum CellType {
 }
 
 export const getCellType = (vertex: Maybe<CellVertex>, address: SimpleCellAddress): CellType => {
-  if (vertex instanceof ArrayVertex) {
+  if (vertex instanceof ArrayFormulaVertex) {
     if (vertex.isLeftCorner(address)) {
       return CellType.ARRAYFORMULA
     } else {
       return CellType.ARRAY
     }
   }
-  if (vertex instanceof FormulaCellVertex || vertex instanceof ParsingErrorVertex) {
+  if (vertex instanceof ScalarFormulaVertex || vertex instanceof ParsingErrorVertex) {
     return CellType.FORMULA
   }
   if (vertex instanceof ValueCellVertex) {
@@ -196,7 +196,12 @@ export interface SimpleCellAddress {
 }
 
 export const simpleCellAddress = (sheet: number, col: number, row: number): SimpleCellAddress => ({sheet, col, row})
-export const invalidSimpleCellAddress = (address: SimpleCellAddress): boolean => (address.col < 0 || address.row < 0)
+
+/**
+ * Checks if the column or row id is negative.
+ */
+export const isColOrRowInvalid = (address: SimpleCellAddress): boolean => (address.col < 0 || address.row < 0)
+
 export const movedSimpleCellAddress = (address: SimpleCellAddress, toSheet: number, toRight: number, toBottom: number): SimpleCellAddress => {
   return simpleCellAddress(toSheet, address.col + toRight, address.row + toBottom)
 }

src/ClipboardOperations.ts

@@ -4,7 +4,7 @@
  */
 
 import {AbsoluteCellRange} from './AbsoluteCellRange'
-import {invalidSimpleCellAddress, simpleCellAddress, SimpleCellAddress} from './Cell'
+import {isColOrRowInvalid, simpleCellAddress, SimpleCellAddress} from './Cell'
 import {RawCellContent} from './CellContentParser'
 import {Config} from './Config'
 import {DependencyGraph} from './DependencyGraph'
@@ -119,7 +119,7 @@ export class ClipboardOperations {
       return
     }
 
-    if (invalidSimpleCellAddress(destinationLeftCorner) ||
+    if (isColOrRowInvalid(destinationLeftCorner) ||
       !this.dependencyGraph.sheetMapping.hasSheetWithId(destinationLeftCorner.sheet)) {
       throw new InvalidArgumentsError('a valid target address.')
     }

src/CrudOperations.ts

@@ -4,7 +4,7 @@
  */
 
 import {AbsoluteCellRange} from './AbsoluteCellRange'
-import {invalidSimpleCellAddress, simpleCellAddress, SimpleCellAddress} from './Cell'
+import {isColOrRowInvalid, simpleCellAddress, SimpleCellAddress} from './Cell'
 import {CellContent, CellContentParser, RawCellContent} from './CellContentParser'
 import {ClipboardCell, ClipboardOperations} from './ClipboardOperations'
 import {Config} from './Config'
@@ -206,29 +206,35 @@ export class CrudOperations {
       this.ensureItIsPossibleToAddSheet(name)
     }
     this.undoRedo.clearRedoStack()
-    const addedSheetName = this.operations.addSheet(name)
-    this.undoRedo.saveOperation(new AddSheetUndoEntry(addedSheetName))
-    return addedSheetName
+    const { sheetName, sheetId } = this.operations.addSheet(name)
+    this.undoRedo.saveOperation(new AddSheetUndoEntry(sheetName, sheetId))
+    return sheetName
   }
 
   public removeSheet(sheetId: number): void {
     this.ensureScopeIdIsValid(sheetId)
     this.undoRedo.clearRedoStack()
     this.clipboardOperations.abortCut()
-    const originalName = this.sheetMapping.fetchDisplayName(sheetId)
+    const originalName = this.sheetMapping.getSheetNameOrThrowError(sheetId)
     const oldSheetContent = this.operations.getSheetClipboardCells(sheetId)
-    const {version, scopedNamedExpressions} = this.operations.removeSheet(sheetId)
-    this.undoRedo.saveOperation(new RemoveSheetUndoEntry(originalName, sheetId, oldSheetContent, scopedNamedExpressions, version))
+    const scopedNamedExpressions = this.operations.removeSheet(sheetId)
+    this.undoRedo.saveOperation(new RemoveSheetUndoEntry(originalName, sheetId, oldSheetContent, scopedNamedExpressions))
   }
 
   public renameSheet(sheetId: number, newName: string): Maybe<string> {
     this.ensureItIsPossibleToRenameSheet(sheetId, newName)
-    const oldName = this.operations.renameSheet(sheetId, newName)
-    if (oldName !== undefined) {
+    const { previousDisplayName, version, mergedPlaceholderSheetId } = this.operations.renameSheet(sheetId, newName)
+    if (previousDisplayName !== undefined) {
       this.undoRedo.clearRedoStack()
-      this.undoRedo.saveOperation(new RenameSheetUndoEntry(sheetId, oldName, newName))
+      this.undoRedo.saveOperation(new RenameSheetUndoEntry(
+        sheetId,
+        previousDisplayName,
+        newName,
+        version,
+        mergedPlaceholderSheetId,
+      ))
     }
-    return oldName
+    return previousDisplayName
   }
 
   public clearSheet(sheetId: number): void {
@@ -495,8 +501,8 @@ export class CrudOperations {
 
     if (
       !this.sheetMapping.hasSheetWithId(sheet)
-      || invalidSimpleCellAddress(sourceStart)
-      || invalidSimpleCellAddress(targetStart)
+      || isColOrRowInvalid(sourceStart)
+      || isColOrRowInvalid(targetStart)
       || !isPositiveInteger(numberOfRows)
       || (targetRow <= startRow + numberOfRows && targetRow >= startRow)
     ) {
@@ -522,8 +528,8 @@ export class CrudOperations {
 
     if (
       !this.sheetMapping.hasSheetWithId(sheet)
-      || invalidSimpleCellAddress(sourceStart)
-      || invalidSimpleCellAddress(targetStart)
+      || isColOrRowInvalid(sourceStart)
+      || isColOrRowInvalid(targetStart)
       || !isPositiveInteger(numberOfColumns)
       || (targetColumn <= startColumn + numberOfColumns && targetColumn >= startColumn)
     ) {
@@ -552,14 +558,14 @@ export class CrudOperations {
       throw new NoSheetWithIdError(sheetId)
     }
 
-    const existingSheetId = this.sheetMapping.get(name)
+    const existingSheetId = this.sheetMapping.getSheetId(name)
     if (existingSheetId !== undefined && existingSheetId !== sheetId) {
       throw new SheetNameAlreadyTakenError(name)
     }
   }
 
   public ensureItIsPossibleToChangeContent(address: SimpleCellAddress): void {
-    if (invalidSimpleCellAddress(address)) {
+    if (isColOrRowInvalid(address)) {
       throw new InvalidAddressError(address)
     }
     if (!this.sheetMapping.hasSheetWithId(address.sheet)) {