Add option to Vite plugin to filter copied dependencies

[Pajn]

Pre-flight checklist

• I have read the contribution documentation for this project.
• I agree to follow the code of conduct that this project uses.
• I have searched the issue tracker for a feature request that matches the one I want to file, without success.

Problem description

The vite plugin copies all node modules over to the package, this creates a lot larger packages than needed. Both due to many packages being able to be bundled and some packages containing unneeded files.
The bundling can be reconfigured in the vite conf but the copying requires monkey-patching the vite plugin. Could options be added to the vite plugin to avoid that?

Proposed solution

I'm thinking like one option to override which dependencies should be copied and another that is just passed on to the fs-extra copy functions filter option. If this is something you are interested in I would be happy to send a pr for it.

Alternatives considered

I'm currently monkey-patching the plugin but this required copying most off the utils file which felt a lot messier than I'd prefer.

Additional information

No response

[caoxiemeihao]

Vite will only collect the dependencies into the App package, which in itself is not a problem.
If you want to exclude some deps, you should put they in devDependenices.

[joeyballentine]

I don't think that's entirely accurate. Why does vite even bundle this stuff anyway? Shouldn't it be tree shaking everything? When I use the forge vite config, it bundles 200mb of unneeded node modules. Webpack bundled 0mb of node modules (none at all). Why is this not possible with vite?

[zknicker]

FWIW, the electron-vite project recommends: "If a third-party npm-pkg can be built normally by Vite, we recommend placing it in devDependencies, which will reduce the size of the packaged App"

Ref: https://electron-vite.github.io/faq/dependencies.html

The thinking, I believe, comes from a similar line of reasoning that Septh (rollup-plugin-node-externals maintainer) spells out in Septh/rollup-plugin-node-externals#29. He says: "...dependencies are packages that are needed to run/use your package at run time. They should always be external because if Rollup bundled them there would be code duplication between the code of your package and the user's installation.".

Unless you disagree with that definition, it would suggest that "devDependencies" is actually the correct way to indicate that a package should be bundled (and tree shaked). Whereas a package in "dependencies" must be included wholesale in the packaged app.

The above seems like convention in the Vite community, and if that's the case, the changes proposed by this issue & PR probably aren't correct.

(As an aside, this suggests that most dependencies in an Electron app should be devDependencies, with exceptions being those that rely on C/C++, native libraries, etc. My general observation is that most maintainers of Electron projects don't follow this convention, and instead list plenty of "dependencies" just as you would if you were developing a run-of-the-mill library for NPM. Does that track?)

[joeyballentine]

FWIW, the electron-vite project recommends

There's a warning on that site saying it specifically pertains to using electron-builder, so it's not entirely relevant here. I don't really care what electron-builder is doing, because electron forge's webpack plugin does this correctly and that is what we should be comparing the vite plugin to (IMO)

there would be code duplication between the code of your package and the user's installation

That does not apply here. What you linked is specifically related to nodejs package development.

What the plugin is currently doing isn't sharing dependencies between some global dependency cache, but rather copying all the node modules to the project without any tree shaking (and as individual files) and configuring the code to link to that. This means that there is more code, both lines of code as well as actual files (many of which are actually unnecessary due to it not tree shaking). In simpler terms, a bunch of stuff I didn't use in my code is being included for no reason.

You are getting this confused with what happens during development. When I install a package from npm, I just want to install that package's code -- any dependency of that package should be downloaded separately, as well as any dependencies of the dependency and so on. This reduces code duplication because like you pointed out, it shares these (locally to the project) in the node_modules folder, so any two packages that depend on the same package with the same version can just share that code.

That is not what happens after you are done developing and want to bundle your project. Typically what happens is you want to build the smallest contained bundle possible. How do you do this? Well for starters, you wouldn't want to include any code you don't use. So what happens is instead of all these individual dependencies getting copied wholesale individually, it takes only the parts you use, and uses just that part in a way that can be shared among your codebase. But, unlike when it was a dependency, it is no longer a separate file, it just becomes part of your code.

This would obviously not be ok for package development, because then if you were to install someone's package that had done this process, you would get all their dependent code redownloaded even if you already had the dependencies it needed installed locally. Your bundler, for your project, can handle bundling everything together nicely.

A dev dependency is just a dependency you need for development purposes. Something like eslint (which checks your code for errors), prettier (a formatter), typescript (another code checker but also a compiler), vite, etc. Those things are not necessary for the code to actually run, just if you check the project out locally and you want to be able to do something like lint it the same way as the original devs. These dependencies would not be applicable to anything that depends on that package, as you are not developing that package, you are developing your own package. You'd never want dev dependencies to be bundled with your code. Luckily that's not what was going on here.

So that is what the rollup issue you linked is talking about. I now realize I spent a lot of time explaining this when it was already explained nicely in the linked issue, but I'm going to leave it as-is.

If you really want to get familiar with the difference, I'd recommend starting up a beginner nodejs project that uses a few simple dependencies (something like axios to make a GET request), and set up the project with eslint. That should (hopefully) make the difference more clear.

Anyway, the reason this is not applicable here is that I am not developing a package that someone is going to npm install later on. I am developing an entire program meant to be installed on someone's computer. So, my dependencies should not be treated the way they would be during development, but rather like they should for a production build.

As a side-note, both the electron-vite page and caox have mentioned just moving dependencies into dev dependencies and suggesting that vite would then correctly bundle everything. I don't know where that idea started, but that's just a total misuse of how dependencies are supposed to work. You can even refer to the official nodejs page about it to see this:

"dependencies": Packages required by your application in production.
"devDependencies": Packages that are only needed for local development and testing.

So no, I do not want to put packages that are required for my application in production in devDependencies.

TLDR: that rollup issue is irrelevant because I am coding an application and not an npm package, and you've misunderstood the difference between dependencies and devDependencies.

[joeyballentine]

Another way of thinking about it: imagine if you made a video game that used a single 10mb asset from a 100gb asset pack, and when you built your game it also included the other 99.99gb of random stuff you didn't use with the game. The person downloading the game would have around 100gb of unneeded extra garbage to download and extract, when it could have just taken the single asset out and ignored the rest.

That's what's going on here.

[zknicker]

As a side-note, both the electron-vite page and caox have mentioned just moving dependencies into dev dependencies and suggesting that vite would then correctly bundle everything. I don't know where that idea started, but that's just a total misuse of how dependencies are supposed to work.

100% agree here. Putting every single npm dependency in your app into "devDependencies" does sound really stupid.

TLDR: that rollup issue is irrelevant because I am coding an application and not an npm package, and you've misunderstood the difference between dependencies and devDependencies.

But I disagree here, because I think interpreting the definitions gets really hairy (unless we're actually talking about the "library" case, which I'd argue is the npm happy path).

Even npm's own definition of dependencies supports a worldview where every package in an Electron app should be a devDependency, precisely because it all gets bundled into an archive, and no end user of that Electron app needs to pre-install any dependencies (with exceptions like C/C++). It's not going to be possible to agree on a specific interpretation of the definitions of "dependencies" and "devDependencies", even in the narrow-scope of an Electron app.

I shared those 2 examples from the Electron-Vite docs, and the rollup-plugin-node-externals maintainer, to illustrate that for better or for worse, the Electron x Vite ecosystem does seem to currently advise extreme over-use of "devDependencies". And, it seems like Electron-Forge explicitly breaking away from that would be breaking convention somewhat from the rest of the community.

At any rate, thanks for putting so much effort into your contributions here. It's certainly more effort than I have or will put in, and as an Electron-Forge user, I appreciate it.

[joeyballentine]

Now that it's been explained like that, that does make more sense. However, I'd still argue in favor of what I've been arguing for.

Rationale: there's still a difference between something like eslint and something like react. If I don't install eslint (or any other of my dev dependencies), I can still build my app code with a suitable external tool. I would not be able to build my app without first installing react (or any other of my dependencies) because code would be missing that references that dependency. By mixing everything into devDepenencies, you are creating a separation of "stuff to copy vs stuff to not copy" instead of a "stuff you need for the app to work vs stuff you need for development". They aren't called copyDependencies and ignoreDependencies after all.

And I think especially when the default behavior of vite without the extra code coax wrote to copy the node modules folder, is to tree shake everything (including regular dependencies!!), well it just seems like that's how they intended it to be used (IMO).

Don't get me wrong, I appreciate the work coax has done on the vite plugin, I just disagree with how the implementation works currently and would prefer it to match the default functionality of both vite and the webpack plugin.

[zknicker]

Now that it's been explained like that, that does make more sense. However, I'd still argue in favor of what I've been arguing for.

Rationale: there's still a difference between something like eslint and something like react. If I don't install eslint (or any other of my dev dependencies), I can still build my app code with a suitable external tool. I would not be able to build my app without first installing react (or any other of my dependencies) because code would be missing that references that dependency. By mixing everything into devDepenencies, you are creating a separation of "stuff to copy vs stuff to not copy" instead of a "stuff you need for the app to work vs stuff you need for development". They aren't called copyDependencies and ignoreDependencies after all.

And I think especially when the default behavior of vite without the extra code coax wrote to copy the node modules folder, is to tree shake everything (including regular dependencies!!), well it just seems like that's how they intended it to be used (IMO).

Don't get me wrong, I appreciate the work coax has done on the vite plugin, I just disagree with how the implementation works currently and would prefer it to match the default functionality of both vite and the webpack plugin.

Sorry, I edited my comment pretty substantially a couple minutes ago, but none of the underlying message changed.

Totally agree with you on that overall. "devDependencies" is being absolutely abused not only by Electron apps, but a handful of other fullstack and frontend apps (e.g. even the Vue.js package.json lists every dependency as a "devDependencies").

[joeyballentine]

I think the even bigger nail in the coffin for this argument is that if I do bite my tongue and just move all my deps to my dev deps, eslint starts telling me (like it should) that I shouldn't do that:

[RunDevelopment]

To cherry-pick 2 quotes from before:

Vite will only collect the dependencies into the App package, which in itself is not a problem.

The above seems like convention in the Vite community, and if that's the case, the changes proposed by this issue & PR probably aren't correct.

I don't think that the problem here is that the current behavior is correct for vite, but that it is inconsistent with the webpack template and itself, and that it is undocumented.

Firstly, the templates for webpack and vite are presented as more or less equivalent choices, but the completely different handling of dependencies dependencies is not documented in forge docs and not at all obvious. This is just a neatly-placed foot gun for anyone that was attracted by vite because of its performance and isn't familiar with its philosophy.

Secondly, vite does not behave like this when bundling for websites. The obvious reason for that is that it doesn't make sense to ship raw node_modules for a website. My point here is that even users that used vite before might not have run into this issue before, as it only seems to affect non-web application bundles.

---

So frankly, I do not care which side of the philosophical argument dependencies vs devDependencies is correct, I only care about the problems this causes. Right now, vite's differing behavior is non-obvious, undocumented, and cannot be changed. To address those problems, I would suggest the following actions:

1. Document vite's behavior. The page for vite should have a big section that describes what vite does, why vite does it, and what problems this might cause.
2. Document how to resolve the problems caused by vite's behavior. This might be as simple as documenting the option required by this issue, or as unsatisfying as "set up your project differently to make vite happy."