amcharts / amcharts4

The most advanced amCharts charting library for JavaScript and TypeScript apps.
https://www.amcharts.com/
1.16k stars 322 forks source link

pdfmake doesn't work with treeshaking... Massive dependency. #813

Closed MarkLyck closed 5 years ago

MarkLyck commented 5 years ago

I don't need any features that creates a pdf, or export charts anywhere in my application. Yet this "pdfmake" library ends up in my final production bundle, taking up a whopping 400kb GZIPPED!

screen shot 2019-01-12 at 5 49 13 pm

When you don't use the export function anywhere. I would expect pdfmake to not end up in the final bundle.

PS. what is vfs_fonts.js for?

5 of the biggest chunks of my production bundle is amcharts. Only two of them actually looks like it's the chart library itself? What is xlsx used for??? What on earth do I need an excel file writer/parser for when all my data is coming in as JSON?

martynasma commented 5 years ago

Hi there,

Please refer to issue #569 as well as solutions proposed there.

Pauan commented 5 years ago

To be clear, pdfmake, xlsx, canvg, and vfs_fonts are already optional: they will only be loaded when they are actually needed.

Yes they appear in the bundle directory, because they are dynamically loaded at runtime, so they cannot be compiled away at build time.

However, assuming you don't actually use the exporting feature, the files will not be loaded. You can use the dev tools in the browser (specifically the "Network" tab) to verify this.

So they do not increase the file download size, and they do not use up any network bandwidth. Essentially, they do nothing, and are harmless.

martynasma commented 5 years ago

I'm closing this, but please feel free to post any follow up questions you may have.

cozuya commented 3 years ago

I'm sure you receive enough flak about this but its not "harmless" to have massive dependencies like this, they still need to be downloaded all the time and can be misconstrued by users analyzing dependencies. Please reconsider making these here by default or provide an option for a "skinny" version.

Pauan commented 3 years ago

@cozuya Unfortunately there is no way to make dependencies optional in npm, so the only way we could provide a "skinny" version is to create a completely separate npm package. We would love to make it optional if we could, but we can't.

As for the download size... well, it's quite normal for npm dependencies to be many hundreds of megabytes in size. This is also unfortunate, but it's just a part of the npm ecosystem that we can't fix. For example, Angular alone is 288 MB, and React is 160 MB.

To be clear, at runtime there is no download cost, the only download cost is during development (when you use npm install to install the dependencies).

cozuya commented 3 years ago

I understand but users of my stuff looks at something like webpack-build-analyzer and see 5mb of JS there and panic. I can't say I've seen a similar situation in any other products. 🤷

Tofandel commented 1 year ago

Unfortunately there is no way to make dependencies optional in npm

That's not really true, you can make it an optionalDepency which then requires the user to add it himself to his list of dependencies (that's a big breaking change)

What could be done otherwise, is simply to make a separate package for the export feature, this would be the one with the dependency on xlsx, pdfmake etc

Then if people want the export feature it needs to be installed, still a breaking change but much more developer friendly

Pauan commented 1 year ago

That's not really true, you can make it an optionalDepency which then requires the user to add it himself to his list of dependencies (that's a big breaking change)

You are misunderstanding how optionalDependencies works. Despite it's name, it is not optional. optionalDependencies will always be installed, just the same as dependencies.

So if it's not optional, then what is it's purpose? The purpose is that if the dependency errors during installation then npm will skip it:

https://docs.npmjs.com/cli/v7/configuring-npm/package-json#optionaldependencies

It is an extremely niche use case, which is why almost nobody uses optionalDependencies, because it's largely useless.

Similarly, peerDependencies also will not work, because it's also not optional. The purpose of peerDependencies is to guarantee that multiple plugins will use the same version:

https://docs.npmjs.com/cli/v7/configuring-npm/package-json#peerdependencies

If you don't install a peer dependency, then you will get warnings (and errors in CI). They are not optional.

As I said before, npm does not provide any way to make a dependency optional.

What could be done otherwise, is simply to make a separate package for the export feature, this would be the one with the dependency on xlsx, pdfmake etc

Yes, we considered that, but decided against it. Because there is no runtime cost (the dependencies are dynamically loaded at runtime only as needed), there is basically no benefit to having a separate package, but there are significant maintenance costs to having a separate package.