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

[DataGridPremium] Use web worker for Excel export #7770

Merged
merged 20 commits into from
Feb 23, 2023

Conversation

m4theushw
Copy link
Member

@m4theushw m4theushw commented Jan 30, 2023

@m4theushw m4theushw added the component: data grid This is the name of the generic UI component, not the React module! label Jan 30, 2023
@mui-bot
Copy link

mui-bot commented Jan 30, 2023

Messages
📖 Netlify deploy preview: https://deploy-preview-7770--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 784.3 1,407 784.3 1,031.18 242.363
Sort 100k rows ms 773.3 1,287.6 1,287.6 1,048.42 185.065
Select 100k rows ms 218.7 408 225.1 291.5 85.647
Deselect 100k rows ms 202 301.1 260.5 250.14 38.577

Generated by 🚫 dangerJS against 0ab1511

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 6, 2023
@github-actions
Copy link

github-actions bot commented Feb 6, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 8, 2023
@m4theushw m4theushw marked this pull request as ready for review February 9, 2023 16:16
### Using a web worker

:::warning
This feature only works with `@mui/styled-engine` v5.11.8 or newer.
Copy link
Member Author

Choose a reason for hiding this comment

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

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 10, 2023
@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

:::

Instead of generating the Excel file locally, you can delegate the task to a web worker.
This method reduces the amount of time that the main thread remains frozen, allowing to interact with the grid while the data is exported in background.
Copy link
Member

Choose a reason for hiding this comment

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

allowing to interact with the grid

May the interaction interfere with the exported data?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it will be a problem. While the UI is frozen (during serialization) the user can't interact with it, but once interaction is possible is because serialization is finished and the worker is preparing the Excel file.

docs/data/data-grid/export/export.md Outdated Show resolved Hide resolved
docs/data/data-grid/export/export.md Outdated Show resolved Hide resolved
docs/data/data-grid/export/export.md Outdated Show resolved Hide resolved
Comment on lines +292 to +295
// Creates a temp worksheet to obtain the column letters
const excelJS = await getExcelJs();
const workbook: Excel.Workbook = new excelJS.Workbook();
const worksheet = workbook.addWorksheet('Sheet1');
Copy link
Member

Choose a reason for hiding this comment

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

I don't get why is it needed, but it should not be a problem

Copy link
Member Author

Choose a reason for hiding this comment

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

I need to get the column letters to be able to serialize the value options. The only why I found to get this without duplicating the code was to create a temp worksheet.

Copy link
Member

@cherniavskii cherniavskii left a comment

Choose a reason for hiding this comment

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

Great work! 🚀

docs/data/data-grid/export/export.md Outdated Show resolved Hide resolved
docs/data/data-grid/export/export.md Outdated Show resolved Hide resolved
docs/data/data-grid/export/export.md Outdated Show resolved Hide resolved
docs/worker.ts Show resolved Hide resolved
) {
// eslint-disable-next-line no-restricted-globals
addEventListener('message', async (event: MessageEvent<ExcelExportInitEvent>) => {
const {
Copy link
Member

Choose a reason for hiding this comment

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

Could we refactor buildExcel to use the serialized data instead of apiRef and reuse buildExcel in setupExcelExportWebWorker?

function setupExcelExportWebWorker() {
  addEventListener('message', async (event: MessageEvent<ExcelExportInitEvent>) => {
    const workbook = await buildExcel(event.data);
    postMessage(await workbook.xlsx.writeBuffer());
  }
}

Copy link
Member Author

Choose a reason for hiding this comment

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

This can be done in a follow-up, I'll open an issue. We need to rework the interfaces for buildExcel to accept the same params that are passed when the worker is used..

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me 👍

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 22, 2023
@m4theushw m4theushw merged commit 18abe20 into mui:next Feb 23, 2023
@m4theushw m4theushw deleted the excel-export-with-web-worker branch February 23, 2023 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data grid] Excel export of a big amount of rows hangs up the page, any suggestions?
4 participants