Yarn 3 module rebuild failing

[davej]

• Electron-Builder Version: 24.9.1

• Node Version: 20.1.0

• Electron Version: 28.2.1
• Electron Type (current, beta, nightly): current

• Target: macOS

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:

➤ YN0009: │ electron-boilerplate@workspace:. couldn't be built successfully (exit code 1, logs can be found here: /private/var/folders/h1/9dzghzlx0xgcdw3j5yjk0hg00000gn/T/xfs-bc5c23f7/build.log)
➤ YN0000: └ Completed in 5s 484ms
➤ YN0000: Failed with errors in 44s 972ms

If you click into the logs then you will see something like this:

# This file contains the result of Yarn building a package (electron-boilerplate@workspace:.)
# Script name: postinstall

  • electron-builder  version=24.9.1
  • loaded configuration  file=package.json ("build" field)
  • rebuilding native dependencies  dependencies=@bugsnag/[email protected], @bugsnag/[email protected] platform=darwin arch=arm64
  • rebuilding native dependency  name=@bugsnag/plugin-electron-client-state-persistence version=7.22.2
  • rebuilding native dependency  name=@bugsnag/plugin-electron-app version=7.22.2
  ⨯ cannot execute  cause=exit status 1
                    out=Usage Error: Couldn't find a script named "install".

$ yarn run [--inspect] [--inspect-brk] [-T,--top-level] [-B,--binaries-only] <scriptName> ...

                    command=/private/var/folders/h1/9dzghzlx0xgcdw3j5yjk0hg00000gn/T/xfs-b1ba03f1/yarn run install
                    workingDir=/Users/davejeffery/code/test/electron-boilerplate-yarn-3-issue/node_modules/@bugsnag/plugin-electron-app

[mmaietta]

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.

[davej]

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?

[mmaietta]

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

[mmaietta]

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

1. node_modules/@bugsnag/plugin-electron-app
2. node_modules/@bugsnag/plugin-electron-client-state-persistence/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

[davej]

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 and yarn 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?

[mmaietta]

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

[davej]

@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?

[mmaietta]

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,

[davej]

@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?

[davej]

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?

[mmaietta]

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).

[mmaietta]

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>

[davej]

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?

[mmaietta]

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.

[davej]

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.

[mmaietta]

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.

[davej]

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:

1. Either adding a configuration property
2. Or, you could potentially return some 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"
  },

[mmaietta]

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)

[davej]

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. 😊

[mmaietta]

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

[mmaietta]

@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