Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ui): cleanup DOMs when dispose #1663

Merged
merged 2 commits into from
Mar 22, 2024
Merged

fix(ui): cleanup DOMs when dispose #1663

merged 2 commits into from
Mar 22, 2024

Conversation

hexf00
Copy link
Member

@hexf00 hexf00 commented Mar 22, 2024

When the univer is destroyed and then re created, there are residual doms that have not been cleaned up

A description of the proposed changes.

remove iframe and .univer-theme when univer dispose called

How to test them.

Replace the file \examples\src\sheets\main.ts with the following content and run setupUniver() multiple times in the console

import { LocaleType, LogLevel, Univer } from '@univerjs/core';
import { defaultTheme } from '@univerjs/design';
import { UniverDocsPlugin } from '@univerjs/docs';
import { UniverDocsUIPlugin } from '@univerjs/docs-ui';
import { UniverFormulaEnginePlugin } from '@univerjs/engine-formula';
import { UniverRenderEnginePlugin } from '@univerjs/engine-render';
import { UniverFindReplacePlugin } from '@univerjs/find-replace';
import type { IUniverRPCMainThreadConfig } from '@univerjs/rpc';
import { UniverRPCMainThreadPlugin } from '@univerjs/rpc';
import { UniverSheetsPlugin } from '@univerjs/sheets';
import { UniverSheetsFindReplacePlugin } from '@univerjs/sheets-find-replace';
import { UniverSheetsFormulaPlugin } from '@univerjs/sheets-formula';
import { UniverSheetsNumfmtPlugin } from '@univerjs/sheets-numfmt';
import { UniverSheetsUIPlugin } from '@univerjs/sheets-ui';
import { UniverSheetsZenEditorPlugin } from '@univerjs/sheets-zen-editor';
import { UniverUIPlugin } from '@univerjs/ui';

import { DEFAULT_WORKBOOK_DATA_DEMO } from '../data';
import { DebuggerPlugin } from '../plugins/debugger';
import { locales } from './locales';

const LOAD_LAZY_PLUGINS_TIMEOUT = 1_000;

declare global {
    interface Window {
        univer?: Univer;
        setupUniver?: () => void;
    }
}

const setupUniver = async () => {
    if (window.univer) {
        window.univer.dispose();
    }

    // univer
    const univer = new Univer({
        theme: defaultTheme,
        locale: LocaleType.ZH_CN,
        locales,
        logLevel: LogLevel.VERBOSE,
    });

// core plugins
    univer.registerPlugin(UniverDocsPlugin, {
        hasScroll: false,
    });
    univer.registerPlugin(UniverRenderEnginePlugin);
    univer.registerPlugin(UniverUIPlugin, {
        container: 'app',
        header: true,
        footer: true,
    });

    univer.registerPlugin(UniverDocsUIPlugin);

    univer.registerPlugin(UniverSheetsPlugin, {
        notExecuteFormula: true,
    });
    univer.registerPlugin(UniverSheetsUIPlugin);

// sheet feature plugins

    univer.registerPlugin(UniverSheetsNumfmtPlugin);
    univer.registerPlugin(DebuggerPlugin);
    univer.registerPlugin(UniverSheetsZenEditorPlugin);
    univer.registerPlugin(UniverFormulaEnginePlugin, {
        notExecuteFormula: true,
    });
    univer.registerPlugin(UniverSheetsFormulaPlugin);
    univer.registerPlugin(UniverRPCMainThreadPlugin, {
        workerURL: './worker.js',
    } as IUniverRPCMainThreadConfig);

// find replace
    univer.registerPlugin(UniverFindReplacePlugin);
    univer.registerPlugin(UniverSheetsFindReplacePlugin);

// create univer sheet instance
    univer.createUniverSheet(DEFAULT_WORKBOOK_DATA_DEMO);
    setTimeout(() => {
        import('./lazy').then((lazy) => {
            const plugins = lazy.default();
            plugins.forEach((p) => univer.registerPlugin(p[0], p[1]));
        });
    }, LOAD_LAZY_PLUGINS_TIMEOUT);

    window.univer = univer;
};
window.setupUniver = setupUniver;

setupUniver();

@univer-bot univer-bot bot added the qa:untested This PR is ready to be tested label Mar 22, 2024
Copy link

github-actions bot commented Mar 22, 2024

View Deployment

📑 Examples 📚 Storybook
🔗 Preview link 🔗 Preview link

Copy link

codecov bot commented Mar 22, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 32.95%. Comparing base (ff686e6) to head (272c564).

Files Patch % Lines
packages/ui/src/views/App.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #1663   +/-   ##
=======================================
  Coverage   32.95%   32.95%           
=======================================
  Files         946      946           
  Lines       52819    52823    +4     
  Branches    11050    11050           
=======================================
+ Hits        17405    17408    +3     
- Misses      35414    35415    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hexf00 hexf00 changed the title fix: cleanup DOMs on dispose fix: cleanup DOMs when dispose Mar 22, 2024
@hexf00 hexf00 removed the qa:untested This PR is ready to be tested label Mar 22, 2024
@hexf00 hexf00 changed the title fix: cleanup DOMs when dispose fix(ui): cleanup DOMs when dispose Mar 22, 2024
Copy link
Member

@wzhudev wzhudev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some code can be simplified.

@hexf00 hexf00 force-pushed the fix/cleanup-doms-on-dispose branch from bc4e5bb to 7f06990 Compare March 22, 2024 12:29
hexf00 and others added 2 commits March 22, 2024 21:11
@hexf00 hexf00 force-pushed the fix/cleanup-doms-on-dispose branch from 7f06990 to 272c564 Compare March 22, 2024 13:11
@wzhudev wzhudev merged commit b81ba1a into dev Mar 22, 2024
8 checks passed
@wzhudev wzhudev deleted the fix/cleanup-doms-on-dispose branch March 22, 2024 13:16
Jocs pushed a commit that referenced this pull request Mar 23, 2024
* fix: cleanup DOMs on dispose

* refactor(sheet-ui): simplified dispose

Co-authored-by: Wenzhao Hu <wzhudev@gmail.com>

---------

Co-authored-by: Wenzhao Hu <wzhudev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants