refactor: minimize production dependency tree by MarshallOfSound · Pull Request #1244 · electron/rebuild

MarshallOfSound · 2026-03-24T22:57:12Z

Summary

Shrinks the dependency tree by replacing several packages with small inline implementations or Node.js builtins:

Removed	Replacement
`debug`	`src/debug.ts` — minimal logger gated on `DEBUG`
`detect-libc`	`src/detect-libc.ts` — `process.report`-based glibc/musl check
`got`	native `fetch`
`graceful-fs`	`node:fs` / `node:fs/promises`
`ora`	`src/spinner.ts` — tiny TTY spinner via `util.styleText`
`semver`, `tar`, `yargs`	node builtins / simpler equivalents

Also bumps node-gyp to ^12.2.0 and drops the now-unused src/promisifiedGracefulFs.ts wrapper along with associated @types/* devDeps.

Test plan

yarn test passes
CLI still parses flags correctly and renders spinner/success/fail states
Rebuild against a native module on Linux (glibc + musl) to validate libc detection
Cache snapshot/apply roundtrip still works with node:fs

socket-security · 2026-03-24T22:59:49Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@types/debug@4.1.7 ⏵ 4.1.12			^-2
	node-gyp@11.2.0 ⏵ 12.2.0			⁺¹

View full report

dsanders11

If the goal is just to drop "heavy" dependencies as in the title, I don't like several of these changes.

debug is small and only has one transitive dependency, but we're losing the richer functionality and consistency across our other packages that use debug
detect-libc I could take or leave, but it's zero dependency
ora is heavy and we should drop it, but there's zero dependency alternatives rather than rolling our own. We have multiple packages that currently use spinners (most use ora), we should migrate them all to the same alternative rather than having each package roll its own implementation that will drift. Or even create @electron/spinner, if we're insistent on cutting third-party dependencies.
We specifically kept graceful-fs when we did the Node 22 migration, we should not drop it here.

MarshallOfSound · 2026-03-25T00:15:31Z

Apologies @dsanders11 this was intended to be a draft PR, twas not ready for prime time

If the goal is just to drop "heavy" dependencies as in the title, I don't like several of these changes.

I'll rephrase the title, the objective is to drop as many dependencies as is reasonable regardless of weight

debug is small and only has one transitive dependency, but we're losing the richer functionality and consistency across our other packages that use debug

Agree on this, was chatting with @erickzhao we have a funky thing in here that confused some stuff. debugLog.ts existed in history and was just "brought back" to remove the dependency.

detect-libc I could take or leave, but it's zero dependency

It's a zero-dependency thing that was trivial to re-implement and avoid a supply-chain risk 🤷 Seems like a free win to inline it

ora is heavy and we should drop it, but there's zero dependency alternatives rather than rolling our own. We have multiple packages that currently use spinners (most use ora), we should migrate them all to the same alternative rather than having each package roll its own implementation that will drift. Or even create @electron/spinner, if we're insistent on cutting third-party dependencies.

Yeah this one I'm not sure about, tbh I think we should just remove the spinner. I don't think anyone actually uses the CLI anymore (it's just the hook for use in packager or forge).

We specifically kept graceful-fs when we did the Node 22 migration, we should not drop it here.

I didn't have that context but on discussion with @erickzhao I don't see a reason to keep it in this module? The only heavy operations that could possible trigger EMFILE (as I understand it the only reason to use this module) are in the cache implementation and whilst both are unbounded neither are going to operate on enough files to trip that behavior.

There are zero historical reports (even predating fs-extra) of EMFILE in this repo: ref and I don't think we should keep a dependency for basic fs operations out of FUD 🤔

Replace several third-party dependencies with small inline implementations or Node.js builtins: - debug → minimal logger gated on DEBUG env var - detect-libc → process.report-based glibc/musl detection - got → native fetch - graceful-fs → node:fs (drop promisifiedGracefulFs wrapper) - ora → tiny TTY spinner using util.styleText - semver, tar, yargs → node builtins / simpler equivalents Also bumps node-gyp to ^12.2.0.

- Restore `debug` dependency for consistency with other @electron packages - Remove spinner entirely instead of inlining (CLI is primarily a packager/forge hook, not used directly) - Add `modules-found` lifecycle event so CLI can print all module names in one line before building starts

dsanders11 · 2026-03-25T02:14:46Z

Yeah this one I'm not sure about, tbh I think we should just remove the spinner. I don't think anyone actually uses the CLI anymore (it's just the hook for use in packager or forge).

I'm fine with removing the spinner, and I'd prefer removing it all together over rolling our own here.

I didn't have that context but on discussion with @erickzhao I don't see a reason to keep it in this module? The only heavy operations that could possible trigger EMFILE (as I understand it the only reason to use this module) are in the cache implementation and whilst both are unbounded neither are going to operate on enough files to trip that behavior.

There are zero historical reports (even predating fs-extra) of EMFILE in this repo: ref and I don't think we should keep a dependency for basic fs operations out of FUD 🤔

A bit more context here. For the usage in this particular repo we might be safe to remove it, but I do have a memory (can't find PR at the moment) of us getting bit on one package during the Node 22 transition where we restored it, which was part of the reason we opted to leave it in repos during that transition.

Also, graceful-fs is a dependency of node-gyp so it's not like we're fully getting rid of that dependency by removing our top-level usage of it. I wouldn't describe it as FUD, just being more conservative to prevent unintended breakage.

MarshallOfSound · 2026-03-25T05:01:02Z

Also, graceful-fs is a dependency of node-gyp so it's not like we're fully getting rid of that dependency by removing our top-level usage of it. I wouldn't describe it as FUD, just being more conservative to prevent unintended breakage.

Yeah I saw that too, minimizing the tree is still important too me. If we have strong reason to believe something will break as a result of removing it I'm happy to put it back (it's also trivial to put back) but right now it seems like a branch we can snip off relatively easily

I cleaned up the other removals and such, should be in a better spot now

erickzhao · 2026-03-30T23:04:25Z

+import debug from 'debug';
+
+export const d = debug('electron-rebuild');


suggestion: I'd remove this one-liner helper export and replace it with direct debug calls for each file.

We do this in a lot of other @electron/ packages, and it gives us the flexibility to add suffixes for each file or logically distinct module.

erickzhao · 2026-03-30T23:05:20Z

@@ -0,0 +1,14 @@
+let cached: string | null | undefined;
+
+export function detectLibcFamily(): string | null {


suggestion: Can we add a TSDoc comment about the purpose of this helper and the various strings it can return?

erickzhao · 2026-03-30T23:11:03Z

+    const [maj, min] = this.rebuilder.electronVersion.split('.').map(Number);
+    if (maj < 14 || (maj === 14 && min < 2) || (maj === 15 && min < 3)) {


atp i'm okay removing the semver dep here but i can see the possibility of having to add it back if we ever need trickier conditions.

waiting for semver parsing to be exposed to node:util one day i hope 😭

erickzhao · 2026-03-30T23:12:07Z

+    const short = 'short' in opt ? `-${opt.short}, ` : '    ';
+    console.log(`  ${short}--${name.padEnd(22)} ${opt.description}`);
+  }
+  console.log('\nCopyright 2016-2021');


funny, do we print copyright notices anywhere else in our CLIs? i'd remove this if not tbh

- Remove src/debug.ts wrapper, inline debug() per-file (restores the pattern on main, allows per-file namespace suffixes later) - Add TSDoc to detectLibcFamily documenting return values - Drop the copyright line from CLI help output

MarshallOfSound requested a review from a team as a code owner March 24, 2026 22:57

dsanders11 requested changes Mar 25, 2026

View reviewed changes

dsanders11 mentioned this pull request Mar 25, 2026

refactor: replace got with native fetch API #1243

Merged

MarshallOfSound added 2 commits March 24, 2026 17:19

MarshallOfSound force-pushed the sam/reduce-dependencies branch from b4ebcdb to f43fb14 Compare March 25, 2026 00:20

MarshallOfSound changed the title ~~refactor: drop heavy dependencies in favor of node builtins~~ refactor: minimize production dependency tree Mar 25, 2026

erickzhao mentioned this pull request Mar 30, 2026

fix: require node-gyp@^12.2.0 #1245

Closed

erickzhao reviewed Mar 30, 2026

View reviewed changes

refactor: address review feedback

0ef17c7

- Remove src/debug.ts wrapper, inline debug() per-file (restores the pattern on main, allows per-file namespace suffixes later) - Add TSDoc to detectLibcFamily documenting return values - Drop the copyright line from CLI help output

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: minimize production dependency tree#1244

refactor: minimize production dependency tree#1244
MarshallOfSound wants to merge 3 commits intomainfrom
sam/reduce-dependencies

MarshallOfSound commented Mar 24, 2026

Uh oh!

socket-security bot commented Mar 24, 2026 •

edited

Loading

Uh oh!

dsanders11 left a comment

Uh oh!

MarshallOfSound commented Mar 25, 2026

Uh oh!

dsanders11 commented Mar 25, 2026

Uh oh!

MarshallOfSound commented Mar 25, 2026

Uh oh!

erickzhao Mar 30, 2026

Uh oh!

erickzhao Mar 30, 2026

Uh oh!

erickzhao Mar 30, 2026

Uh oh!

erickzhao Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		import debug from 'debug';

		export const d = debug('electron-rebuild');

		@@ -0,0 +1,14 @@
		let cached: string \| null \| undefined;

		export function detectLibcFamily(): string \| null {

		const [maj, min] = this.rebuilder.electronVersion.split('.').map(Number);
		if (maj < 14 \|\| (maj === 14 && min < 2) \|\| (maj === 15 && min < 3)) {

Conversation

MarshallOfSound commented Mar 24, 2026

Summary

Test plan

Uh oh!

socket-security bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dsanders11 left a comment

Choose a reason for hiding this comment

Uh oh!

MarshallOfSound commented Mar 25, 2026

Uh oh!

dsanders11 commented Mar 25, 2026

Uh oh!

MarshallOfSound commented Mar 25, 2026

Uh oh!

erickzhao Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

erickzhao Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

erickzhao Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

erickzhao Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

socket-security bot commented Mar 24, 2026 •

edited

Loading