[Case Study] Electron

[Mike-Dax]

First off, thank you so much for your hard work on this project. I'm very excited to have Yarn PnP integration in our projects.

What package is covered by this investigations?
Electron

Describe the goal of the investigation

Clear Electron integration, a lack of v8 crashes.

Investigation report

The current latest stable version of Electron has a v8 crash when booting with Yarn PnP enabled.

With nodeLinker: node-modules enabled, no such crash occurs.

When upgrading to electron v9 beta branch, no such crash occurs.

I've opened an issue at Electron's end electron/electron#22472

I have a minimum reproducible example here: https://github.com/Mike-Dax/electron-yarn-pnp-crash.

The tentative release date for v9 will be 2020-04-28

[arcanis]

Hey! I think @merceyz has made some experiments, maybe they'll have more insight to share!

If I remember correctly, the main issue is that part of Electron is running within a browser context where the libzip layer cannot be instantiated synchronously on the main thread due to arbitrary Chrome restrictions - preventing to load the other files 🙁

There's also something about electron being a global module and PnP not being aware of that, but it might not be a problem anymore after #848.

[Mike-Dax]

You seem to be correct regarding libzip being the culprit, at least for this crash. It's confusing that v9 of Electron (on Chrome 82) doesn't seem to have this problem (Chrome 80 does).

Most (many?) Electron apps will have Webpack build the application before passing it to Electron to run, perhaps running Electron explicitly not with yarn, disabling the PnP integration for the actual Electron renderer / main will make this smoother.

Any tools that copy the process.env and spawn the electron process with it will need to delete the --require in NODE_OPTIONS.

Our internal toolchain handles that spawn and we can delete the require call, however users that boot Electron via yarn run will have it automatically injected.

Would it be possible to add a flag / environment variable to yarn run so that it doesn't automatically inject the require flag? Things might get a bit messy calling yarn run electron ./main.js --no-require -- --enable-logging.

Regarding bundling, there might need to be a custom webpack plugin that takes the externals entries in Webpack and 'unplugs' those specific dependencies into the node_modules folder, a partial nodeLinker: node-modules solution? Or perhaps a custom hybrid linker that supports 'unplugging' some modules into the node_modules folder whilst using the regular PnP linker for everything else.

My high level goal here is essentially for support of native modules in Electron with PnP using a bundler.

I'll have to look into how electron-builder packages up the app. Unfortunately it seems like it makes a lot of those node_modules assumptions.

https://github.com/electron-userland/electron-builder/blob/f0cb15d2723ee0da263cf2d1a71fcba777133f3f/packages/app-builder-lib/src/electron/electronVersion.ts#L29

But it also looks like they've added some explicit support?

https://github.com/electron-userland/electron-builder/blob/0fe8f1279112fa8bcb1e795e2fcd3594accc0b1f/packages/app-builder-lib/src/util/yarn.ts#L17

They have an issue here already.

electron-userland/electron-builder#4688

Having PnP work as a runtime in Electron isn't an explicit goal for me.

Does Yarn PnP's require override explicitly remove the checks for files in node_modules? If a node_modules folder exists, and contains a package, and you're running PnP and you require that package, will there be any conflict?

I assume there's some conflict given this functionality to explicitly remove any node_modules folders that exist after a PnP installation.

berry/packages/plugin-pnp/sources/PnpLinker.ts

Line 117 in 0d4a6f4

const nodeModules = await this.locateNodeModules();

---

Several hours of work later I've done some prototyping.

I prototyped a couple custom linker solutions. One that unplugged specific packages into the node_modules folder, but then you end up needing their dependencies unplugged and that becomes a slippery slope that I don't want to go down.

Another variation copied the packages with native modules into node_modules but kept the JS around in PnP.

This exposed the issue that the standard seems to be to use the bindings npm package to grab the file location of the .node native file.

https://github.com/serialport/node-serialport/blob/258574260f312cdeb81bb265a437762f8e5e0ae4/packages/bindings/lib/darwin.js#L2

https://github.com/tessel/node-usb/blob/249f0a96afd5dc91cb0c096f195c5e957607e5ce/usb.js#L1

It uses the v8 stack trace API to figure out what file called the function, to then start searching for the bindings file.

https://github.com/TooTallNate/node-bindings/blob/c8033dcfc04c34397384e23f7399a30e6c13830d/bindings.js#L155

For some reason in my testbed, the CallSite.getFileName(); calls return undefined, causing this not to work at all.

At this point I'm leaning toward a Webpack plugin that at a high level does this:

• Find the const binding = require('bindings')('bindings.node') calls.
• Statically work out which package those calls are in.
• Grab the node files
• Put them somewhere
• Rewrite the binding call to a regular node requirecall pointing at the new path that's actually on the filesystem.

I prototyped a Webpack plugin that would find these bindings calls and rewrite them, injecting the caller statically in the form YARN_SHOULD_GRAB_PREBUILD_FOR_<PACKAGE>

This gets us to the stage of this:

ERROR in ./.yarn/unplugged/serialport-npm-8.0.7-2339aa9a65/node_modules/serialport/lib/index.js
Module not found: Error: A package is trying to access another package without the second one being listed as a dependency of the first one

Required package: YARN_SHOULD_GRAB_PREBUILD_FOR_<@serialport (via "YARN_SHOULD_GRAB_PREBUILD_FOR_<@serialport/bindings>")
Required by: serialport@npm:8.0.7 (via ~/electron-pnp-bundler-test/.yarn/unplugged/serialport-npm-8.0.7-2339aa9a65/node_modules/serialport/lib/index.js)

 @ ./.yarn/unplugged/serialport-npm-8.0.7-2339aa9a65/node_modules/serialport/lib/index.js 2:16-47
 @ ./src/index.js

We can replace 'that' module with one that looks like:

const binding = require(<statically injected location of .node file>)
module.exports = () => binding

And we don't need rewrite the consumers of bindings at all.

I reckon a custom Yarn resolver and fetcher could be used to apply the prebuild-install spec. Yarn PnP knows exactly what electron version is installed! It could fetch the prebuild files and prevent the need for these packages to be unplugged in the first place.

Webpack can then treat the .node files like any other asset, hash them and store them where-ever.

---

A couple days later I've had some more time.

I wrote a yarn plugin that detects dependencies on bindings and grabs the native bindings using the prebuild-installer spec. It then replaces the dependency with one that statically grabs the .node bindings file.

https://github.com/electricui/yarn-prebuilds

This can then be used with a Webpack plugin or Rollup plugin to treat the .node file as any other asset.

With this I'm successfully able to have Webpack run in PnP land, bundle everything (including the native files), then I run Electron, explicitly removing the NODE_OPTIONS settings, and everything works as expected.

[alecmev]

Thank you for putting effort into this, @Mike-Dax! I went the NormalModuleReplacementPlugin way a couple of years ago, but this is much nicer, of course, since I don't need to list each native dependency in my Webpack config anymore.

Quick question, have you considering leaving the fetching to the packages containing the native modules, running binding's lookup code statically, and then replacing bindings itself with a require("foo.node")? If so, what made you give it a pass? If not, do you see anything wrong with such an approach? Packages that contain native modules download/build the modules anyway, so everything is already there, just gotta use it, unless I'm missing something.

Edit: Okay, yarn-prebuilds also selects an appropriate runtime, e.g. Electron. So it's kind of a prebuild-only replacement for electron-rebuild.

[rtritto]

@arcanis @alecmev @Mike-Dax is the issue electron/forge#3611 related to yarn's PnP (this issue) or electron/electron-forge?