rfc: lazy electron download, remove postinstall

[MarshallOfSound]

Raising this as an RFC mostly because I want to be sure I haven't missed something obvious but also I don't super have the time to do it so hopefully this sketches out a nice win that someone else can run with.

[reitowo]

I'm in favor of this, developers use pnpm@10+ will default skip running postinstall, and they are aware of this if they choose to upgrade pnpm, and for older developers we can keep the behavior to prevent break workflows. The lazy download could be a additional check if developers doesn't install the binary for whatever reason.

[MarshallOfSound]

The reason I excluded this was consistency, the first question in the bug report shouldn't be "ok so do you have postinstall enabled? oh you don't know? ok are you using pnpm? or yarn > 4.10?". That just doesn't found fun. I think the experience should be consistent across all package managers

[erickzhao]

To me, being able to run npx install-electron on postinstall sounds like a happy workaround for devs who want to still run it that way 👍

[itsananderson]

I wonder if it'd be better to define a bin script in the electron module called install-electron?

This would avoid needing a separate package, and I think it would also simplify the install-electron logic, because now the install-electron script would be inside the electron package, so it wouldn't need to deal with package hoisting or Yarn workspaces or anything, because it would always know exactly where the electron package is installed.

With this approach, people could still run:

npm install electron
npx install-electron

Or run the bin command in their own postinstall:

{
  "scripts": {
    "postinstall": "install-electron"
  }
}

[MarshallOfSound]

To clarify it would be a bin command in the same electron package.

npx runs any bin command, not just package names

[erickzhao]

I wonder if we should still reserve the install-electron package name so people don't run npx install-electron in the wrong dir and run some random untrusted code by accident.

[MarshallOfSound]

Good call, it's mine 😅 we can do what we want with it

[MarshallOfSound]

But also, npx considered harmful. I'd advocate for not documenting it (used in this spec because it's so easy to type lol)

[itsananderson]

One other thing that we may want to call out in the RFC is that usage of ELECTRON_MIRROR (and friends) will change. Previously you would set those variables when running npm install, but now I guess you'd need to set them either when running install-electron or when invoking electron.

There's also ELECTRON_SKIP_BINARY_DOWNLOAD which probably just goes away with this new approach?

[MarshallOfSound]

There's also ELECTRON_SKIP_BINARY_DOWNLOAD which probably just goes away with this new approach?

Yup

One other thing that we may want to call out in the RFC is that usage of ELECTRON_MIRROR (and friends) will change. Previously you would set those variables when running npm install, but now I guess you'd need to set them either when running install-electron or when invoking electron.

Yeah, that should be fairly trivial for folks to update though, just set the env var more globally in CI or locally it's probably already in their zshrc or something anyway and will Just Work

[erickzhao]

RFC LGTM

[itsananderson]

RFC LGTM

[samuelmaddock]

RFC LGTM

Thanks for the great writeup and explainer!

[reitowo]

RFC LGTM

[jkleinsc]

RFC LGTM