Closed davej closed 7 months ago
Sooo I still haven't figured out the core issue with yarn, but it seems to be happening in the app-builder-bin (upstream npm app-builder project) and I'm not sure why.
The more annoying part for debugging this is that npm i
works perfectly fine for me.
Sooo I still haven't figured out the core issue with yarn, but it seems to be happening in the app-builder-bin (upstream npm app-builder project) and I'm not sure why.
The more annoying part for debugging this is that
npm i
works perfectly fine for me.
Anything I can do to help from my end Mike? Should I create an issue on the upstream project?
You can definitely create an issue in the upstream project, but unfortunately, I doubt it'll get attended to. Even worse, I can't upgrade electron-builder to its latest version anyhow due to changes in how it detects symlinks (it breaks our unit tests) (ref: https://github.com/electron-userland/electron-builder/pull/7774)
Let me debug further when I have more time apart from work and see what I can come back with. The current error looks familiar, and I think it may actually be an issue with the native dependency not having the right node-gyp configuration or it's doing some compilation outside the default install
flow
Actually, I think I figured it out. Looking at your dependencies package.json, it only no install
step, and that seems to be confusing app-builder
Added a blank "install": ""
entry in the dependency's package.json and yarn install
works perfectly fine
My recommendation is to test editing the dependencies package.json
Use this:
"scripts": {
"install": "",
"build": "node-gyp configure build",
...
}
These changes can be persisted in your repo via patch-package
if it works for you. Tested the distributable locally and your app runs, not sure what logic triggers the native binaries though, but they're indeed in the package
Thanks @mmaietta, I did some testing.
It seems that yarn run install
was a way of forcing yarn to run install scripts and rebuild the module in Yarn 1.
However, in Yarn 3, it seems like they dropped support for this:
I've tried to find documentation for this change in Yarn 3 but I can't find anything.
Added a blank
"install": ""
entry in the dependency's package.json andyarn install
works perfectly fine
I presume you mean yarn run install
. While this will suppress the error, it will also presumably prevent it from rebuilding?
One potential way to fix this would be to edit the rebuildUsingYarn
function so that it detects the version of Yarn and runs a command that would be compatible with Yarn 3+, perhaps yarn run node-gyp rebuild
? What do you think?
Ah, fantastic debugging work there. Yes, then definitely need to open an issue on that repo in order for any changes to be made there. There's a separate issue that still needs to be attended to to allow electron-builder to update if app-builder-bin were to be updated though (https://github.com/electron-userland/electron-builder/pull/7774).
I presume you mean yarn run install. While this will suppress the error, it will also presumably prevent it from rebuilding?
The .node
file seemed to be compiled AFAICT with the additional empty install
command added to your node module's package.json though, so I'm not sure if it's preventing it from rebuilding
@mmaietta I'm getting reports of this affecting more customers. Is there anything that I can work on that would help with this? Perhaps some way to easily switch between @electron/rebuild and app-builder by choosing a config option?
Off the top of my head, can you try removing the postinstall
install-app-deps
step and instead switch to using electron-rebuild directly in your electron-builder config? I had to do that with a previous project when macos universal builds originally were released.
beforeBuild: async (context) => {
const { appDir, electronVersion, arch } = context
await electronRebuild.rebuild({ buildPath: appDir, electronVersion, arch })
return false
},
nodeGypRebuild: false,
buildDependenciesFromSource: false,
@mmaietta Thanks! I tried this and it seemed to work but when I return false
from the beforeBuild
function then the app that is output had no node_modules
directory in the asar. If I return true
then I have a node_modules
directory but the app-builder rebuild step runs after the @electron/rebuild
rebuild step.
Any idea how what is going on?
This seems to be the problem: https://github.com/electron-userland/electron-builder/blob/538dd86bf52f0091dbb1120bdd30f56dfdbd5747/packages/app-builder-lib/src/platformPackager.ts#L389
If I hardcode this.info.areNodeModulesHandledExternally
to false
then the app builds correctly and includes node_modules
in the output directory.
- if (!this.info.isPrepackedAppAsar && !this.info.areNodeModulesHandledExternally) {
+ if (!this.info.isPrepackedAppAsar && !false) {
@mmaietta Is there a way to get this working without needing to run a patched version of app-builder-lib
?
I'll need to take another look at your test repo then to see what's going on. I remember in my old project I was actually bundling all of node_modules into the main.js
and renderer.js
via webpack, and then excluding node_modules
from the app.asar
. It was an interesting setup, but was a middle ground (at the time) from the project's previous haphazard implementation of building an electron app that wasn't using upstream electron (instead it was a custom distributable).
Looking at the code, in order to avoid the rebuild
step and instead go with installDependencies
, you need to not have node_modules
present? 🤔 Seems odd of a flow, but that would execute install
instead of app-builder's rebuild
https://github.com/electron-userland/electron-builder/blob/538dd86bf52f0091dbb1120bdd30f56dfdbd5747/packages/app-builder-lib/src/util/yarn.ts#L10-L31
Can you try giving that a shot? I suppose that would mean running the command via npx electron-builder <args>
Can you try giving that a shot? I suppose that would mean running the command via
npx electron-builder <args>
I'm not sure that I understand.
The solution that I settled on was to add a config option forceHandleNodeModules
which allows me to tell app-builder-lib that it should handle nodeModules
even when @electron/rebuild
is handling the rebuild process (and false
is returned from beforeBuild
.
The diff is basically this:
--- a/node_modules/app-builder-lib/out/packager.js
+++ b/node_modules/app-builder-lib/out/packager.js
@@ -115,7 +115,8 @@ class Packager {
constructor(options, cancellationToken = new builder_util_runtime_1.CancellationToken()) {
this.cancellationToken = cancellationToken;
this._metadata = null;
- this._nodeModulesHandledExternally = false;
+ this._forceHandleNodeModules = options.config.forceHandleNodeModules
+ this._nodeModulesHandledExternally = !this._forceHandleNodeModules || false;
this._isPrepackedAppAsar = false;
this._devMetadata = null;
this._configuration = null;
@@ -418,7 +419,8 @@ class Packager {
arch: builder_util_1.Arch[arch],
});
// If beforeBuild resolves to false, it means that handling node_modules is done outside of electron-builder.
- this._nodeModulesHandledExternally = !performDependenciesInstallOrRebuild;
+ this._nodeModulesHandledExternally = this._forceHandleNodeModules ? false : !performDependenciesInstallOrRebuild;
+
if (!performDependenciesInstallOrRebuild) {
return;
}
@mmaietta I can PR this if you like, but I'm not sure if it is desired?
I'm not too big a fan of adding another property to the config, but am open to the idea if we're unable to make progress with https://github.com/develar/app-builder/issues/103. Tbh, I really think we need to fix the issue at its core in that project
Either that or figure out a way to decouple from app-builder-bin dependency, or at least the node-rebuild+prebuild-cli+asar packaging steps. I've tried many a time with no success though. The asar implementation in electron-builder is impressively complex to me and I haven't been able to get symlinks working correctly or the asar unpack step.
I'm not too big a fan of adding another property to the config, but am open to the idea if we're unable to make progress with develar/app-builder#103. Tbh, I really think we need to fix the issue at its core in that project
I just decided to patch this during postinstall for my purposes. I think I agree with you about wanting to avoid adding a new property to config. I guess I don't have context about why node_modules
are handled externally to be able to understand the "correct" fix.
Regarding handling of node_modules externally to determine production dependencies, I haven't figured out a way to handle asar unpack with symlinks in conjunction with electron-builder's files
exclusion configuration.
I've previously read through electron-forge and electron/asar code to attempt to recreate electron-builder's implementation and IIRC, electron-forge doesn't copy dependencies, rather it reinstalls the production-only dependencies based on a copy of the package.json. I'll need to re-read through their latest implementation though and can give it another go on translating electron-builder's custom asar implementation to use the official package.
Got it, thanks for the context Mike! Even if the upstream Yarn bug gets fixed then I imagine that some users might still like to use @electron/rebuild
to rebuild the deps (instead of app-builder
). I think that is currently impossible because node_modules
are removed when beforeBuild
returns false
. Is there a workaround that I'm missing for this? Otherwise, I can't think of a solution other than:
enum
/string
in beforeBuild
that would signify that electron-builder would handle the node_modules
but would not use app-builder to rebuild.
beforeBuild: async (context) => {
const { appDir, electronVersion, arch } = context
await electronRebuild.rebuild({ buildPath: appDir, electronVersion, arch })
return "DISABLE_REBUILD_BUT_HANDLE_NODE_MODULES_INTERNALLY"
},
Hmmm, that would be a breaking API change for users, so I'm cautious in doing that.
The problem with @electron/rebuild is that it doesn't handle prebuilds, which was why we reverted back to app-builder, so I've yet to figure out an npm package replacement for handling the case of prebuilds
Maybe I can add a process.env.OVERRIDE_xxx
to the logic so that you can explicitly override the functionality for now at least. The goal would be to remove it though in favor of a fully JS solution of electron/rebuild, prebuild-cli, and asar packaging. I also intend to do the same for DMG and Snap, but don't know if I can do the same for AppImage. (This will be a hefty undertaking though that I don't have time to do so right now)
Hmmm, that would be a breaking API change for users, so I'm cautious in doing that.
I presume you're referring to the beforeBuild
return value? It might technically be a breaking change but it won't break any existing behavior because nobody will be using that string.
Maybe I can add a
process.env.OVERRIDE_xxx
to the logic so that you can explicitly override the functionality for now at least.
Sounds good to me but just to be clear, I have manually patched the library so there is no pressure from my end to resolve this. 😊
Alright, I think it's time to migrate to electron/rebuild
. Going to (re)try it again in a set of alpha builds. @davej would you be willing to test it out in your specific project once I can get it integrated? That should resolve your yarn 3 issue since it won't leverage app-builder any longer for native dependencies
@davej give the latest alpha version (25.0.0-alpha.1) electron-builder a shot - main addition is electron/rebuild implementation (full release notes posted). New configuration option:
/**
* Use `legacy` app-builder binary for installing native dependencies, or @electron/rebuild in `sequential` or `parallel` compilation modes.
* @default legacy
*/
readonly nativeRebuilder?: "legacy" | "sequential" | "parallel" | null
When moving from Yarn 1 to Yarn 3 there is now an issue when rebuilding a module. This issue was not a problem with Yarn 1. I did some debugging and rebuild seems to be working fine with @electron/rebuild. I can even get it working if I use
v24.6.0
of app-builder-lib (when it used @electron/rebuild). But it seems to be broken when using Electron Builder's rebuild functionality.Here is a repro that you can clone: https://github.com/davej/electron-boilerplate-yarn-3-issue
To recreate simple clone the repo and run
yarn
. After install has finished then you should see output that ends like this:If you click into the logs then you will see something like this: