-
-
Notifications
You must be signed in to change notification settings - Fork 267
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
build: use vite to bundle deps at main/preload package to avoid using broken electron-builder deps function. #903
Conversation
Hi there. Thanks for you contrib. Basically you propose a kinda revert d002d6f. From another issue
As example: axios. Also I remember some issues with electron-updater since it use semver that not bundling properly. |
Thanks for your quick reply and the summary. It seems I've totally missed the point that ssr is the mode for node env. I've found that Anyway, turn back to web mode like this PR is the wrong way. I'll close this PR now. Thanks for your time. |
I've tried using
It's odd that you can specify one external pattern and one anti-pattern since you'll wonder how they overwrite the other one once colliding. If I specify all node/electron externals in import nodeBuiltins from 'builtin-modules/static';
import electronBuiltins from 'electron-builtins';
export const config = {
// ...
build: {
ssr: true,
rollupOptions: {
// ...
external: (src) => {
const [name] = src.split('/');
const externalNames = [
...nodeBuiltins,
...nodeBuiltins.map((name) => `node:${name}`),
...electronBuiltins,
];
return externalNames.includes(name);
},
},
},
ssr: { noExternal: true },
// ...
} There are a few amendments to |
The remaining questions are now:
I'll setup some reproductions and see if those problem exist. |
This have no sence, since |
Update: For
Which tends to be their invalid key was not compatible with export const invalidPrivateKey = createPrivateKey(`-----BEGIN RSA PRIVATE KEY-----
MB0CAQACAQ8CAwEAAQIBAQIBBQIBAwIBAQIBAQIBAg==
-----END RSA PRIVATE KEY-----`); |
The new PR is going to be sent from branch fix-ssr。 Is there any other issues related to this PR? Let me know if any still exists, thanks! |
Pls. check also ipfs-http-client, usb, electron-devtools-installer, koa and @koa/router and semver |
Update:
|
I was able to perform a fix for the binary node issues.
I've test it in both watch mode and prod builds. Both work flawlessly. I'll apply the fix to previous repros to see if there is any further bugs. |
I've encountered some build bugs when applying the same methods for main package with rollup and vite. That issue was tracking at vitejs/vite#12012 |
Workaround: set |
Main purpose of this PR
Currently deps used at
main
/preload
package is put underdependencies
underpackage.json
and building of those packages is usingvite
'sssr
mode, which means deps will not be handled byvite
butelectron-builder
instead. This PR will usevite
to bundle deps atmain
/preload
package just as the same as therenderer
package to avoid using brokenelectron-builder
deps function.Why switching how deps get bundled?
Because deps bundle function is broken in
electron-builder
:You need to maintain good old
npm
node_modules
structure.electron-builder
internally uses develar/app-builder whosenode_modules
bundling function (nodeModuleCollector.go) only recognize good oldnpm
node_modules
structure.It doesn't recognize
pnpm
's soft-linkingnode_modules
structure. So if your deps used inmain
/preload
have their own sub-dependencies, it will NOT get copied byelectron-builder
resulting in runtime error because of missing deps (electron-userland/electron-builder#6289). The workaround currently is to tellpnpm
using hoist deps mode which is basically to forcepnpm
using good oldnpm
node_modules
structure and forgets about soft-linking packages. It's painful if you are usingpnpm
in monorepos to avoid files copying.I don't use
yarn
but I supposeyarn
's PnP structure will also been rejected. (electron-userland/electron-builder#6639)For monorepos, it has to maintain the same structure. This has been reported in electron-userland/electron-builder#6933 , electron-userland/electron-builder#7371 .
Those issues don't seem to be addressed by the maintainers.
Poor deps bundling
The deps bundling by
electron-builder
is poorly done:*.md
and*.d.ts
gets removed. Other files (e.g. sourcemaps, artifacts of different archs) will be copied as-is.vite
's tree-shaking. Unused code chunks, cjs+esm codes will be bundled.rxjs
which is optional devdep oftyped-emitter
, which is dep ofelectron-updater
, will be bundled. The dep search strategy ofelectron-builder
is just "grab all deps and optional deps" regardless of whether a optional dep is devdep.In my opinion, this is done poorly compared to
vite
/esbuild
. It will increase the whole production build size which electron apps are often criticized for.How this PR is going to fix it?
builtin-modules
andelectron-builtins
to determinenode
andelectron
builtin modules.build.rollupOptions.external
to externalize those modules. Optionally you can use vite-plugin-commonjs-externals if any cjs-esm interop issues is met.electron-builder
from copying anynode_module
files as now all deps are bundled byvite
.vue
todependencies
underpackage.json
(revert move vue to dev dependencies #901). Any further deps used by all three packages can also be added for distinguish which will be ships to builds clearly.Have this PR been tested?
I've tested the template and my own
rush
+pnpm
monorepo setup on Windows 10 which is flawless. Both watch and production build work as intended.I want to share package/code between
main
/preload
packageFor packages, if it's written in ESM and the code used in
main
/preload
is from different chunk, it will be fine due tovite
's tree-shaking feature. Otherwise you can transform this project's structure to utilizevite
's multi-entry feature to achieve the goal of reusing codes.