Skip to content

Commit

Permalink
fix: clear timers when controller disposed (#4528)
Browse files Browse the repository at this point in the history
Co-authored-by: Wubo Cao <[email protected]>
  • Loading branch information
wubocao and Wubo Cao authored Feb 6, 2025
1 parent 4b3dd11 commit 6a871b0
Show file tree
Hide file tree
Showing 15 changed files with 17 additions and 13 deletions.
2 changes: 1 addition & 1 deletion e2e/disposing/disposing.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ test('no error on constructing and disposing sheet unit', async ({ page }) => {
errored = true;
});

await page.goto('http://localhost:3000/sheets/', { waitUntil: 'networkidle' });
await page.goto('http://localhost:3000/sheets/');
await page.evaluate(() => window.E2EControllerAPI.disposeCurrSheetUnit());
await page.evaluate(() => window.E2EControllerAPI.loadDefaultSheet());

Expand Down
19 changes: 10 additions & 9 deletions e2e/perf/scroll.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import type { Page } from '@playwright/test';
/* eslint-disable no-console */
import { chromium, expect, test } from '@playwright/test';
import { expect, test } from '@playwright/test';
import { sheetData as emptySheetData } from '../__testing__/emptysheet';
import { sheetData as freezeData } from '../__testing__/freezesheet';
import { sheetData as mergeCellData } from '../__testing__/mergecell';
Expand Down Expand Up @@ -135,14 +135,15 @@ async function measureFPS(page: Page, testDuration = 5, deltaX: number, deltaY:
const createTest = (title: string, sheetData: IJsonObject, minFpsValue: number, deltaX = 0, deltaY = 0) => {
// Default Size Of browser: 1280x720 pixels. And default DPR is 1.
test(title, async ({ page }) => {
let port = 3000;
if (!isCI) {
const browser = await chromium.launch({ headless: false }); // launch browser
page = await browser.newPage();
port = 3002;
}

await page.goto(`http://localhost:${port}/sheets/`);
// dev:e2e open localhost:3000, not 3002
// let port = 3000;
// if (!isCI) {
// const browser = await chromium.launch({ headless: false }); // launch browser
// page = await browser.newPage();
// port = 3002;
// }

await page.goto('http://localhost:3000/sheets/');
await page.waitForTimeout(2000);

const windowOfPage = await page.evaluateHandle('window');
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ export class TriggerCalculationController extends Disposable {

this._progress$.next(NilProgress);
this._progress$.complete();
// clear timer when disposed
clearTimeout(this._setTimeoutKey);
}

private _commandExecutedListener() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

/* eslint-disable max-lines-per-function */

import type { DocumentDataModel, ICellData, ICommandInfo, IDisposable, IDocumentBody, IDocumentData, IDocumentStyle, IStyleData, Nullable, Styles, UnitModel, Workbook } from '@univerjs/core';
import type { DocumentDataModel, ICellData, ICommandInfo, IDisposable, IDocumentBody, IDocumentData, IDocumentStyle, IStyleData, Nullable, Styles, Workbook } from '@univerjs/core';
import type { IRichTextEditingMutationParams } from '@univerjs/docs';
import type { IRenderContext, IRenderModule } from '@univerjs/engine-render';
import type { WorkbookSelectionModel } from '@univerjs/sheets';
Expand Down Expand Up @@ -158,9 +158,10 @@ export class EditingRenderController extends Disposable implements IRenderModule
this._commandExecutedListener(d);
this._initSkeletonListener(d);

this.disposeWithMe(this._univerInstanceService.unitDisposed$.subscribe((_unit: UnitModel) => {
// RenderManagerService._disposeItem will calls EditingRenderController.dispose before unitDisposed$.subscribe,so that the timer won't be cleared
d.add(() => {
clearTimeout(this._cursorTimeout);
}));
});

// FIXME: this problem is the same with slide. Should be fixed when refactoring editor.
this._cursorTimeout = setTimeout(() => {
Expand Down

0 comments on commit 6a871b0

Please sign in to comment.