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

kendo-grid -> kendo-angular-excel-export = not tree-shakable #2783

Closed
meriturva opened this issue Mar 2, 2020 · 19 comments
Closed

kendo-grid -> kendo-angular-excel-export = not tree-shakable #2783

meriturva opened this issue Mar 2, 2020 · 19 comments

Comments

@meriturva
Copy link

I'm starting to analyze my application using tool to analyze source-map or JSON stats.

Actually I see few packages on the bundle like "jszip" or "pako" that are dependencies of package @progress/kendo-angular-excel-export.

What i have to say is that I don't use any excel directive from the grid component (or other packages).
Just a short question? is kendo-angular-excel-export tree-shakable on your grid library?

Just see image:

image

@tsvetomir tsvetomir self-assigned this Mar 10, 2020
@tsvetomir
Copy link
Member

Thank you for reporting this problem.

The jszip and pako modules appear not to be tree-shakeable. This is due to side effects caused by code that is executed outside the exported functions.

We're researching the option to repackage them as ES modules. This would enable module concatenation as well.

@tsvetomir
Copy link
Member

The development builds now reference ES-module forks of the JSZip and Pako libraries that should be tree-shakeable. You can try it out by switching to the "dev" versions of the following packages:

dependencies: {
    "@progress/kendo-angular-excel-export": "dev",
    "@progress/kendo-drawing": "dev",
    "@progress/kendo-ooxml": "dev",
    ...
}

@tsvetomir
Copy link
Member

The fix is published in the latest versions of the packages.

You should see around 140kB improvement in the final bundle size for projects that do not use the PDF and Excel export.
Even when the export is in use, the bundle should be around 90kB smaller.

@meriturva
Copy link
Author

Thanks @tsvetomir I have analyzed again our project with the latest versions of all packages (still angular 9.x).
We have noted that jszip disappears correctly and we have little improvements on package size but what about kendo.angular.excel-export and pako packages?

Is there any possibility to have @progress/kendo-angular-excel-export tree-shakable? Actually we don't use any excel export function.

image

@tsvetomir
Copy link
Member

Modules that are not in use should be removed in production builds. Is this a map of the production bundle?

@meriturva
Copy link
Author

Yes is the map produce by command: ng build --prod --stats-json and the analyzed by:
webpack-bundle-analyzer .\\dist\\xxx\\stats-es2015.json

@tsvetomir tsvetomir reopened this Jun 26, 2020
@tsvetomir
Copy link
Member

I've confirmed that excel-export is getting included. The bundle size is at least somewhat affected by the fix for #2966 that removed the "side-effect free" flag on kendo-drawing. Reopening to investigate.

@tsvetomir
Copy link
Member

tsvetomir commented Aug 28, 2020

This issue turned out to have a really long tail 🐍 In addition to a few unnoticed side effects in Pako and JSZip we had to redo a good deal of the Drawing package.

The changes are now in the "dev" channel. You can give it another shot by installing the following packages:
npm install --save @progress/kendo-angular-excel-export@dev @progress/kendo-drawing@dev

Please note that the map still shows JSZip and other modules as part of the bundle. They most certainly are not, as confirmed by inspecting the bundle for some well-known strings from these packages.

To be certain, we've set up a few basic scenarios to confirm if tree-shaking is working properly. The results are summarized in the table below:

Platform Component Scenario Bundle size, kB New size, kB Change, kB Change, %
Angular v10 Chart No Export 421 361 -60 -14%
Grid No Export 489 397 -92 -19%
Grid Excel Export 525 453 -72 -14%
Grid Excel + PDF Export 537 521 -16 -3%
Angular v9 Chart No Export 425 365 -60 -14%
Grid No Export 489 397 -92 -19%
Grid Excel Export 525 453 -72 -14%
Grid Excel + PDF Export 537 521 -16 -3%
Angular v8 Chart No Export 525 469 -56 -11%
Grid No Export 461 369 -92 -20%
Grid Excel Export 501 429 -72 -14%
Grid Excel + PDF Export 513 493 -20 -4%

Test apps:

Edit: Fixed project download link

@meriturva
Copy link
Author

meriturva commented Aug 28, 2020

So thanks @tsvetomir

just to know about JSON stats produced by ng. Do you think that the presence of Pako andJSZip on stat file is an issue to report to angular cli team?

Secondly, did you test your project with new Angular 10 side effect flag? I'm curious if it decreases bundle size more.

Ps: file project link is broken

@tsvetomir
Copy link
Member

The closest issue I could find is webpack-contrib/webpack-bundle-analyzer#161. Based on the comments, it seems like source-map-explorer is more reliable in this case.

The report can be generated using the following steps:

ng build --prod --stats-json --source-map
sme dist/*/main* --html result.html

It does seem more accurate, see result.zip. The bundle indeed includes some code from JSZip, but it's an utility function that takes 2kB. We should be able to clear it out as well, but it's nowhere near the figures reported by Webpack Bundle Analyzer.

I couldn't find a reference to "sideEffects" in the Angular docs, apart from strict mode. I tried it in a separate project, but it doesn't seem to have an effect on the bundle size.

@meriturva
Copy link
Author

I couldn't find a reference to "sideEffects" in the Angular docs, apart from strict mode. I tried it in a separate project, but it doesn't seem to have an effect on the bundle size.

You are right, I mean strict mode that enables side effect flag on web pack configuration.

Anyway thanks for the fix I will try fix after stable release!

@tsvetomir
Copy link
Member

We'll be doing extended validation in more complex applications in the following week or so. I'll post updates here.

I'm curious to see what the savings look like in a "real world" application like yours. Let's hope they're at least close to what we see in the test cases.

@meriturva
Copy link
Author

Here we are developing a really big enterprise framework/application for the automation industry and also a few admin interfaces for some instruments/devices based on our framework. For the first case, nobody cares about bundle size, for device interface, we have limited storage space and it is crucial for us to reduce bytes.

What I would like to say is that the kendo grid is really full-featured but really big!

@tsvetomir
Copy link
Member

tsvetomir commented Sep 1, 2020

Some stats for the "main.js" bundle size for the applications in examples (dev release channel):

Component Project Bundle size, kB New size, kB Change, kB Change, %
Chart chart-websockets 441 389 -52 -12%
Grid grid-firebase 645 533 -112 -17%
Grid grid-graphql 617 497 -120 -19%
Grid grid-jsdo 745 637 -108 -14%
Grid grid-signalr 577 461 -116 -20%
Grid integration-i18n 521 413 -108 -21%
integration-jquery-partial 81 81 0 0%
integration-jquery 2389 2269 -120 -5%
integration-pwa-material 877 785 -92 -10%
integration-pwa 861 781 -80 -9%
PDF Export pdf-embedded-fonts 569 517 -52 -9%

@tsvetomir
Copy link
Member

The fix has been released in:

  • @progress/kendo-drawing v1.9.1 (changelog)
  • @progress/kendo-angular-excel-export v3.1.5 (changelog)

@meriturva
Copy link
Author

We have introduced on our CI (TeamCity) report tabs with source map explorer. We have for sure see improvements on bundle size during packages updates. Unfortunately, we have no time to check old builds right now (and make comparisons).

Main question is about pako package. It is still present on result.html, do you think it is just an issue of source map analyzer?
See:
image

@tsvetomir
Copy link
Member

Sorry for the delay. I don't see pako in a minimal project with the Grid and Chart included.

image

Attaching the sample project for reference.
ng10-bundle.zip

@meriturva
Copy link
Author

Thanks for your test @tsvetomir . Just to know pako is used only on Grid and Chart components?

@tsvetomir
Copy link
Member

It's a dependency of both of kendo-ooxml and the kendo-drawing package. So Pako could end up as an indirect dependency for any component that supports Excel or PDF export.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants