From 7c773554f1235a1856bcf8c17dafc8c69a361d20 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Thu, 23 Jan 2025 16:29:51 -0800 Subject: [PATCH 01/30] chore: rename yarn-or-npm -> package-manager --- .../spec/{yarn-or-npm.spec.ts => package-manager.spec.ts} | 6 +++--- packages/utils/core-utils/src/index.ts | 2 +- .../core-utils/src/{yarn-or-npm.ts => package-manager.ts} | 0 3 files changed, 4 insertions(+), 4 deletions(-) rename packages/utils/core-utils/spec/{yarn-or-npm.spec.ts => package-manager.spec.ts} (88%) rename packages/utils/core-utils/src/{yarn-or-npm.ts => package-manager.ts} (100%) diff --git a/packages/utils/core-utils/spec/yarn-or-npm.spec.ts b/packages/utils/core-utils/spec/package-manager.spec.ts similarity index 88% rename from packages/utils/core-utils/spec/yarn-or-npm.spec.ts rename to packages/utils/core-utils/spec/package-manager.spec.ts index ec02b9b1a8..380ec87d9a 100644 --- a/packages/utils/core-utils/spec/yarn-or-npm.spec.ts +++ b/packages/utils/core-utils/spec/package-manager.spec.ts @@ -1,9 +1,9 @@ import { detect } from 'detect-package-manager'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { safeYarnOrNpm } from '../src/yarn-or-npm'; +import { safeYarnOrNpm } from '../src/package-manager'; -describe('yarn-or-npm', () => { +describe('package-manager', () => { let nodeInstaller: string | undefined; beforeEach(() => { @@ -19,7 +19,7 @@ describe('yarn-or-npm', () => { } }); - it('should by default equal the system yarn-or-npm value', async () => { + it('should by default equal the system package manager value', async () => { const pm = await detect(); await expect(safeYarnOrNpm()).resolves.toEqual(pm); }); diff --git a/packages/utils/core-utils/src/index.ts b/packages/utils/core-utils/src/index.ts index 2b4f1876ef..e5b67335f6 100644 --- a/packages/utils/core-utils/src/index.ts +++ b/packages/utils/core-utils/src/index.ts @@ -1,3 +1,3 @@ export * from './rebuild'; export * from './electron-version'; -export * from './yarn-or-npm'; +export * from './package-manager'; diff --git a/packages/utils/core-utils/src/yarn-or-npm.ts b/packages/utils/core-utils/src/package-manager.ts similarity index 100% rename from packages/utils/core-utils/src/yarn-or-npm.ts rename to packages/utils/core-utils/src/package-manager.ts From 2f6ef3d9d53a5509a2b4f284316e8eb119e9ddbd Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Thu, 23 Jan 2025 16:44:08 -0800 Subject: [PATCH 02/30] refactor: remove hasYarn function and rename spawn --- packages/api/cli/package.json | 1 + packages/api/cli/src/util/check-system.ts | 6 +- .../fast/util/install-dependencies.spec.ts | 26 ++-- packages/api/core/src/api/import.ts | 4 +- .../core/src/api/init-scripts/init-link.ts | 6 +- .../api/core/src/api/init-scripts/init-npm.ts | 4 +- packages/api/core/src/api/init.ts | 4 +- packages/api/core/src/util/index.ts | 6 +- .../api/core/src/util/install-dependencies.ts | 9 +- .../core-utils/spec/package-manager.spec.ts | 10 +- .../utils/core-utils/src/package-manager.ts | 6 +- yarn.lock | 132 +----------------- 12 files changed, 41 insertions(+), 173 deletions(-) diff --git a/packages/api/cli/package.json b/packages/api/cli/package.json index a4fde83251..9a8e93e45c 100644 --- a/packages/api/cli/package.json +++ b/packages/api/cli/package.json @@ -16,6 +16,7 @@ }, "dependencies": { "@electron-forge/core": "7.6.1", + "@electron-forge/core-utils": "7.6.1", "@electron-forge/shared-types": "7.6.1", "@electron/get": "^3.0.0", "chalk": "^4.0.0", diff --git a/packages/api/cli/src/util/check-system.ts b/packages/api/cli/src/util/check-system.ts index a07993e579..fab3685b0f 100644 --- a/packages/api/cli/src/util/check-system.ts +++ b/packages/api/cli/src/util/check-system.ts @@ -2,7 +2,7 @@ import { exec } from 'node:child_process'; import os from 'node:os'; import path from 'node:path'; -import { utils as forgeUtils } from '@electron-forge/core'; +import { resolvePackageManager, spawnPackageManager } from '@electron-forge/core-utils'; import { ForgeListrTask } from '@electron-forge/shared-types'; import debug from 'debug'; import fs from 'fs-extra'; @@ -55,9 +55,9 @@ function warnIfPackageManagerIsntAKnownGoodVersion(packageManager: string, versi } async function checkPackageManagerVersion() { - const version = await forgeUtils.yarnOrNpmSpawn(['--version']); + const version = await spawnPackageManager(['--version']); const versionString = version.toString().trim(); - if (await forgeUtils.hasYarn()) { + if ((await resolvePackageManager()) === 'yarn') { warnIfPackageManagerIsntAKnownGoodVersion('Yarn', versionString, YARN_ALLOWLISTED_VERSIONS); return `yarn@${versionString}`; } else { diff --git a/packages/api/core/spec/fast/util/install-dependencies.spec.ts b/packages/api/core/spec/fast/util/install-dependencies.spec.ts index c382321cd2..21249e891b 100644 --- a/packages/api/core/spec/fast/util/install-dependencies.spec.ts +++ b/packages/api/core/spec/fast/util/install-dependencies.spec.ts @@ -1,4 +1,4 @@ -import { hasYarn, yarnOrNpmSpawn } from '@electron-forge/core-utils'; +import { resolvePackageManager, spawnPackageManager } from '@electron-forge/core-utils'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import installDependencies, { DepType, DepVersionRestriction } from '../../../src/util/install-dependencies'; @@ -7,53 +7,53 @@ vi.mock(import('@electron-forge/core-utils'), async (importOriginal) => { const mod = await importOriginal(); return { ...mod, - hasYarn: vi.fn(), - yarnOrNpmSpawn: vi.fn(), + resolvePackageManager: vi.fn(), + spawnPackageManager: vi.fn(), }; }); describe('installDependencies', () => { it('should immediately resolve if no deps are provided', async () => { await installDependencies('mydir', []); - expect(yarnOrNpmSpawn).not.toHaveBeenCalled(); + expect(spawnPackageManager).not.toHaveBeenCalled(); }); it('should reject if reject the promise if exit code is not 0', async () => { - vi.mocked(yarnOrNpmSpawn).mockRejectedValueOnce('fail'); + vi.mocked(spawnPackageManager).mockRejectedValueOnce('fail'); await expect(installDependencies('void', ['electron'])).rejects.toThrow('fail'); }); it('should resolve if reject the promise if exit code is 0', async () => { - vi.mocked(yarnOrNpmSpawn).mockResolvedValueOnce('pass'); + vi.mocked(spawnPackageManager).mockResolvedValueOnce('pass'); await expect(installDependencies('void', ['electron'])).resolves.toBe(undefined); }); describe.each([ - { pm: 'npm', install: 'install', flags: { exact: '--save-exact', dev: '--save-dev' } }, - { pm: 'yarn', install: 'add', flags: { exact: '--exact', dev: '--dev' } }, + { pm: 'npm' as const, install: 'install', flags: { exact: '--save-exact', dev: '--save-dev' } }, + { pm: 'yarn' as const, install: 'add', flags: { exact: '--exact', dev: '--dev' } }, ])('$pm', ({ pm, install, flags }) => { beforeEach(() => { - vi.mocked(hasYarn).mockResolvedValue(pm === 'yarn'); + vi.mocked(resolvePackageManager).mockResolvedValue(pm); }); it('should install deps', async () => { await installDependencies('mydir', ['react']); - expect(yarnOrNpmSpawn).toHaveBeenCalledWith([install, 'react'], expect.anything()); + expect(spawnPackageManager).toHaveBeenCalledWith([install, 'react'], expect.anything()); }); it('should install dev deps', async () => { await installDependencies('mydir', ['eslint'], DepType.DEV); - expect(yarnOrNpmSpawn).toHaveBeenCalledWith([install, 'eslint', flags.dev], expect.anything()); + expect(spawnPackageManager).toHaveBeenCalledWith([install, 'eslint', flags.dev], expect.anything()); }); it('should install exact deps', async () => { await installDependencies('mydir', ['react'], DepType.PROD, DepVersionRestriction.EXACT); - expect(yarnOrNpmSpawn).toHaveBeenCalledWith([install, 'react', flags.exact], expect.anything()); + expect(spawnPackageManager).toHaveBeenCalledWith([install, 'react', flags.exact], expect.anything()); }); it('should install exact dev deps', async () => { await installDependencies('mydir', ['eslint'], DepType.DEV, DepVersionRestriction.EXACT); - expect(yarnOrNpmSpawn).toHaveBeenCalledWith([install, 'eslint', flags.dev, flags.exact], expect.anything()); + expect(spawnPackageManager).toHaveBeenCalledWith([install, 'eslint', flags.dev, flags.exact], expect.anything()); }); }); }); diff --git a/packages/api/core/src/api/import.ts b/packages/api/core/src/api/import.ts index ed764e6404..31c491f7bd 100644 --- a/packages/api/core/src/api/import.ts +++ b/packages/api/core/src/api/import.ts @@ -1,6 +1,6 @@ import path from 'node:path'; -import { safeYarnOrNpm, updateElectronDependency } from '@electron-forge/core-utils'; +import { resolvePackageManager, updateElectronDependency } from '@electron-forge/core-utils'; import { ForgeListrOptions, ForgeListrTaskFn } from '@electron-forge/shared-types'; import baseTemplate from '@electron-forge/template-base'; import { autoTrace } from '@electron-forge/tracer'; @@ -193,7 +193,7 @@ export default autoTrace( { title: 'Installing dependencies', task: async (_, task) => { - const packageManager = await safeYarnOrNpm(); + const packageManager = await resolvePackageManager(); await writeChanges(); d('deleting old dependencies forcefully'); diff --git a/packages/api/core/src/api/init-scripts/init-link.ts b/packages/api/core/src/api/init-scripts/init-link.ts index 93ebde4a7e..8a6fe8b434 100644 --- a/packages/api/core/src/api/init-scripts/init-link.ts +++ b/packages/api/core/src/api/init-scripts/init-link.ts @@ -1,6 +1,6 @@ import path from 'node:path'; -import { safeYarnOrNpm, yarnOrNpmSpawn } from '@electron-forge/core-utils'; +import { resolvePackageManager, spawnPackageManager } from '@electron-forge/core-utils'; import { ForgeListrTask } from '@electron-forge/shared-types'; import debug from 'debug'; @@ -22,12 +22,12 @@ export async function initLink(dir: string, task?: ForgeListrTask) { if (shouldLink) { d('Linking forge dependencies'); const packageJson = await readRawPackageJson(dir); - const packageManager = await safeYarnOrNpm(); + const packageManager = await resolvePackageManager(); const linkFolder = path.resolve(__dirname, '..', '..', '..', '..', '..', '..', '.links'); for (const packageName of Object.keys(packageJson.devDependencies)) { if (packageName.startsWith('@electron-forge/')) { if (task) task.output = `${packageManager} link --link-folder ${linkFolder} ${packageName}`; - await yarnOrNpmSpawn(['link', '--link-folder', linkFolder, packageName], { + await spawnPackageManager(['link', '--link-folder', linkFolder, packageName], { cwd: dir, }); } diff --git a/packages/api/core/src/api/init-scripts/init-npm.ts b/packages/api/core/src/api/init-scripts/init-npm.ts index ecc63e8546..c686dc2cdb 100644 --- a/packages/api/core/src/api/init-scripts/init-npm.ts +++ b/packages/api/core/src/api/init-scripts/init-npm.ts @@ -1,6 +1,6 @@ import path from 'node:path'; -import { safeYarnOrNpm } from '@electron-forge/core-utils'; +import { resolvePackageManager } from '@electron-forge/core-utils'; import { ForgeListrTask } from '@electron-forge/shared-types'; import debug from 'debug'; import fs from 'fs-extra'; @@ -29,7 +29,7 @@ export const exactDevDeps = ['electron']; export const initNPM = async (dir: string, task: ForgeListrTask): Promise => { d('installing dependencies'); - const packageManager = await safeYarnOrNpm(); + const packageManager = await resolvePackageManager(); task.output = `${packageManager} install ${deps.join(' ')}`; await installDepList(dir, deps); diff --git a/packages/api/core/src/api/init.ts b/packages/api/core/src/api/init.ts index d3c92f1f35..6e8529af7c 100644 --- a/packages/api/core/src/api/init.ts +++ b/packages/api/core/src/api/init.ts @@ -1,6 +1,6 @@ import path from 'node:path'; -import { safeYarnOrNpm } from '@electron-forge/core-utils'; +import { resolvePackageManager } from '@electron-forge/core-utils'; import { ForgeTemplate } from '@electron-forge/shared-types'; import debug from 'debug'; import { Listr } from 'listr2'; @@ -56,7 +56,7 @@ async function validateTemplate(template: string, templateModule: ForgeTemplate) export default async ({ dir = process.cwd(), interactive = false, copyCIFiles = false, force = false, template = 'base' }: InitOptions): Promise => { d(`Initializing in: ${dir}`); - const packageManager = await safeYarnOrNpm(); + const packageManager = await resolvePackageManager(); const runner = new Listr<{ templateModule: ForgeTemplate; diff --git a/packages/api/core/src/util/index.ts b/packages/api/core/src/util/index.ts index c9d76afaec..b8f7752dcf 100644 --- a/packages/api/core/src/util/index.ts +++ b/packages/api/core/src/util/index.ts @@ -1,4 +1,4 @@ -import { getElectronVersion, hasYarn, yarnOrNpmSpawn } from '@electron-forge/core-utils'; +import { getElectronVersion, spawnPackageManager } from '@electron-forge/core-utils'; import { BuildIdentifierConfig, @@ -24,9 +24,7 @@ export default class ForgeUtils { getElectronVersion = getElectronVersion; - hasYarn = hasYarn; - - yarnOrNpmSpawn = yarnOrNpmSpawn; + spawnPackageManager = spawnPackageManager; /** * Register a virtual config file for forge to find. diff --git a/packages/api/core/src/util/install-dependencies.ts b/packages/api/core/src/util/install-dependencies.ts index ca616b10e0..81896b6c58 100644 --- a/packages/api/core/src/util/install-dependencies.ts +++ b/packages/api/core/src/util/install-dependencies.ts @@ -1,4 +1,4 @@ -import { hasYarn, yarnOrNpmSpawn } from '@electron-forge/core-utils'; +import { resolvePackageManager, spawnPackageManager } from '@electron-forge/core-utils'; import { ExitError } from '@malept/cross-spawn-promise'; import debug from 'debug'; @@ -15,13 +15,14 @@ export enum DepVersionRestriction { } export default async (dir: string, deps: string[], depType = DepType.PROD, versionRestriction = DepVersionRestriction.RANGE): Promise => { - d('installing', JSON.stringify(deps), 'in:', dir, `depType=${depType},versionRestriction=${versionRestriction},withYarn=${await hasYarn()}`); + const pm = await resolvePackageManager(); + d('installing', JSON.stringify(deps), 'in:', dir, `depType=${depType},versionRestriction=${versionRestriction},withPackageManager=${pm}`); if (deps.length === 0) { d('nothing to install, stopping immediately'); return Promise.resolve(); } let cmd = ['install'].concat(deps); - if (await hasYarn()) { + if (pm === 'yarn') { cmd = ['add'].concat(deps); if (depType === DepType.DEV) cmd.push('--dev'); if (versionRestriction === DepVersionRestriction.EXACT) cmd.push('--exact'); @@ -31,7 +32,7 @@ export default async (dir: string, deps: string[], depType = DepType.PROD, versi } d('executing', JSON.stringify(cmd), 'in:', dir); try { - await yarnOrNpmSpawn(cmd, { + await spawnPackageManager(cmd, { cwd: dir, stdio: 'pipe', }); diff --git a/packages/utils/core-utils/spec/package-manager.spec.ts b/packages/utils/core-utils/spec/package-manager.spec.ts index 380ec87d9a..c4c0df21ed 100644 --- a/packages/utils/core-utils/spec/package-manager.spec.ts +++ b/packages/utils/core-utils/spec/package-manager.spec.ts @@ -1,7 +1,7 @@ import { detect } from 'detect-package-manager'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { safeYarnOrNpm } from '../src/package-manager'; +import { resolvePackageManager } from '../src/package-manager'; describe('package-manager', () => { let nodeInstaller: string | undefined; @@ -21,24 +21,24 @@ describe('package-manager', () => { it('should by default equal the system package manager value', async () => { const pm = await detect(); - await expect(safeYarnOrNpm()).resolves.toEqual(pm); + await expect(resolvePackageManager()).resolves.toEqual(pm); }); it('should return yarn if NODE_INSTALLER=yarn', async () => { process.env.NODE_INSTALLER = 'yarn'; - await expect(safeYarnOrNpm()).resolves.toEqual('yarn'); + await expect(resolvePackageManager()).resolves.toEqual('yarn'); }); it('should return npm if NODE_INSTALLER=npm', async () => { process.env.NODE_INSTALLER = 'npm'; - await expect(safeYarnOrNpm()).resolves.toEqual('npm'); + await expect(resolvePackageManager()).resolves.toEqual('npm'); }); it('should return system value if NODE_INSTALLER is an unrecognized installer', async () => { process.env.NODE_INSTALLER = 'magical_unicorn'; console.warn = vi.fn(); const pm = await detect(); - await expect(safeYarnOrNpm()).resolves.toEqual(pm); + await expect(resolvePackageManager()).resolves.toEqual(pm); expect(console.warn).toHaveBeenCalledWith('⚠', expect.stringContaining('Unknown NODE_INSTALLER')); }); }); diff --git a/packages/utils/core-utils/src/package-manager.ts b/packages/utils/core-utils/src/package-manager.ts index ad8055e455..b5741ebceb 100644 --- a/packages/utils/core-utils/src/package-manager.ts +++ b/packages/utils/core-utils/src/package-manager.ts @@ -3,7 +3,7 @@ import chalk from 'chalk'; import { detect } from 'detect-package-manager'; import logSymbols from 'log-symbols'; -export const safeYarnOrNpm = async () => { +export const resolvePackageManager = async () => { const system = await detect(); switch (process.env.NODE_INSTALLER) { case 'yarn': @@ -17,6 +17,4 @@ export const safeYarnOrNpm = async () => { } }; -export const yarnOrNpmSpawn = async (args?: CrossSpawnArgs, opts?: CrossSpawnOptions): Promise => spawn(await safeYarnOrNpm(), args, opts); - -export const hasYarn = async () => (await safeYarnOrNpm()) === 'yarn'; +export const spawnPackageManager = async (args?: CrossSpawnArgs, opts?: CrossSpawnOptions): Promise => spawn(await resolvePackageManager(), args, opts); diff --git a/yarn.lock b/yarn.lock index 06c2c42a92..64a578ab16 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3372,16 +3372,6 @@ resolved "https://registry.yarnpkg.com/@ungap/structured-clone/-/structured-clone-1.2.1.tgz#28fa185f67daaf7b7a1a8c1d445132c5d979f8bd" integrity sha512-fEzPV3hSkSMltkw152tJKNARhOupqbH96MZWyRjNaYZOMIzbrTeQDG+MTc6Mr2pgzFQzFxAfmhGDNP5QK++2ZA== -"@vitest/expect@2.1.6": - version "2.1.6" - resolved "https://registry.yarnpkg.com/@vitest/expect/-/expect-2.1.6.tgz#5a334eb9ee9287292fbe961955cafb06f7af7da6" - integrity sha512-9M1UR9CAmrhJOMoSwVnPh2rELPKhYo0m/CSgqw9PyStpxtkwhmdM6XYlXGKeYyERY1N6EIuzkQ7e3Lm1WKCoUg== - dependencies: - "@vitest/spy" "2.1.6" - "@vitest/utils" "2.1.6" - chai "^5.1.2" - tinyrainbow "^1.2.0" - "@vitest/expect@3.0.3": version "3.0.3" resolved "https://registry.yarnpkg.com/@vitest/expect/-/expect-3.0.3.tgz#a83af04a68e70a9af8aa6f68442a696b4bc599c5" @@ -3392,15 +3382,6 @@ chai "^5.1.2" tinyrainbow "^2.0.0" -"@vitest/mocker@2.1.6": - version "2.1.6" - resolved "https://registry.yarnpkg.com/@vitest/mocker/-/mocker-2.1.6.tgz#d13c5a7bd0abf432e1030f68acb43f51c4b3692e" - integrity sha512-MHZp2Z+Q/A3am5oD4WSH04f9B0T7UvwEb+v5W0kCYMhtXGYbdyl2NUk1wdSMqGthmhpiThPDp/hEoVwu16+u1A== - dependencies: - "@vitest/spy" "2.1.6" - estree-walker "^3.0.3" - magic-string "^0.30.12" - "@vitest/mocker@3.0.3": version "3.0.3" resolved "https://registry.yarnpkg.com/@vitest/mocker/-/mocker-3.0.3.tgz#f63a7e2e93fecaab1046038f3a9f60ea6b369173" @@ -3410,13 +3391,6 @@ estree-walker "^3.0.3" magic-string "^0.30.17" -"@vitest/pretty-format@2.1.6", "@vitest/pretty-format@^2.1.6": - version "2.1.6" - resolved "https://registry.yarnpkg.com/@vitest/pretty-format/-/pretty-format-2.1.6.tgz#9bc642047a3efc637b41492b1f222c43be3822e4" - integrity sha512-exZyLcEnHgDMKc54TtHca4McV4sKT+NKAe9ix/yhd/qkYb/TP8HTyXRFDijV19qKqTZM0hPL4753zU/U8L/gAA== - dependencies: - tinyrainbow "^1.2.0" - "@vitest/pretty-format@3.0.3", "@vitest/pretty-format@^3.0.3": version "3.0.3" resolved "https://registry.yarnpkg.com/@vitest/pretty-format/-/pretty-format-3.0.3.tgz#4bd59463d1c944c22287c3da2060785269098183" @@ -3424,14 +3398,6 @@ dependencies: tinyrainbow "^2.0.0" -"@vitest/runner@2.1.6": - version "2.1.6" - resolved "https://registry.yarnpkg.com/@vitest/runner/-/runner-2.1.6.tgz#948cad2cccfe2e56be5b3f9979cf9a417ca59737" - integrity sha512-SjkRGSFyrA82m5nz7To4CkRSEVWn/rwQISHoia/DB8c6IHIhaE/UNAo+7UfeaeJRE979XceGl00LNkIz09RFsA== - dependencies: - "@vitest/utils" "2.1.6" - pathe "^1.1.2" - "@vitest/runner@3.0.3": version "3.0.3" resolved "https://registry.yarnpkg.com/@vitest/runner/-/runner-3.0.3.tgz#c123e3225ccdd52c5a8e45edb59340ec8dcb6df2" @@ -3440,15 +3406,6 @@ "@vitest/utils" "3.0.3" pathe "^2.0.1" -"@vitest/snapshot@2.1.6": - version "2.1.6" - resolved "https://registry.yarnpkg.com/@vitest/snapshot/-/snapshot-2.1.6.tgz#21740449221e37f80c4a8fb3e15f100f30e7934d" - integrity sha512-5JTWHw8iS9l3v4/VSuthCndw1lN/hpPB+mlgn1BUhFbobeIUj1J1V/Bj2t2ovGEmkXLTckFjQddsxS5T6LuVWw== - dependencies: - "@vitest/pretty-format" "2.1.6" - magic-string "^0.30.12" - pathe "^1.1.2" - "@vitest/snapshot@3.0.3": version "3.0.3" resolved "https://registry.yarnpkg.com/@vitest/snapshot/-/snapshot-3.0.3.tgz#a20a8cfa0e7434ef94f4dff40d946a57922119de" @@ -3458,13 +3415,6 @@ magic-string "^0.30.17" pathe "^2.0.1" -"@vitest/spy@2.1.6": - version "2.1.6" - resolved "https://registry.yarnpkg.com/@vitest/spy/-/spy-2.1.6.tgz#229f9d48b90b8bdd6573723bdec0699915598080" - integrity sha512-oTFObV8bd4SDdRka5O+mSh5w9irgx5IetrD5i+OsUUsk/shsBoHifwCzy45SAORzAhtNiprUVaK3hSCCzZh1jQ== - dependencies: - tinyspy "^3.0.2" - "@vitest/spy@3.0.3": version "3.0.3" resolved "https://registry.yarnpkg.com/@vitest/spy/-/spy-3.0.3.tgz#ea4e5f7f8b3513e3ac0e556557e4ed339edc82e8" @@ -3472,15 +3422,6 @@ dependencies: tinyspy "^3.0.2" -"@vitest/utils@2.1.6": - version "2.1.6" - resolved "https://registry.yarnpkg.com/@vitest/utils/-/utils-2.1.6.tgz#2af6a82c5c45da35ecd322d0568247a6e9c95c5f" - integrity sha512-ixNkFy3k4vokOUTU2blIUvOgKq/N2PW8vKIjZZYsGJCMX69MRa9J2sKqX5hY/k5O5Gty3YJChepkqZ3KM9LyIQ== - dependencies: - "@vitest/pretty-format" "2.1.6" - loupe "^3.1.2" - tinyrainbow "^1.2.0" - "@vitest/utils@3.0.3": version "3.0.3" resolved "https://registry.yarnpkg.com/@vitest/utils/-/utils-3.0.3.tgz#25d5a2e0cd0b5529132b76482fd48139ca56c197" @@ -5216,13 +5157,6 @@ debug@^3.2.7: dependencies: ms "^2.1.1" -debug@^4.3.7: - version "4.3.7" - resolved "https://registry.yarnpkg.com/debug/-/debug-4.3.7.tgz#87945b4151a011d76d95a198d7111c865c360a52" - integrity sha512-Er2nc/H7RrMXZBFCEim6TCmMk02Z8vLC2Rbi1KEBggpo0fS6l0S1nnapwmIi3yW/+GOJap1Krg4w0Hg80oCqgQ== - dependencies: - ms "^2.1.3" - debug@^4.4.0: version "4.4.0" resolved "https://registry.yarnpkg.com/debug/-/debug-4.4.0.tgz#2b3f2aea2ffeb776477460267377dc8710faba8a" @@ -5977,11 +5911,6 @@ es-module-lexer@^1.2.1: resolved "https://registry.yarnpkg.com/es-module-lexer/-/es-module-lexer-1.4.1.tgz#41ea21b43908fe6a287ffcbe4300f790555331f5" integrity sha512-cXLGjP0c4T3flZJKQSuziYoq7MlT+rnvfZjfp7h+I7K9BNX54kP9nyWvdbwjQ4u1iWbOL4u96fgeZLToQlZC7w== -es-module-lexer@^1.5.4: - version "1.5.4" - resolved "https://registry.yarnpkg.com/es-module-lexer/-/es-module-lexer-1.5.4.tgz#a8efec3a3da991e60efa6b633a7cad6ab8d26b78" - integrity sha512-MVNK56NiMrOwitFB7cqDwq0CQutbw+0BvLshJSse0MUNU+y1FC3bUS/AQg7oUng+/wKrrki7JfmwtVHkVfPLlw== - es-module-lexer@^1.6.0: version "1.6.0" resolved "https://registry.yarnpkg.com/es-module-lexer/-/es-module-lexer-1.6.0.tgz#da49f587fd9e68ee2404fe4e256c0c7d3a81be21" @@ -9482,13 +9411,6 @@ macos-alias@~0.2.5: dependencies: nan "^2.4.0" -magic-string@^0.30.12: - version "0.30.14" - resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.30.14.tgz#e9bb29870b81cfc1ec3cc656552f5a7fcbf19077" - integrity sha512-5c99P1WKTed11ZC0HMJOj6CDIue6F8ySu+bJL+85q1zBEIY8IklrJ1eiKC2NDRh3Ct3FcvmJPyQHb9erXMTJNw== - dependencies: - "@jridgewell/sourcemap-codec" "^1.5.0" - magic-string@^0.30.17: version "0.30.17" resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.30.17.tgz#450a449673d2460e5bbcfba9a61916a1714c7453" @@ -11204,11 +11126,6 @@ path-type@^4.0.0: resolved "https://registry.yarnpkg.com/path-type/-/path-type-4.0.0.tgz#84ed01c0a7ba380afe09d90a8c180dcd9d03043b" integrity sha512-gDKb8aZMDeD/tZWs9P6+q0J9Mwkdl6xMV8TjnGP3qJVJ06bdMgkbBlLU8IdfOsIsFz2BW1rNVT3XuNEl8zPAvw== -pathe@^1.1.2: - version "1.1.2" - resolved "https://registry.yarnpkg.com/pathe/-/pathe-1.1.2.tgz#6c4cb47a945692e48a1ddd6e4094d170516437ec" - integrity sha512-whLdWMYL2TwI08hn8/ZqAbrVemu0LNaNNJZX73O6qaIdCTfXutsLhMkjdENX0qhsQ9uIimo4/aQOmXkoon2nDQ== - pathe@^2.0.1: version "2.0.2" resolved "https://registry.yarnpkg.com/pathe/-/pathe-2.0.2.tgz#5ed86644376915b3c7ee4d00ac8c348d671da3a5" @@ -13108,26 +13025,16 @@ tinybench@^2.9.0: resolved "https://registry.yarnpkg.com/tinybench/-/tinybench-2.9.0.tgz#103c9f8ba6d7237a47ab6dd1dcff77251863426b" integrity sha512-0+DUvqWMValLmha6lr4kD8iAMK1HzV0/aKnCtWb9v9641TnP/MFb7Pc2bxoxQjTXAErryXVgUOfv2YqNllqGeg== -tinyexec@^0.3.1: - version "0.3.1" - resolved "https://registry.yarnpkg.com/tinyexec/-/tinyexec-0.3.1.tgz#0ab0daf93b43e2c211212396bdb836b468c97c98" - integrity sha512-WiCJLEECkO18gwqIp6+hJg0//p23HXp4S+gGtAKu3mI2F2/sXC4FvHvXvB0zJVVaTPhx1/tOwdbRsa1sOBIKqQ== - tinyexec@^0.3.2: version "0.3.2" resolved "https://registry.yarnpkg.com/tinyexec/-/tinyexec-0.3.2.tgz#941794e657a85e496577995c6eef66f53f42b3d2" integrity sha512-KQQR9yN7R5+OSwaK0XQoj22pwHoTlgYqmUscPYoknOoWCWfj/5/ABTMRi69FrKU5ffPVh5QcFikpWJI/P1ocHA== -tinypool@^1.0.1, tinypool@^1.0.2: +tinypool@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/tinypool/-/tinypool-1.0.2.tgz#706193cc532f4c100f66aa00b01c42173d9051b2" integrity sha512-al6n+QEANGFOMf/dmUMsuS5/r9B06uwlyNjZZql/zv8J7ybHCgoihBNORZCY2mzUuAnomQa2JdhyHKzZxPCrFA== -tinyrainbow@^1.2.0: - version "1.2.0" - resolved "https://registry.yarnpkg.com/tinyrainbow/-/tinyrainbow-1.2.0.tgz#5c57d2fc0fb3d1afd78465c33ca885d04f02abb5" - integrity sha512-weEDEq7Z5eTHPDh4xjX789+fHfF+P8boiFB+0vbWzpbnbsEr/GRaohi/uMKxg8RZMXnl1ItAi/IUHWMsjDV7kQ== - tinyrainbow@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/tinyrainbow/-/tinyrainbow-2.0.0.tgz#9509b2162436315e80e3eee0fcce4474d2444294" @@ -13707,17 +13614,6 @@ version-guard@^1.1.1: resolved "https://registry.yarnpkg.com/version-guard/-/version-guard-1.1.1.tgz#7a6e87a1babff1b43d6a7b0fd239731e278262fa" integrity sha512-MGQLX89UxmYHgDvcXyjBI0cbmoW+t/dANDppNPrno64rYr8nH4SHSuElQuSYdXGEs0mUzdQe1BY+FhVPNsAmJQ== -vite-node@2.1.6: - version "2.1.6" - resolved "https://registry.yarnpkg.com/vite-node/-/vite-node-2.1.6.tgz#d7b79c5cde56c749f619dead049944918726b91e" - integrity sha512-DBfJY0n9JUwnyLxPSSUmEePT21j8JZp/sR9n+/gBwQU6DcQOioPdb8/pibWfXForbirSagZCilseYIwaL3f95A== - dependencies: - cac "^6.7.14" - debug "^4.3.7" - es-module-lexer "^1.5.4" - pathe "^1.1.2" - vite "^5.0.0 || ^6.0.0" - vite-node@3.0.3: version "3.0.3" resolved "https://registry.yarnpkg.com/vite-node/-/vite-node-3.0.3.tgz#2127458eae8c78b92f609f4c84d613599cd14317" @@ -13751,32 +13647,6 @@ vite@^5.0.12: optionalDependencies: fsevents "~2.3.3" -vitest@^2.1.6: - version "2.1.6" - resolved "https://registry.yarnpkg.com/vitest/-/vitest-2.1.6.tgz#44d661c6b3f3a3a0c597143f78d27215ee4666cc" - integrity sha512-isUCkvPL30J4c5O5hgONeFRsDmlw6kzFEdLQHLezmDdKQHy8Ke/B/dgdTMEgU0vm+iZ0TjW8GuK83DiahBoKWQ== - dependencies: - "@vitest/expect" "2.1.6" - "@vitest/mocker" "2.1.6" - "@vitest/pretty-format" "^2.1.6" - "@vitest/runner" "2.1.6" - "@vitest/snapshot" "2.1.6" - "@vitest/spy" "2.1.6" - "@vitest/utils" "2.1.6" - chai "^5.1.2" - debug "^4.3.7" - expect-type "^1.1.0" - magic-string "^0.30.12" - pathe "^1.1.2" - std-env "^3.8.0" - tinybench "^2.9.0" - tinyexec "^0.3.1" - tinypool "^1.0.1" - tinyrainbow "^1.2.0" - vite "^5.0.0 || ^6.0.0" - vite-node "2.1.6" - why-is-node-running "^2.3.0" - vitest@^3.0.3: version "3.0.3" resolved "https://registry.yarnpkg.com/vitest/-/vitest-3.0.3.tgz#e7bcf3ba82e4a18f1f2c5083b3d989cd344cb78c" From c70224d95edc90ee32071b573a3134e4a18cd25f Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Mon, 27 Jan 2025 21:32:49 -0800 Subject: [PATCH 03/30] pnpm support take 1 --- packages/api/cli/spec/check-system.spec.ts | 62 ++++++++++++++++--- packages/api/cli/src/util/check-system.ts | 56 +++++++---------- .../fast/util/install-dependencies.spec.ts | 23 ++++--- packages/api/core/spec/slow/api.slow.spec.ts | 12 ++-- packages/api/core/src/api/import.ts | 11 ++-- .../core/src/api/init-scripts/init-link.ts | 7 ++- packages/api/core/src/api/init.ts | 6 +- .../api/core/src/util/install-dependencies.ts | 13 ++-- .../spec/ViteTypeScriptTemplate.slow.spec.ts | 8 +-- .../spec/WebpackTypeScript.slow.spec.ts | 8 +-- .../core-utils/spec/package-manager.spec.ts | 15 +++-- .../utils/core-utils/src/package-manager.ts | 48 +++++++++++--- 12 files changed, 168 insertions(+), 101 deletions(-) diff --git a/packages/api/cli/spec/check-system.spec.ts b/packages/api/cli/spec/check-system.spec.ts index 6ec9a1d680..1e7c08bc7e 100644 --- a/packages/api/cli/spec/check-system.spec.ts +++ b/packages/api/cli/spec/check-system.spec.ts @@ -1,19 +1,61 @@ -import { describe, expect, it } from 'vitest'; +import { resolvePackageManager, spawnPackageManager } from '@electron-forge/core-utils'; +import { describe, expect, it, vi } from 'vitest'; -import { checkValidPackageManagerVersion } from '../src/util/check-system'; +import { checkPackageManagerVersion } from '../src/util/check-system'; -describe('check-system', () => { - describe('validPackageManagerVersion', () => { - it('should consider whitelisted versions to be valid', () => { - expect(() => checkValidPackageManagerVersion('NPM', '3.10.1', '^3.0.0')).not.toThrow(); +vi.mock(import('@electron-forge/core-utils'), async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + resolvePackageManager: vi.fn(), + spawnPackageManager: vi.fn(), + }; +}); + +describe('checkPackageManagerVersion', () => { + it('should consider allowlisted versions to be valid', async () => { + vi.mocked(resolvePackageManager).mockResolvedValue({ + executable: 'npm', + install: 'install', + dev: '--save-dev', + exact: '--save-exact', + }); + vi.mocked(spawnPackageManager).mockResolvedValue('10.9.2'); + await expect(checkPackageManagerVersion()).resolves.not.toThrow(); + }); + + it('rejects versions that are outside of the supported range', async () => { + vi.mocked(resolvePackageManager).mockResolvedValue({ + executable: 'yarn', + install: 'add', + dev: '--dev', + exact: '--exact', }); - it('should consider Yarn nightly versions to be invalid', () => { - expect(() => checkValidPackageManagerVersion('Yarn', '0.23.0-20170311.0515', '0.23.0')).toThrow(); + // yarn 0.x unsupported + vi.mocked(spawnPackageManager).mockResolvedValue('0.22.0'); + await expect(checkPackageManagerVersion()).rejects.toThrow(); + }); + + it('should consider Yarn nightly versions to be invalid', async () => { + vi.mocked(resolvePackageManager).mockResolvedValue({ + executable: 'yarn', + install: 'add', + dev: '--dev', + exact: '--exact', }); + vi.mocked(spawnPackageManager).mockResolvedValue('0.23.0-20170311.0515'); + await expect(checkPackageManagerVersion()).rejects.toThrow(); + }); - it('should consider invalid semver versions to be invalid', () => { - expect(() => checkValidPackageManagerVersion('Yarn', '0.22', '0.22.0')).toThrow(); + it('should consider invalid semver versions to be invalid', async () => { + vi.mocked(resolvePackageManager).mockResolvedValue({ + executable: 'yarn', + install: 'add', + dev: '--dev', + exact: '--exact', }); + vi.mocked(spawnPackageManager).mockResolvedValue('1.22'); + await expect(checkPackageManagerVersion()).rejects.toThrow(); }); }); diff --git a/packages/api/cli/src/util/check-system.ts b/packages/api/cli/src/util/check-system.ts index fab3685b0f..6ef5109531 100644 --- a/packages/api/cli/src/util/check-system.ts +++ b/packages/api/cli/src/util/check-system.ts @@ -2,7 +2,7 @@ import { exec } from 'node:child_process'; import os from 'node:os'; import path from 'node:path'; -import { resolvePackageManager, spawnPackageManager } from '@electron-forge/core-utils'; +import { resolvePackageManager, spawnPackageManager, SupportedPackageManager } from '@electron-forge/core-utils'; import { ForgeListrTask } from '@electron-forge/shared-types'; import debug from 'debug'; import fs from 'fs-extra'; @@ -27,43 +27,35 @@ async function checkNodeVersion() { return process.versions.node; } -const NPM_ALLOWLISTED_VERSIONS = { - all: '^3.0.0 || ^4.0.0 || ~5.1.0 || ~5.2.0 || >= 5.4.2', - darwin: '>= 5.4.0', - linux: '>= 5.4.0', -}; -const YARN_ALLOWLISTED_VERSIONS = { - all: '>= 1.0.0', +const ALLOWLISTED_VERSIONS: Record> = { + npm: { + all: '^3.0.0 || ^4.0.0 || ~5.1.0 || ~5.2.0 || >= 5.4.2', + darwin: '>= 5.4.0', + linux: '>= 5.4.0', + }, + yarn: { + all: '>= 1.0.0', + }, + pnpm: { + all: '>= 8.0.0', + }, }; -export function checkValidPackageManagerVersion(packageManager: string, version: string, allowlistedVersions: string) { +export async function checkPackageManagerVersion() { + const version = await spawnPackageManager(['--version']); + const versionString = version.toString().trim(); + const pm = await resolvePackageManager(); + + const range = ALLOWLISTED_VERSIONS[pm.executable][process.platform] ?? ALLOWLISTED_VERSIONS[pm.executable].all; if (!semver.valid(version)) { d(`Invalid semver-string while checking version: ${version}`); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - throw new Error(`Could not check ${packageManager} version "${version}", assuming incompatible`); + throw new Error(`Could not check ${pm.executable} version "${version}", assuming incompatible`); } - if (!semver.satisfies(version, allowlistedVersions)) { - throw new Error(`Incompatible version of ${packageManager} detected "${version}", must be in range ${allowlistedVersions}`); + if (!semver.satisfies(version, range)) { + throw new Error(`Incompatible version of ${pm.executable} detected: "${version}" must be in range ${range}`); } -} -function warnIfPackageManagerIsntAKnownGoodVersion(packageManager: string, version: string, allowlistedVersions: { [key: string]: string }) { - const osVersions = allowlistedVersions[process.platform]; - const versions = osVersions ? `${allowlistedVersions.all} || ${osVersions}` : allowlistedVersions.all; - const versionString = version.toString(); - checkValidPackageManagerVersion(packageManager, versionString, versions); -} - -async function checkPackageManagerVersion() { - const version = await spawnPackageManager(['--version']); - const versionString = version.toString().trim(); - if ((await resolvePackageManager()) === 'yarn') { - warnIfPackageManagerIsntAKnownGoodVersion('Yarn', versionString, YARN_ALLOWLISTED_VERSIONS); - return `yarn@${versionString}`; - } else { - warnIfPackageManagerIsntAKnownGoodVersion('NPM', versionString, NPM_ALLOWLISTED_VERSIONS); - return `npm@${versionString}`; - } + return `${pm.executable}@${versionString}`; } /** @@ -106,7 +98,7 @@ export async function checkSystem(task: ForgeListrTask) { }, }, { - title: 'Checking packageManager version', + title: 'Checking package manager version', task: async (_, task) => { const packageManager = await checkPackageManagerVersion(); task.title = `Found ${packageManager}`; diff --git a/packages/api/core/spec/fast/util/install-dependencies.spec.ts b/packages/api/core/spec/fast/util/install-dependencies.spec.ts index 21249e891b..057238192d 100644 --- a/packages/api/core/spec/fast/util/install-dependencies.spec.ts +++ b/packages/api/core/spec/fast/util/install-dependencies.spec.ts @@ -18,42 +18,45 @@ describe('installDependencies', () => { expect(spawnPackageManager).not.toHaveBeenCalled(); }); - it('should reject if reject the promise if exit code is not 0', async () => { + it('should reject if the package manager fails to spawn', async () => { + vi.mocked(resolvePackageManager).mockResolvedValue({ executable: 'npm', install: 'install', dev: '--save-dev', exact: '--save-exact' }); vi.mocked(spawnPackageManager).mockRejectedValueOnce('fail'); await expect(installDependencies('void', ['electron'])).rejects.toThrow('fail'); }); - it('should resolve if reject the promise if exit code is 0', async () => { + it('should resolve if the package manager command succeeds', async () => { + vi.mocked(resolvePackageManager).mockResolvedValue({ executable: 'npm', install: 'install', dev: '--save-dev', exact: '--save-exact' }); vi.mocked(spawnPackageManager).mockResolvedValueOnce('pass'); await expect(installDependencies('void', ['electron'])).resolves.toBe(undefined); }); describe.each([ - { pm: 'npm' as const, install: 'install', flags: { exact: '--save-exact', dev: '--save-dev' } }, - { pm: 'yarn' as const, install: 'add', flags: { exact: '--exact', dev: '--dev' } }, - ])('$pm', ({ pm, install, flags }) => { + { executable: 'npm' as const, install: 'install', exact: '--save-exact', dev: '--save-dev' }, + { executable: 'yarn' as const, install: 'add', exact: '--exact', dev: '--dev' }, + { executable: 'pnpm' as const, install: 'install', exact: '--save-exact', dev: '--save-dev' }, + ])('$executable', (args) => { beforeEach(() => { - vi.mocked(resolvePackageManager).mockResolvedValue(pm); + vi.mocked(resolvePackageManager).mockResolvedValue(args); }); it('should install deps', async () => { await installDependencies('mydir', ['react']); - expect(spawnPackageManager).toHaveBeenCalledWith([install, 'react'], expect.anything()); + expect(spawnPackageManager).toHaveBeenCalledWith([args.install, 'react'], expect.anything()); }); it('should install dev deps', async () => { await installDependencies('mydir', ['eslint'], DepType.DEV); - expect(spawnPackageManager).toHaveBeenCalledWith([install, 'eslint', flags.dev], expect.anything()); + expect(spawnPackageManager).toHaveBeenCalledWith([args.install, 'eslint', args.dev], expect.anything()); }); it('should install exact deps', async () => { await installDependencies('mydir', ['react'], DepType.PROD, DepVersionRestriction.EXACT); - expect(spawnPackageManager).toHaveBeenCalledWith([install, 'react', flags.exact], expect.anything()); + expect(spawnPackageManager).toHaveBeenCalledWith([args.install, 'react', args.exact], expect.anything()); }); it('should install exact dev deps', async () => { await installDependencies('mydir', ['eslint'], DepType.DEV, DepVersionRestriction.EXACT); - expect(spawnPackageManager).toHaveBeenCalledWith([install, 'eslint', flags.dev, flags.exact], expect.anything()); + expect(spawnPackageManager).toHaveBeenCalledWith([args.install, 'eslint', args.dev, args.exact], expect.anything()); }); }); }); diff --git a/packages/api/core/spec/slow/api.slow.spec.ts b/packages/api/core/spec/slow/api.slow.spec.ts index 22a5689d45..80711e5a6b 100644 --- a/packages/api/core/spec/slow/api.slow.spec.ts +++ b/packages/api/core/spec/slow/api.slow.spec.ts @@ -3,7 +3,7 @@ import { execSync } from 'node:child_process'; import fs from 'node:fs'; import path from 'node:path'; -import { yarnOrNpmSpawn } from '@electron-forge/core-utils'; +import { spawnPackageManager } from '@electron-forge/core-utils'; import { createDefaultCertificate } from '@electron-forge/maker-appx'; import { ForgeConfig, IForgeResolvableMaker } from '@electron-forge/shared-types'; import { ensureTestDirIsNonexistent, expectLintToPass } from '@electron-forge/test-utils'; @@ -29,12 +29,12 @@ async function updatePackageJSON(dir: string, packageJSONUpdater: (packageJSON: await fs.promises.writeFile(path.resolve(dir, 'package.json'), JSON.stringify(packageJSON), 'utf-8'); } -describe.each([{ installer: 'npm' }, { installer: 'yarn' }])(`init (with $installer)`, ({ installer }) => { +describe.each([{ installer: 'npm' }, { installer: 'npm' }, { installer: 'pnpm' }])(`init (with $installer)`, ({ installer }) => { let dir: string; beforeAll(async () => { - await yarnOrNpmSpawn(['run', 'link:prepare']); process.env.NODE_INSTALLER = installer; + await spawnPackageManager(['run', 'link:prepare']); }); const beforeInitTest = (params?: Partial, beforeInit?: BeforeInitFunction) => { @@ -213,7 +213,7 @@ describe.each([{ installer: 'npm' }, { installer: 'yarn' }])(`init (with $instal }); afterAll(async () => { - await yarnOrNpmSpawn(['run', 'link:remove']); + await spawnPackageManager(['run', 'link:remove']); }); }); @@ -221,7 +221,7 @@ describe('Electron Forge API', () => { let dir: string; beforeAll(async () => { - await yarnOrNpmSpawn(['run', 'link:prepare']); + await spawnPackageManager(['run', 'link:prepare']); }); describe('after init', () => { @@ -487,6 +487,6 @@ describe('Electron Forge API', () => { }); afterAll(async () => { - await yarnOrNpmSpawn(['link:remove']); + await spawnPackageManager(['link:remove']); }); }); diff --git a/packages/api/core/src/api/import.ts b/packages/api/core/src/api/import.ts index 31c491f7bd..bbcc564af5 100644 --- a/packages/api/core/src/api/import.ts +++ b/packages/api/core/src/api/import.ts @@ -193,23 +193,24 @@ export default autoTrace( { title: 'Installing dependencies', task: async (_, task) => { - const packageManager = await resolvePackageManager(); + const pm = await resolvePackageManager(); await writeChanges(); d('deleting old dependencies forcefully'); await fs.remove(path.resolve(dir, 'node_modules/.bin/electron')); await fs.remove(path.resolve(dir, 'node_modules/.bin/electron.cmd')); + // FIXME(erickzhao): these flags should be package-manager-specific. d('installing dependencies'); - task.output = `${packageManager} install ${importDeps.join(' ')}`; + task.output = `${pm.executable} ${pm.install} ${importDeps.join(' ')}`; await installDepList(dir, importDeps); d('installing devDependencies'); - task.output = `${packageManager} install --dev ${importDevDeps.join(' ')}`; + task.output = `${pm} ${pm.install} ${pm.dev} ${importDevDeps.join(' ')}`; await installDepList(dir, importDevDeps, DepType.DEV); - d('installing exactDevDependencies'); - task.output = `${packageManager} install --dev --exact ${importExactDevDeps.join(' ')}`; + d('installing devDependencies with exact versions'); + task.output = `${pm} ${pm.install} ${pm.dev} ${pm.exact} ${importExactDevDeps.join(' ')}`; await installDepList(dir, importExactDevDeps, DepType.DEV, DepVersionRestriction.EXACT); }, }, diff --git a/packages/api/core/src/api/init-scripts/init-link.ts b/packages/api/core/src/api/init-scripts/init-link.ts index 8a6fe8b434..ec1e1805b2 100644 --- a/packages/api/core/src/api/init-scripts/init-link.ts +++ b/packages/api/core/src/api/init-scripts/init-link.ts @@ -22,11 +22,14 @@ export async function initLink(dir: string, task?: ForgeListrTask) { if (shouldLink) { d('Linking forge dependencies'); const packageJson = await readRawPackageJson(dir); - const packageManager = await resolvePackageManager(); + const pm = await resolvePackageManager(); + // TODO(erickzhao): the `--link-folder` argument only works for `yarn`. Since this command is + // only made for Forge contributors, it isn't a big deal if it doesn't work for other package managers, + // but we should make it cleaner. const linkFolder = path.resolve(__dirname, '..', '..', '..', '..', '..', '..', '.links'); for (const packageName of Object.keys(packageJson.devDependencies)) { if (packageName.startsWith('@electron-forge/')) { - if (task) task.output = `${packageManager} link --link-folder ${linkFolder} ${packageName}`; + if (task) task.output = `${pm.executable} link --link-folder ${linkFolder} ${packageName}`; await spawnPackageManager(['link', '--link-folder', linkFolder, packageName], { cwd: dir, }); diff --git a/packages/api/core/src/api/init.ts b/packages/api/core/src/api/init.ts index 6e8529af7c..4cd510f856 100644 --- a/packages/api/core/src/api/init.ts +++ b/packages/api/core/src/api/init.ts @@ -56,7 +56,7 @@ async function validateTemplate(template: string, templateModule: ForgeTemplate) export default async ({ dir = process.cwd(), interactive = false, copyCIFiles = false, force = false, template = 'base' }: InitOptions): Promise => { d(`Initializing in: ${dir}`); - const packageManager = await resolvePackageManager(); + const pm = await resolvePackageManager(); const runner = new Listr<{ templateModule: ForgeTemplate; @@ -103,7 +103,7 @@ export default async ({ dir = process.cwd(), interactive = false, copyCIFiles = task: async (_, task) => { d('installing dependencies'); if (templateModule.dependencies?.length) { - task.output = `${packageManager} install ${templateModule.dependencies.join(' ')}`; + task.output = `${pm.executable} ${pm.install} ${pm.dev} ${templateModule.dependencies.join(' ')}`; } return await installDepList(dir, templateModule.dependencies || [], DepType.PROD, DepVersionRestriction.RANGE); }, @@ -114,7 +114,7 @@ export default async ({ dir = process.cwd(), interactive = false, copyCIFiles = task: async (_, task) => { d('installing devDependencies'); if (templateModule.devDependencies?.length) { - task.output = `${packageManager} install --dev ${templateModule.devDependencies.join(' ')}`; + task.output = `${pm.executable} ${pm.install} ${pm.dev} ${templateModule.devDependencies.join(' ')}`; } await installDepList(dir, templateModule.devDependencies || [], DepType.DEV); }, diff --git a/packages/api/core/src/util/install-dependencies.ts b/packages/api/core/src/util/install-dependencies.ts index 81896b6c58..01c00ca90c 100644 --- a/packages/api/core/src/util/install-dependencies.ts +++ b/packages/api/core/src/util/install-dependencies.ts @@ -21,15 +21,10 @@ export default async (dir: string, deps: string[], depType = DepType.PROD, versi d('nothing to install, stopping immediately'); return Promise.resolve(); } - let cmd = ['install'].concat(deps); - if (pm === 'yarn') { - cmd = ['add'].concat(deps); - if (depType === DepType.DEV) cmd.push('--dev'); - if (versionRestriction === DepVersionRestriction.EXACT) cmd.push('--exact'); - } else { - if (depType === DepType.DEV) cmd.push('--save-dev'); - if (versionRestriction === DepVersionRestriction.EXACT) cmd.push('--save-exact'); - } + const cmd = [pm.install].concat(deps); + if (depType === DepType.DEV) cmd.push(pm.dev); + if (versionRestriction === DepVersionRestriction.EXACT) cmd.push(pm.exact); + d('executing', JSON.stringify(cmd), 'in:', dir); try { await spawnPackageManager(cmd, { diff --git a/packages/template/vite-typescript/spec/ViteTypeScriptTemplate.slow.spec.ts b/packages/template/vite-typescript/spec/ViteTypeScriptTemplate.slow.spec.ts index da8912de7d..9dd6aaa27c 100644 --- a/packages/template/vite-typescript/spec/ViteTypeScriptTemplate.slow.spec.ts +++ b/packages/template/vite-typescript/spec/ViteTypeScriptTemplate.slow.spec.ts @@ -2,7 +2,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { yarnOrNpmSpawn } from '@electron-forge/core-utils'; +import { spawnPackageManager } from '@electron-forge/core-utils'; import testUtils from '@electron-forge/test-utils'; import glob from 'fast-glob'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; @@ -15,12 +15,12 @@ describe('ViteTypeScriptTemplate', () => { let dir: string; beforeAll(async () => { - await yarnOrNpmSpawn(['run', 'link:prepare']); + await spawnPackageManager(['run', 'link:prepare']); dir = await testUtils.ensureTestDirIsNonexistent(); }); afterAll(async () => { - await yarnOrNpmSpawn(['run', 'link:remove']); + await spawnPackageManager(['run', 'link:remove']); if (os.platform() !== 'win32') { // Windows platform `fs.remove(dir)` logic using `npm run test:clear`. await fs.promises.rm(dir, { force: true, recursive: true }); @@ -81,7 +81,7 @@ describe('ViteTypeScriptTemplate', () => { vite: `${require('../../../../node_modules/vite/package.json').version}`, }; await fs.promises.writeFile(path.resolve(dir, 'package.json'), JSON.stringify(pj)); - await yarnOrNpmSpawn(['install'], { + await spawnPackageManager(['install'], { cwd: dir, }); diff --git a/packages/template/webpack-typescript/spec/WebpackTypeScript.slow.spec.ts b/packages/template/webpack-typescript/spec/WebpackTypeScript.slow.spec.ts index 1ebb10b7dc..200dc029e4 100644 --- a/packages/template/webpack-typescript/spec/WebpackTypeScript.slow.spec.ts +++ b/packages/template/webpack-typescript/spec/WebpackTypeScript.slow.spec.ts @@ -1,7 +1,7 @@ import fs from 'node:fs'; import path from 'node:path'; -import { yarnOrNpmSpawn } from '@electron-forge/core-utils'; +import { spawnPackageManager } from '@electron-forge/core-utils'; import testUtils from '@electron-forge/test-utils'; import glob from 'fast-glob'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; @@ -14,7 +14,7 @@ describe('WebpackTypeScriptTemplate', () => { let dir: string; beforeAll(async () => { - await yarnOrNpmSpawn(['run', 'link:prepare']); + await spawnPackageManager(['run', 'link:prepare']); dir = await testUtils.ensureTestDirIsNonexistent(); }); @@ -71,7 +71,7 @@ describe('WebpackTypeScriptTemplate', () => { webpack: `${require('../../../../node_modules/webpack/package.json').version}`, }; await fs.promises.writeFile(path.resolve(dir, 'package.json'), JSON.stringify(pj)); - await yarnOrNpmSpawn(['install'], { + await spawnPackageManager(['install'], { cwd: dir, }); @@ -94,7 +94,7 @@ describe('WebpackTypeScriptTemplate', () => { }); afterAll(async () => { - await yarnOrNpmSpawn(['link:remove']); + await spawnPackageManager(['link:remove']); await fs.promises.rm(dir, { recursive: true, force: true }); }); }); diff --git a/packages/utils/core-utils/spec/package-manager.spec.ts b/packages/utils/core-utils/spec/package-manager.spec.ts index c4c0df21ed..91b8c282c4 100644 --- a/packages/utils/core-utils/spec/package-manager.spec.ts +++ b/packages/utils/core-utils/spec/package-manager.spec.ts @@ -21,24 +21,23 @@ describe('package-manager', () => { it('should by default equal the system package manager value', async () => { const pm = await detect(); - await expect(resolvePackageManager()).resolves.toEqual(pm); + await expect(resolvePackageManager()).resolves.toHaveProperty('executable', pm); }); it('should return yarn if NODE_INSTALLER=yarn', async () => { process.env.NODE_INSTALLER = 'yarn'; - await expect(resolvePackageManager()).resolves.toEqual('yarn'); + await expect(resolvePackageManager()).resolves.toHaveProperty('executable', 'yarn'); }); it('should return npm if NODE_INSTALLER=npm', async () => { process.env.NODE_INSTALLER = 'npm'; - await expect(resolvePackageManager()).resolves.toEqual('npm'); + await expect(resolvePackageManager()).resolves.toHaveProperty('executable', 'npm'); }); - it('should return system value if NODE_INSTALLER is an unrecognized installer', async () => { - process.env.NODE_INSTALLER = 'magical_unicorn'; + it('should return npm if package manager is unsupported', async () => { + process.env.NODE_INSTALLER = 'bun'; console.warn = vi.fn(); - const pm = await detect(); - await expect(resolvePackageManager()).resolves.toEqual(pm); - expect(console.warn).toHaveBeenCalledWith('⚠', expect.stringContaining('Unknown NODE_INSTALLER')); + await expect(resolvePackageManager()).resolves.toHaveProperty('executable', 'npm'); + expect(console.warn).toHaveBeenCalledWith('⚠', expect.stringContaining('Package manager bun is unsupported')); }); }); diff --git a/packages/utils/core-utils/src/package-manager.ts b/packages/utils/core-utils/src/package-manager.ts index b5741ebceb..1d33d1b791 100644 --- a/packages/utils/core-utils/src/package-manager.ts +++ b/packages/utils/core-utils/src/package-manager.ts @@ -3,18 +3,50 @@ import chalk from 'chalk'; import { detect } from 'detect-package-manager'; import logSymbols from 'log-symbols'; -export const resolvePackageManager = async () => { +export type SupportedPackageManager = 'yarn' | 'npm' | 'pnpm'; +export type PMDetails = { executable: SupportedPackageManager; install: string; dev: string; exact: string }; + +const MANAGERS: Record = { + yarn: { + executable: 'yarn', + install: 'add', + dev: '--dev', + exact: '--exact', + }, + npm: { + executable: 'npm', + install: 'install', + dev: '--save-dev', + exact: '--save-exact', + }, + pnpm: { + executable: 'pnpm', + install: 'add', + dev: '--save-dev', + exact: '--save-exact', + }, +}; + +/** + * Resolves the package manager to use. Prioritizes the `NODE_INSTALLER` environment variable. + * Supported package managers are `yarn`, `pnpm`, and `npm`. + * + * If an unknown package manager is used, then a warning is logged and we fall back to `npm`. + */ +export const resolvePackageManager: () => Promise = async () => { const system = await detect(); - switch (process.env.NODE_INSTALLER) { + const installer = process.env.NODE_INSTALLER || system; + + switch (installer) { case 'yarn': case 'npm': - return process.env.NODE_INSTALLER; + case 'pnpm': + return MANAGERS[installer]; default: - if (process.env.NODE_INSTALLER) { - console.warn(logSymbols.warning, chalk.yellow(`Unknown NODE_INSTALLER, using detected installer ${system}`)); - } - return system; + console.warn(logSymbols.warning, chalk.yellow(`Package manager ${chalk.red(installer)} is unsupported. Falling back to ${chalk.green('npm')} instead.`)); + return MANAGERS['npm']; } }; -export const spawnPackageManager = async (args?: CrossSpawnArgs, opts?: CrossSpawnOptions): Promise => spawn(await resolvePackageManager(), args, opts); +export const spawnPackageManager = async (args?: CrossSpawnArgs, opts?: CrossSpawnOptions): Promise => + spawn((await resolvePackageManager()).executable, args, opts); From 0acb1f372c7d5bec045904a2192d2404fe4f12c4 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Mon, 27 Jan 2025 21:39:33 -0800 Subject: [PATCH 04/30] install pnpm in CI --- .circleci/config.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 648d6723cf..5b8f081bd9 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -110,6 +110,10 @@ jobs: libgtk-3-0 \ libgbm1 sudo add-apt-repository -y ppa:alexlarsson/flatpak + - run: + name: 'Install pnpm' + command: | + npm install -g pnpm@10.0.0 - run-fast-tests - store_test_results: path: ./reports/ @@ -155,6 +159,10 @@ jobs: libgdk-pixbuf2.0-dev \ libgtk-3-0 \ libgbm1 + - run: + name: 'Install pnpm' + command: | + npm install -g pnpm@10.0.0 - run-slow-tests - run: when: always # the report is generated on pass or fail From 284e21957b3452e367dcd9cbd921d8621d4c7036 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Mon, 27 Jan 2025 23:03:16 -0800 Subject: [PATCH 05/30] reverse order of link:prepare --- packages/api/core/spec/slow/api.slow.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/core/spec/slow/api.slow.spec.ts b/packages/api/core/spec/slow/api.slow.spec.ts index 80711e5a6b..f9802418f9 100644 --- a/packages/api/core/spec/slow/api.slow.spec.ts +++ b/packages/api/core/spec/slow/api.slow.spec.ts @@ -33,8 +33,8 @@ describe.each([{ installer: 'npm' }, { installer: 'npm' }, { installer: 'pnpm' } let dir: string; beforeAll(async () => { - process.env.NODE_INSTALLER = installer; await spawnPackageManager(['run', 'link:prepare']); + process.env.NODE_INSTALLER = installer; }); const beforeInitTest = (params?: Partial, beforeInit?: BeforeInitFunction) => { From 1d85d64f8f0fba338bed63df6e088c6dee243d70 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 28 Jan 2025 10:03:22 -0800 Subject: [PATCH 06/30] attempt to remove corepack for now --- package.json | 1 - packages/api/core/spec/slow/api.slow.spec.ts | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index dcfb0170b0..64ec38d751 100644 --- a/package.json +++ b/package.json @@ -151,7 +151,6 @@ "prettier": { "singleQuote": true }, - "packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e", "workspaces": { "packages": [ "packages/api/*", diff --git a/packages/api/core/spec/slow/api.slow.spec.ts b/packages/api/core/spec/slow/api.slow.spec.ts index f9802418f9..2096b7db54 100644 --- a/packages/api/core/spec/slow/api.slow.spec.ts +++ b/packages/api/core/spec/slow/api.slow.spec.ts @@ -29,12 +29,12 @@ async function updatePackageJSON(dir: string, packageJSONUpdater: (packageJSON: await fs.promises.writeFile(path.resolve(dir, 'package.json'), JSON.stringify(packageJSON), 'utf-8'); } -describe.each([{ installer: 'npm' }, { installer: 'npm' }, { installer: 'pnpm' }])(`init (with $installer)`, ({ installer }) => { +describe.each([{ pm: 'npm' }, { pm: 'npm' }, { pm: 'pnpm' }])(`init (with $installer)`, ({ pm }) => { let dir: string; beforeAll(async () => { await spawnPackageManager(['run', 'link:prepare']); - process.env.NODE_INSTALLER = installer; + process.env.NODE_INSTALLER = pm; }); const beforeInitTest = (params?: Partial, beforeInit?: BeforeInitFunction) => { @@ -196,7 +196,7 @@ describe.each([{ installer: 'npm' }, { installer: 'npm' }, { installer: 'pnpm' } expect(fs.existsSync(path.join(dir, 'forge.config.js'))).toEqual(true); - execSync(`${installer} install`, { + execSync(`${pm} install`, { cwd: dir, }); From 431116d8fa809046ccf8d4b9adb5ff0227ed028d Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 28 Jan 2025 12:10:36 -0800 Subject: [PATCH 07/30] hoist it! --- packages/api/core/spec/slow/api.slow.spec.ts | 22 +++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/api/core/spec/slow/api.slow.spec.ts b/packages/api/core/spec/slow/api.slow.spec.ts index 2096b7db54..50461a7543 100644 --- a/packages/api/core/spec/slow/api.slow.spec.ts +++ b/packages/api/core/spec/slow/api.slow.spec.ts @@ -35,6 +35,18 @@ describe.each([{ pm: 'npm' }, { pm: 'npm' }, { pm: 'pnpm' }])(`init (with $insta beforeAll(async () => { await spawnPackageManager(['run', 'link:prepare']); process.env.NODE_INSTALLER = pm; + + if (pm === 'pnpm') { + await spawnPackageManager('config set node-linker hoisted'.split(' ')); + } + }); + + afterAll(async () => { + await spawnPackageManager(['run', 'link:remove']); + + if (pm === 'pnpm') { + await spawnPackageManager('config delete node-linker'.split(' ')); + } }); const beforeInitTest = (params?: Partial, beforeInit?: BeforeInitFunction) => { @@ -50,6 +62,8 @@ describe.each([{ pm: 'npm' }, { pm: 'npm' }, { pm: 'pnpm' }])(`init (with $insta describe('init', () => { beforeInitTest(); + afterAll(() => fs.promises.rm(dir, { recursive: true, force: true })); + it('should fail in initializing an already initialized directory', async () => { await expect(api.init({ dir })).rejects.toThrow( `The specified path: "${dir}" is not empty. Please ensure it is empty before initializing a new project` @@ -76,8 +90,6 @@ describe.each([{ pm: 'npm' }, { pm: 'npm' }, { pm: 'pnpm' }])(`init (with $insta describe('lint', () => { it('should initially pass the linting process', () => expectLintToPass(dir)); }); - - afterAll(() => fs.promises.rm(dir, { recursive: true, force: true })); }); describe.skip('init with CI files enabled', () => { @@ -186,7 +198,7 @@ describe.each([{ pm: 'npm' }, { pm: 'npm' }, { pm: 'pnpm' }])(`init (with $insta }); }); - it('creates forge.config.js and is packageable', async () => { + it('creates forge.config.js and can successfully package the application', async () => { await updatePackageJSON(dir, async (packageJSON) => { packageJSON.name = 'Name'; packageJSON.productName = 'ProductName'; @@ -211,10 +223,6 @@ describe.each([{ pm: 'npm' }, { pm: 'npm' }, { pm: 'pnpm' }])(`init (with $insta await fs.promises.rm(dir, { recursive: true, force: true }); }); }); - - afterAll(async () => { - await spawnPackageManager(['run', 'link:remove']); - }); }); describe('Electron Forge API', () => { From cafacc3a83c0f56dca801e6fe98eb2032b25ace6 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 28 Jan 2025 12:21:47 -0800 Subject: [PATCH 08/30] downgrade to pnpm 9 --- .circleci/config.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 5b8f081bd9..4165ef5d69 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -113,7 +113,8 @@ jobs: - run: name: 'Install pnpm' command: | - npm install -g pnpm@10.0.0 + # FIXME: upgrade to pnpm 10 and add Electron to allowlist for postinstall scripts + npm install -g pnpm@9.15.4 - run-fast-tests - store_test_results: path: ./reports/ From a19a6eaccce5461e3a81531c9e98c35daaec4979 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 28 Jan 2025 13:54:47 -0800 Subject: [PATCH 09/30] adjustments --- packages/api/core/spec/slow/api.slow.spec.ts | 454 +++++++++---------- packages/template/base/package.json | 1 + packages/template/base/src/BaseTemplate.ts | 10 + 3 files changed, 232 insertions(+), 233 deletions(-) diff --git a/packages/api/core/spec/slow/api.slow.spec.ts b/packages/api/core/spec/slow/api.slow.spec.ts index 50461a7543..dc63da365f 100644 --- a/packages/api/core/spec/slow/api.slow.spec.ts +++ b/packages/api/core/spec/slow/api.slow.spec.ts @@ -29,7 +29,7 @@ async function updatePackageJSON(dir: string, packageJSONUpdater: (packageJSON: await fs.promises.writeFile(path.resolve(dir, 'package.json'), JSON.stringify(packageJSON), 'utf-8'); } -describe.each([{ pm: 'npm' }, { pm: 'npm' }, { pm: 'pnpm' }])(`init (with $installer)`, ({ pm }) => { +describe.each([{ pm: 'npm' }, { pm: 'yarn' }, { pm: 'pnpm' }])(`init (with $pm)`, ({ pm }) => { let dir: string; beforeAll(async () => { @@ -43,10 +43,6 @@ describe.each([{ pm: 'npm' }, { pm: 'npm' }, { pm: 'pnpm' }])(`init (with $insta afterAll(async () => { await spawnPackageManager(['run', 'link:remove']); - - if (pm === 'pnpm') { - await spawnPackageManager('config delete node-linker'.split(' ')); - } }); const beforeInitTest = (params?: Partial, beforeInit?: BeforeInitFunction) => { @@ -223,278 +219,270 @@ describe.each([{ pm: 'npm' }, { pm: 'npm' }, { pm: 'pnpm' }])(`init (with $insta await fs.promises.rm(dir, { recursive: true, force: true }); }); }); -}); - -describe('Electron Forge API', () => { - let dir: string; - beforeAll(async () => { - await spawnPackageManager(['run', 'link:prepare']); - }); + describe('Electron Forge API', () => { + let dir: string; - describe('after init', () => { - let devCert: string; + describe('after init', () => { + let devCert: string; - beforeAll(async () => { - dir = path.join(await ensureTestDirIsNonexistent(), 'electron-forge-test'); - await api.init({ dir }); + beforeAll(async () => { + dir = path.join(await ensureTestDirIsNonexistent(), 'electron-forge-test'); + await api.init({ dir }); - await updatePackageJSON(dir, async (packageJSON) => { - packageJSON.name = 'testapp'; - packageJSON.version = '1.0.0-beta.1'; - packageJSON.productName = 'Test-App'; - packageJSON.config = packageJSON.config || {}; - packageJSON.config.forge = { - ...packageJSON.config.forge, - packagerConfig: { - asar: false, - }, - }; - if (process.platform === 'win32') { - await fs.promises.rename(path.join(__dirname, '..', 'fixture', 'bogus-private-key.pvk'), path.join(dir, 'default.pvk')); - devCert = await createDefaultCertificate('CN=Test Author', { certFilePath: dir }); - } else if (process.platform === 'linux') { - packageJSON.config.forge.packagerConfig = { - ...packageJSON.config.forge.packagerConfig, - executableName: 'testapp', + await updatePackageJSON(dir, async (packageJSON) => { + packageJSON.name = 'testapp'; + packageJSON.version = '1.0.0-beta.1'; + packageJSON.productName = 'Test-App'; + packageJSON.config = packageJSON.config || {}; + packageJSON.config.forge = { + ...packageJSON.config.forge, + packagerConfig: { + asar: false, + }, }; - } - packageJSON.homepage = 'http://www.example.com/'; - packageJSON.author = 'Test Author'; + if (process.platform === 'win32') { + await fs.promises.rename(path.join(__dirname, '..', 'fixture', 'bogus-private-key.pvk'), path.join(dir, 'default.pvk')); + devCert = await createDefaultCertificate('CN=Test Author', { certFilePath: dir }); + } else if (process.platform === 'linux') { + packageJSON.config.forge.packagerConfig = { + ...packageJSON.config.forge.packagerConfig, + executableName: 'testapp', + }; + } + packageJSON.homepage = 'http://www.example.com/'; + packageJSON.author = 'Test Author'; + }); }); - }); - it('throws an error when all is set', async () => { - await updatePackageJSON(dir, async (packageJSON) => { - assert(packageJSON.config.forge.packagerConfig); - packageJSON.config.forge.packagerConfig.all = true; - }); - await expect(api.package({ dir })).rejects.toThrow(/packagerConfig\.all is not supported by Electron Forge/); - await updatePackageJSON(dir, async (packageJSON) => { - assert(packageJSON.config.forge.packagerConfig); - delete packageJSON.config.forge.packagerConfig.all; + it('throws an error when all is set', async () => { + await updatePackageJSON(dir, async (packageJSON) => { + assert(packageJSON.config.forge.packagerConfig); + packageJSON.config.forge.packagerConfig.all = true; + }); + await expect(api.package({ dir })).rejects.toThrow(/packagerConfig\.all is not supported by Electron Forge/); + await updatePackageJSON(dir, async (packageJSON) => { + assert(packageJSON.config.forge.packagerConfig); + delete packageJSON.config.forge.packagerConfig.all; + }); }); - }); - it('can package to outDir without errors', async () => { - const outDir = `${dir}/foo`; + it('can package to outDir without errors', async () => { + const outDir = `${dir}/foo`; - expect(fs.existsSync(outDir)).toEqual(false); + expect(fs.existsSync(outDir)).toEqual(false); - await api.package({ dir, outDir }); + await api.package({ dir, outDir }); - expect(fs.existsSync(outDir)).toEqual(true); - }); - - it('can make from custom outDir without errors', async () => { - await updatePackageJSON(dir, async (packageJSON) => { - // eslint-disable-next-line n/no-missing-require - packageJSON.config.forge.makers = [{ name: require.resolve('@electron-forge/maker-zip') } as IForgeResolvableMaker]; + expect(fs.existsSync(outDir)).toEqual(true); }); - await api.make({ dir, skipPackage: true, outDir: `${dir}/foo` }); + it('can make from custom outDir without errors', async () => { + await updatePackageJSON(dir, async (packageJSON) => { + // eslint-disable-next-line n/no-missing-require + packageJSON.config.forge.makers = [{ name: require.resolve('@electron-forge/maker-zip') } as IForgeResolvableMaker]; + }); - // Cleanup here to ensure things dont break in the make tests - await fs.promises.rm(path.resolve(dir, 'foo'), { recursive: true, force: true }); - await fs.promises.rm(path.resolve(dir, 'out'), { recursive: true, force: true }); - }); + await api.make({ dir, skipPackage: true, outDir: `${dir}/foo` }); - // FIXME(erickzhao): This test hangs on the electron-rebuild step - // with Electron 19. It was tested to work on Electron 18. - // see https://github.com/electron/forge/pull/2869 - describe.skip('with prebuilt native module deps installed', () => { - beforeAll(async () => { - await installDeps(dir, ['ref-napi']); + // Cleanup here to ensure things dont break in the make tests + await fs.promises.rm(path.resolve(dir, 'foo'), { recursive: true, force: true }); + await fs.promises.rm(path.resolve(dir, 'out'), { recursive: true, force: true }); }); - it('can package without errors', async () => { - await api.package({ dir }); - }); + // FIXME(erickzhao): This test hangs on the electron-rebuild step + // with Electron 19. It was tested to work on Electron 18. + // see https://github.com/electron/forge/pull/2869 + describe.skip('with prebuilt native module deps installed', () => { + beforeAll(async () => { + await installDeps(dir, ['ref-napi']); + }); - afterAll(async () => { - await fs.promises.rm(path.resolve(dir, 'node_modules/ref-napi'), { recursive: true, force: true }); - await updatePackageJSON(dir, async (packageJSON) => { - delete packageJSON.dependencies['ref-napi']; + it('can package without errors', async () => { + await api.package({ dir }); }); - }); - }); - it('can package without errors', async () => { - await updatePackageJSON(dir, async (packageJSON) => { - assert(packageJSON.config.forge.packagerConfig); - packageJSON.config.forge.packagerConfig.asar = true; + afterAll(async () => { + await fs.promises.rm(path.resolve(dir, 'node_modules/ref-napi'), { recursive: true, force: true }); + await updatePackageJSON(dir, async (packageJSON) => { + delete packageJSON.dependencies['ref-napi']; + }); + }); }); - await api.package({ dir }); - }); - - describe('after package', () => { - it('should have deleted the forge config from the packaged app', async () => { - const cleanPackageJSON = await readMetadata({ - src: path.resolve(dir, 'out', `Test-App-${process.platform}-${process.arch}`), - logger: console.error, + it('can package without errors', async () => { + await updatePackageJSON(dir, async (packageJSON) => { + assert(packageJSON.config.forge.packagerConfig); + packageJSON.config.forge.packagerConfig.asar = true; }); - expect(cleanPackageJSON).not.toHaveProperty('config.forge'); - }); - it('should not affect the actual forge config', async () => { - const normalPackageJSON = await readRawPackageJson(dir); - expect(normalPackageJSON).toHaveProperty('config.forge'); + await api.package({ dir }); }); - if (process.platform !== 'win32') { - process.env.DISABLE_SQUIRREL_TEST = 'true'; - } + describe('after package', () => { + it('should have deleted the forge config from the packaged app', async () => { + const cleanPackageJSON = await readMetadata({ + src: path.resolve(dir, 'out', `Test-App-${process.platform}-${process.arch}`), + logger: console.error, + }); + expect(cleanPackageJSON).not.toHaveProperty('config.forge'); + }); - function getMakers(good: boolean) { - const allMakers = [ - '@electron-forge/maker-appx', - '@electron-forge/maker-deb', - '@electron-forge/maker-dmg', - '@electron-forge/maker-flatpak', - '@electron-forge/maker-rpm', - '@electron-forge/maker-snap', - '@electron-forge/maker-squirrel', - '@electron-forge/maker-wix', - '@electron-forge/maker-zip', - ]; - return allMakers - .map((maker) => require.resolve(maker)) - .filter((makerPath) => { - // eslint-disable-next-line @typescript-eslint/no-require-imports - const MakerClass = require(makerPath).default; - const maker = new MakerClass(); - return maker.isSupportedOnCurrentPlatform() === good && maker.externalBinariesExist() === good; - }) - .map((makerPath) => () => { - const makerDefinition = { - name: makerPath, - platforms: [process.platform], - config: { - devCert, - }, - }; + it('should not affect the actual forge config', async () => { + const normalPackageJSON = await readRawPackageJson(dir); + expect(normalPackageJSON).toHaveProperty('config.forge'); + }); - if (process.platform === 'win32') { - (makerDefinition.config as Record).makeVersionWinStoreCompatible = true; - } + if (process.platform !== 'win32') { + process.env.DISABLE_SQUIRREL_TEST = 'true'; + } - return makerDefinition; - }); - } + function getMakers(good: boolean) { + const allMakers = [ + '@electron-forge/maker-appx', + '@electron-forge/maker-deb', + '@electron-forge/maker-dmg', + '@electron-forge/maker-flatpak', + '@electron-forge/maker-rpm', + '@electron-forge/maker-snap', + '@electron-forge/maker-squirrel', + '@electron-forge/maker-wix', + '@electron-forge/maker-zip', + ]; + return allMakers + .map((maker) => require.resolve(maker)) + .filter((makerPath) => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const MakerClass = require(makerPath).default; + const maker = new MakerClass(); + return maker.isSupportedOnCurrentPlatform() === good && maker.externalBinariesExist() === good; + }) + .map((makerPath) => () => { + const makerDefinition = { + name: makerPath, + platforms: [process.platform], + config: { + devCert, + }, + }; + + if (process.platform === 'win32') { + (makerDefinition.config as Record).makeVersionWinStoreCompatible = true; + } + + return makerDefinition; + }); + } - const goodMakers = getMakers(true); - const badMakers = getMakers(false); - - const testMakeTarget = function testMakeTarget( - target: () => { name: string }, - shouldPass: boolean, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ...options: any[] - ) { - describe(`make (with target=${target().name})`, async () => { - beforeAll(async () => { - await updatePackageJSON(dir, async (packageJSON) => { - packageJSON.config.forge.makers = [target() as IForgeResolvableMaker]; + const goodMakers = getMakers(true); + const badMakers = getMakers(false); + + const testMakeTarget = function testMakeTarget( + target: () => { name: string }, + shouldPass: boolean, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ...options: any[] + ) { + describe(`make (with target=${target().name})`, async () => { + beforeAll(async () => { + await updatePackageJSON(dir, async (packageJSON) => { + packageJSON.config.forge.makers = [target() as IForgeResolvableMaker]; + }); }); - }); - for (const optionsFetcher of options) { - if (shouldPass) { - it(`successfully makes for config: ${JSON.stringify(optionsFetcher())}`, async () => { - const outputs = await api.make(optionsFetcher()); - for (const outputResult of outputs) { - for (const output of outputResult.artifacts) { - expect(fs.existsSync(output)).toEqual(true); - expect(output.startsWith(path.resolve(dir, 'out', 'make'))).toEqual(true); + for (const optionsFetcher of options) { + if (shouldPass) { + it(`successfully makes for config: ${JSON.stringify(optionsFetcher())}`, async () => { + const outputs = await api.make(optionsFetcher()); + for (const outputResult of outputs) { + for (const output of outputResult.artifacts) { + expect(fs.existsSync(output)).toEqual(true); + expect(output.startsWith(path.resolve(dir, 'out', 'make'))).toEqual(true); + } } - } - }); - } else { - it(`fails for config: ${JSON.stringify(optionsFetcher())}`, async () => { - await expect(api.make(optionsFetcher())).rejects.toThrow(); - }); + }); + } else { + it(`fails for config: ${JSON.stringify(optionsFetcher())}`, async () => { + await expect(api.make(optionsFetcher())).rejects.toThrow(); + }); + } } - } - }); - }; + }); + }; - const targetOptionFetcher = () => ({ dir, skipPackage: true }); - for (const maker of goodMakers) { - testMakeTarget(maker, true, targetOptionFetcher); - } + const targetOptionFetcher = () => ({ dir, skipPackage: true }); + for (const maker of goodMakers) { + testMakeTarget(maker, true, targetOptionFetcher); + } - for (const maker of badMakers) { - testMakeTarget(maker, false, targetOptionFetcher); - } + for (const maker of badMakers) { + testMakeTarget(maker, false, targetOptionFetcher); + } - describe('make', () => { - it('throws an error when given an unrecognized platform', async () => { - await expect(api.make({ dir, platform: 'dos' })).rejects.toThrow(/invalid platform/); - }); + describe('make', () => { + it('throws an error when given an unrecognized platform', async () => { + await expect(api.make({ dir, platform: 'dos' })).rejects.toThrow(/invalid platform/); + }); - it("throws an error when the specified maker doesn't support the current platform", async () => { - const makerPath = path.resolve(__dirname, '../fixture/maker-unsupported'); - await expect( - api.make({ - dir, - overrideTargets: [ - { - name: makerPath, - } as IForgeResolvableMaker, - ], - skipPackage: true, - }) - ).rejects.toThrow(/the maker declared that it cannot run/); - }); + it("throws an error when the specified maker doesn't support the current platform", async () => { + const makerPath = path.resolve(__dirname, '../fixture/maker-unsupported'); + await expect( + api.make({ + dir, + overrideTargets: [ + { + name: makerPath, + } as IForgeResolvableMaker, + ], + skipPackage: true, + }) + ).rejects.toThrow(/the maker declared that it cannot run/); + }); - it("throws an error when the specified maker doesn't implement isSupportedOnCurrentPlatform()", async () => { - const makerPath = path.resolve(__dirname, '../fixture/maker-incompatible'); - await expect( - api.make({ - dir, - overrideTargets: [ - { - name: makerPath, - } as IForgeResolvableMaker, - ], - skipPackage: true, - }) - ).rejects.toThrow(/incompatible with this version/); - }); + it("throws an error when the specified maker doesn't implement isSupportedOnCurrentPlatform()", async () => { + const makerPath = path.resolve(__dirname, '../fixture/maker-incompatible'); + await expect( + api.make({ + dir, + overrideTargets: [ + { + name: makerPath, + } as IForgeResolvableMaker, + ], + skipPackage: true, + }) + ).rejects.toThrow(/incompatible with this version/); + }); - it('throws an error when no makers are configured for the given platform', async () => { - await expect( - api.make({ - dir, - overrideTargets: [ - { - name: path.resolve(__dirname, '../fixture/maker-wrong-platform'), - } as IForgeResolvableMaker, - ], - platform: 'linux', - skipPackage: true, - }) - ).rejects.toThrow('Could not find any make targets configured for the "linux" platform.'); - }); + it('throws an error when no makers are configured for the given platform', async () => { + await expect( + api.make({ + dir, + overrideTargets: [ + { + name: path.resolve(__dirname, '../fixture/maker-wrong-platform'), + } as IForgeResolvableMaker, + ], + platform: 'linux', + skipPackage: true, + }) + ).rejects.toThrow('Could not find any make targets configured for the "linux" platform.'); + }); - it.runIf(process.platform === 'darwin')('can make for the MAS platform successfully', async () => { - await expect( - api.make({ - dir, - // eslint-disable-next-line n/no-missing-require - overrideTargets: [require.resolve('@electron-forge/maker-zip'), require.resolve('@electron-forge/maker-dmg')], - platform: 'mas', - }) - ).resolves.toHaveLength(2); + it.runIf(process.platform === 'darwin')('can make for the MAS platform successfully', async () => { + await expect( + api.make({ + dir, + // eslint-disable-next-line n/no-missing-require + overrideTargets: [require.resolve('@electron-forge/maker-zip'), require.resolve('@electron-forge/maker-dmg')], + platform: 'mas', + }) + ).resolves.toHaveLength(2); + }); }); }); - }); - afterAll(() => fs.promises.rm(dir, { recursive: true, force: true })); - }); - - afterAll(async () => { - await spawnPackageManager(['link:remove']); + afterAll(() => fs.promises.rm(dir, { recursive: true, force: true })); + }); }); }); diff --git a/packages/template/base/package.json b/packages/template/base/package.json index 8a183f5195..831fa8c100 100644 --- a/packages/template/base/package.json +++ b/packages/template/base/package.json @@ -11,6 +11,7 @@ "node": ">= 16.4.0" }, "dependencies": { + "@electron-forge/core-utils": "7.6.1", "@electron-forge/shared-types": "7.6.1", "@malept/cross-spawn-promise": "^2.0.0", "debug": "^4.3.1", diff --git a/packages/template/base/src/BaseTemplate.ts b/packages/template/base/src/BaseTemplate.ts index e3af328608..408261b8e8 100644 --- a/packages/template/base/src/BaseTemplate.ts +++ b/packages/template/base/src/BaseTemplate.ts @@ -1,5 +1,6 @@ import path from 'node:path'; +import { resolvePackageManager } from '@electron-forge/core-utils'; import { ForgeListrTaskDefinition, ForgeTemplate, InitTemplateOptions } from '@electron-forge/shared-types'; import debug from 'debug'; import fs from 'fs-extra'; @@ -79,6 +80,15 @@ export class BaseTemplate implements ForgeTemplate { packageJSON.productName = packageJSON.name = path.basename(directory).toLowerCase(); packageJSON.author = await determineAuthor(directory); + const pm = await resolvePackageManager(); + + // As of pnpm v10, no postinstall scripts will run unless allowlisted through `onlyBuiltDependencies` + if (pm.executable === 'pnpm') { + packageJSON.pnpm = { + onlyBuiltDependencies: ['electron'], + }; + } + packageJSON.scripts.lint = 'echo "No linting configured"'; d('writing package.json to:', directory); From 15db452e4c63caa63a99eda910db949cfb660262 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 28 Jan 2025 15:13:34 -0800 Subject: [PATCH 10/30] copyFile instead of rename --- packages/api/core/spec/slow/api.slow.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/core/spec/slow/api.slow.spec.ts b/packages/api/core/spec/slow/api.slow.spec.ts index dc63da365f..322e47f8f7 100644 --- a/packages/api/core/spec/slow/api.slow.spec.ts +++ b/packages/api/core/spec/slow/api.slow.spec.ts @@ -242,7 +242,7 @@ describe.each([{ pm: 'npm' }, { pm: 'yarn' }, { pm: 'pnpm' }])(`init (with $pm)` }, }; if (process.platform === 'win32') { - await fs.promises.rename(path.join(__dirname, '..', 'fixture', 'bogus-private-key.pvk'), path.join(dir, 'default.pvk')); + await fs.promises.copyFile(path.join(__dirname, '..', 'fixture', 'bogus-private-key.pvk'), path.join(dir, 'default.pvk')); devCert = await createDefaultCertificate('CN=Test Author', { certFilePath: dir }); } else if (process.platform === 'linux') { packageJSON.config.forge.packagerConfig = { From 89a0fd1c6e9218dda97d9e1e3fad9bf113e59d75 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 28 Jan 2025 16:03:08 -0800 Subject: [PATCH 11/30] add additional checks --- .circleci/config.yml | 3 +- packages/api/cli/spec/check-system.spec.ts | 33 ++++++++++++++++++---- packages/api/cli/src/util/check-system.ts | 16 +++++++++-- packages/template/base/src/BaseTemplate.ts | 7 +++++ packages/template/base/tmpl/.npmrc | 1 + 5 files changed, 50 insertions(+), 10 deletions(-) create mode 100644 packages/template/base/tmpl/.npmrc diff --git a/.circleci/config.yml b/.circleci/config.yml index 4165ef5d69..5b8f081bd9 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -113,8 +113,7 @@ jobs: - run: name: 'Install pnpm' command: | - # FIXME: upgrade to pnpm 10 and add Electron to allowlist for postinstall scripts - npm install -g pnpm@9.15.4 + npm install -g pnpm@10.0.0 - run-fast-tests - store_test_results: path: ./reports/ diff --git a/packages/api/cli/spec/check-system.spec.ts b/packages/api/cli/spec/check-system.spec.ts index 1e7c08bc7e..f0d1d73af9 100644 --- a/packages/api/cli/spec/check-system.spec.ts +++ b/packages/api/cli/spec/check-system.spec.ts @@ -1,7 +1,7 @@ import { resolvePackageManager, spawnPackageManager } from '@electron-forge/core-utils'; import { describe, expect, it, vi } from 'vitest'; -import { checkPackageManagerVersion } from '../src/util/check-system'; +import { checkPackageManager } from '../src/util/check-system'; vi.mock(import('@electron-forge/core-utils'), async (importOriginal) => { const mod = await importOriginal(); @@ -12,7 +12,7 @@ vi.mock(import('@electron-forge/core-utils'), async (importOriginal) => { }; }); -describe('checkPackageManagerVersion', () => { +describe('checkPackageManager', () => { it('should consider allowlisted versions to be valid', async () => { vi.mocked(resolvePackageManager).mockResolvedValue({ executable: 'npm', @@ -21,7 +21,7 @@ describe('checkPackageManagerVersion', () => { exact: '--save-exact', }); vi.mocked(spawnPackageManager).mockResolvedValue('10.9.2'); - await expect(checkPackageManagerVersion()).resolves.not.toThrow(); + await expect(checkPackageManager()).resolves.not.toThrow(); }); it('rejects versions that are outside of the supported range', async () => { @@ -34,7 +34,7 @@ describe('checkPackageManagerVersion', () => { // yarn 0.x unsupported vi.mocked(spawnPackageManager).mockResolvedValue('0.22.0'); - await expect(checkPackageManagerVersion()).rejects.toThrow(); + await expect(checkPackageManager()).rejects.toThrow(); }); it('should consider Yarn nightly versions to be invalid', async () => { @@ -45,7 +45,7 @@ describe('checkPackageManagerVersion', () => { exact: '--exact', }); vi.mocked(spawnPackageManager).mockResolvedValue('0.23.0-20170311.0515'); - await expect(checkPackageManagerVersion()).rejects.toThrow(); + await expect(checkPackageManager()).rejects.toThrow(); }); it('should consider invalid semver versions to be invalid', async () => { @@ -56,6 +56,27 @@ describe('checkPackageManagerVersion', () => { exact: '--exact', }); vi.mocked(spawnPackageManager).mockResolvedValue('1.22'); - await expect(checkPackageManagerVersion()).rejects.toThrow(); + await expect(checkPackageManager()).rejects.toThrow(); + }); + + it('should throw if using pnpm without node-linker=hoisted', async () => { + vi.mocked(resolvePackageManager).mockResolvedValue({ + executable: 'pnpm', + install: 'add', + dev: '--dev', + exact: '--exact', + }); + vi.mocked(spawnPackageManager).mockImplementation((args) => { + if (args?.join(' ') === 'config get node-linker') { + return Promise.resolve('isolated'); + } else if (args?.join(' ') === '--version') { + return Promise.resolve('10.0.0'); + } else { + throw new Error('Unexpected command'); + } + }); + await expect(checkPackageManager()).rejects.toThrow( + 'When using pnpm, `node-linker` must be set to "hoisted". Run `pnpm config set node-linker hoisted` to set this config value.' + ); }); }); diff --git a/packages/api/cli/src/util/check-system.ts b/packages/api/cli/src/util/check-system.ts index 6ef5109531..1df5e77258 100644 --- a/packages/api/cli/src/util/check-system.ts +++ b/packages/api/cli/src/util/check-system.ts @@ -27,6 +27,14 @@ async function checkNodeVersion() { return process.versions.node; } +async function checkPnpmNodeLinker() { + const nodeLinkerValue = await spawnPackageManager(['config', 'get', 'node-linker']); + + if (nodeLinkerValue !== 'hoisted') { + throw new Error('When using pnpm, `node-linker` must be set to "hoisted". Run `pnpm config set node-linker hoisted` to set this config value.'); + } +} + const ALLOWLISTED_VERSIONS: Record> = { npm: { all: '^3.0.0 || ^4.0.0 || ~5.1.0 || ~5.2.0 || >= 5.4.2', @@ -41,7 +49,7 @@ const ALLOWLISTED_VERSIONS: Record) { { title: 'Checking package manager version', task: async (_, task) => { - const packageManager = await checkPackageManagerVersion(); + const packageManager = await checkPackageManager(); task.title = `Found ${packageManager}`; }, }, diff --git a/packages/template/base/src/BaseTemplate.ts b/packages/template/base/src/BaseTemplate.ts index 408261b8e8..34237f3378 100644 --- a/packages/template/base/src/BaseTemplate.ts +++ b/packages/template/base/src/BaseTemplate.ts @@ -40,12 +40,19 @@ export class BaseTemplate implements ForgeTemplate { { title: 'Copying starter files', task: async () => { + const pm = await resolvePackageManager(); d('creating directory:', path.resolve(directory, 'src')); await fs.mkdirs(path.resolve(directory, 'src')); const rootFiles = ['_gitignore', 'forge.config.js']; + + if (pm.executable === 'pnpm') { + rootFiles.push('.npmrc'); + } + if (copyCIFiles) { d(`Copying CI files is currently not supported - this will be updated in a later version of Forge`); } + const srcFiles = ['index.css', 'index.js', 'index.html', 'preload.js']; for (const file of rootFiles) { diff --git a/packages/template/base/tmpl/.npmrc b/packages/template/base/tmpl/.npmrc new file mode 100644 index 0000000000..1d6a0d53cb --- /dev/null +++ b/packages/template/base/tmpl/.npmrc @@ -0,0 +1 @@ +node-linker = hoisted \ No newline at end of file From e23518f2d6d8fefff714e22a8b8cc84ded3243ec Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 28 Jan 2025 16:45:53 -0800 Subject: [PATCH 12/30] use default reporter in CI --- .circleci/config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 5b8f081bd9..caf86e1166 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -46,7 +46,7 @@ commands: - run: name: 'Run fast tests' command: | - yarn test:fast --reporter=junit --outputFile="./reports/out/test_output.xml" + yarn test:fast --reporter=default --reporter=junit --outputFile="./reports/out/test_output.xml" run-slow-tests: steps: @@ -57,7 +57,7 @@ commands: - run: name: 'Run slow tests' command: | - yarn test:slow --reporter=junit --outputFile="./reports/out/test_output.xml" + yarn test:slow --reporter=default --reporter=junit --outputFile="./reports/out/test_output.xml" jobs: lint-and-build: From 93c0ef801ba3c3ab265d9d4478853610bb73e49b Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 28 Jan 2025 18:58:47 -0800 Subject: [PATCH 13/30] fix [object Object] --- packages/api/core/src/api/import.ts | 5 ++--- packages/api/core/src/util/install-dependencies.ts | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/api/core/src/api/import.ts b/packages/api/core/src/api/import.ts index bbcc564af5..321f3c2afb 100644 --- a/packages/api/core/src/api/import.ts +++ b/packages/api/core/src/api/import.ts @@ -200,17 +200,16 @@ export default autoTrace( await fs.remove(path.resolve(dir, 'node_modules/.bin/electron')); await fs.remove(path.resolve(dir, 'node_modules/.bin/electron.cmd')); - // FIXME(erickzhao): these flags should be package-manager-specific. d('installing dependencies'); task.output = `${pm.executable} ${pm.install} ${importDeps.join(' ')}`; await installDepList(dir, importDeps); d('installing devDependencies'); - task.output = `${pm} ${pm.install} ${pm.dev} ${importDevDeps.join(' ')}`; + task.output = `${pm.executable} ${pm.install} ${pm.dev} ${importDevDeps.join(' ')}`; await installDepList(dir, importDevDeps, DepType.DEV); d('installing devDependencies with exact versions'); - task.output = `${pm} ${pm.install} ${pm.dev} ${pm.exact} ${importExactDevDeps.join(' ')}`; + task.output = `${pm.executable} ${pm.install} ${pm.dev} ${pm.exact} ${importExactDevDeps.join(' ')}`; await installDepList(dir, importExactDevDeps, DepType.DEV, DepVersionRestriction.EXACT); }, }, diff --git a/packages/api/core/src/util/install-dependencies.ts b/packages/api/core/src/util/install-dependencies.ts index 01c00ca90c..77f08d73ce 100644 --- a/packages/api/core/src/util/install-dependencies.ts +++ b/packages/api/core/src/util/install-dependencies.ts @@ -16,7 +16,7 @@ export enum DepVersionRestriction { export default async (dir: string, deps: string[], depType = DepType.PROD, versionRestriction = DepVersionRestriction.RANGE): Promise => { const pm = await resolvePackageManager(); - d('installing', JSON.stringify(deps), 'in:', dir, `depType=${depType},versionRestriction=${versionRestriction},withPackageManager=${pm}`); + d('installing', JSON.stringify(deps), 'in:', dir, `depType=${depType},versionRestriction=${versionRestriction},withPackageManager=${pm.executable}`); if (deps.length === 0) { d('nothing to install, stopping immediately'); return Promise.resolve(); From 8b0e22f7ef8a0e58b63148d858658c9ff4296f35 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 29 Jan 2025 15:24:03 -0800 Subject: [PATCH 14/30] add back `packageManager` and work around with `.npmrc` setting --- .npmrc | 2 ++ package.json | 1 + 2 files changed, 3 insertions(+) create mode 100644 .npmrc diff --git a/.npmrc b/.npmrc new file mode 100644 index 0000000000..acec48d1fc --- /dev/null +++ b/.npmrc @@ -0,0 +1,2 @@ +# Required to run pnpm in tests because `packageManager` is set to `yarn` in `package.json` +package-manager-strict=false \ No newline at end of file diff --git a/package.json b/package.json index 8f84b8f6e5..f1396f5fbf 100644 --- a/package.json +++ b/package.json @@ -151,6 +151,7 @@ "prettier": { "singleQuote": true }, + "packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e", "workspaces": { "packages": [ "packages/api/*", From 1592d8564bde1528657a94af0276cddbfee3d0e4 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 29 Jan 2025 15:36:19 -0800 Subject: [PATCH 15/30] add missing mock :) --- packages/api/core/spec/fast/util/install-dependencies.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/api/core/spec/fast/util/install-dependencies.spec.ts b/packages/api/core/spec/fast/util/install-dependencies.spec.ts index 057238192d..6a4ee1a91d 100644 --- a/packages/api/core/spec/fast/util/install-dependencies.spec.ts +++ b/packages/api/core/spec/fast/util/install-dependencies.spec.ts @@ -14,6 +14,7 @@ vi.mock(import('@electron-forge/core-utils'), async (importOriginal) => { describe('installDependencies', () => { it('should immediately resolve if no deps are provided', async () => { + vi.mocked(resolvePackageManager).mockResolvedValue({ executable: 'npm', install: 'install', dev: '--save-dev', exact: '--save-exact' }); await installDependencies('mydir', []); expect(spawnPackageManager).not.toHaveBeenCalled(); }); From d5a43a2a38b976bcfda1e26d3a2cfd23af33b951 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 29 Jan 2025 16:53:28 -0800 Subject: [PATCH 16/30] fix CLI output for initNPM commands --- packages/api/core/src/api/init-scripts/init-npm.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/api/core/src/api/init-scripts/init-npm.ts b/packages/api/core/src/api/init-scripts/init-npm.ts index c686dc2cdb..98f9f2e2f3 100644 --- a/packages/api/core/src/api/init-scripts/init-npm.ts +++ b/packages/api/core/src/api/init-scripts/init-npm.ts @@ -29,17 +29,17 @@ export const exactDevDeps = ['electron']; export const initNPM = async (dir: string, task: ForgeListrTask): Promise => { d('installing dependencies'); - const packageManager = await resolvePackageManager(); - task.output = `${packageManager} install ${deps.join(' ')}`; + const pm = await resolvePackageManager(); + task.output = `${pm.executable} ${pm.install} ${deps.join(' ')}`; await installDepList(dir, deps); d('installing devDependencies'); - task.output = `${packageManager} install --dev ${deps.join(' ')}`; + task.output = `${pm.executable} ${pm.install} ${pm.dev} ${deps.join(' ')}`; await installDepList(dir, devDeps, DepType.DEV); d('installing exact devDependencies'); for (const packageName of exactDevDeps) { - task.output = `${packageManager} install --dev --exact ${packageName}`; + task.output = `${pm.executable} ${pm.install} ${pm.dev} ${pm.exact} ${packageName}`; await installDepList(dir, [packageName], DepType.DEV, DepVersionRestriction.EXACT); } }; From d695b267efecafec5e22ad9e6e3a030c8dc984f1 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Mon, 3 Feb 2025 17:41:13 -0800 Subject: [PATCH 17/30] use npm_config_user_agent --- package.json | 1 - packages/api/cli/spec/check-system.spec.ts | 14 +++ packages/api/cli/src/util/check-system.ts | 5 +- packages/api/core/package.json | 1 - packages/utils/core-utils/package.json | 1 - .../core-utils/spec/electron-version.spec.ts | 4 +- .../core-utils/spec/package-manager.spec.ts | 101 +++++++++++++----- .../utils/core-utils/src/electron-version.ts | 5 +- .../utils/core-utils/src/package-manager.ts | 67 ++++++++++-- yarn.lock | 7 -- 10 files changed, 156 insertions(+), 50 deletions(-) diff --git a/package.json b/package.json index f1396f5fbf..c29173d4dc 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,6 @@ "cross-spawn": "^7.0.3", "cross-zip": "^4.0.0", "debug": "^4.3.1", - "detect-package-manager": "^3.0.2", "express": "^4.17.1", "express-ws": "^5.0.2", "fast-glob": "^3.2.7", diff --git a/packages/api/cli/spec/check-system.spec.ts b/packages/api/cli/spec/check-system.spec.ts index f0d1d73af9..62b79575c9 100644 --- a/packages/api/cli/spec/check-system.spec.ts +++ b/packages/api/cli/spec/check-system.spec.ts @@ -79,4 +79,18 @@ describe('checkPackageManager', () => { 'When using pnpm, `node-linker` must be set to "hoisted". Run `pnpm config set node-linker hoisted` to set this config value.' ); }); + + // resolvePackageManager optionally returns a `version` if `npm_config_user_agent` was used to + // resolve the package manager being used. + it('should not shell out to child process if version was already parsed via npm_config_user_agent', async () => { + vi.mocked(resolvePackageManager).mockResolvedValue({ + executable: 'npm', + install: 'install', + dev: '--save-dev', + exact: '--save-exact', + version: '10.9.2', + }); + await expect(checkPackageManager()).resolves.not.toThrow(); + expect(spawnPackageManager).not.toHaveBeenCalled(); + }); }); diff --git a/packages/api/cli/src/util/check-system.ts b/packages/api/cli/src/util/check-system.ts index 1df5e77258..30f2eac0a8 100644 --- a/packages/api/cli/src/util/check-system.ts +++ b/packages/api/cli/src/util/check-system.ts @@ -35,6 +35,7 @@ async function checkPnpmNodeLinker() { } } +// TODO(v8): Drop antiquated versions of npm const ALLOWLISTED_VERSIONS: Record> = { npm: { all: '^3.0.0 || ^4.0.0 || ~5.1.0 || ~5.2.0 || >= 5.4.2', @@ -50,9 +51,9 @@ const ALLOWLISTED_VERSIONS: Record { describe('with npm workspaces', () => { beforeAll(() => { - process.env.NODE_INSTALLER = 'npm'; + process.env.npm_config_user_agent = 'npm/10.9.2 node/v22.13.0 darwin arm64 workspaces/false'; }); afterAll(() => { - delete process.env.NODE_INSTALLER; + delete process.env.npm_config_user_agent; }); it('finds the top-level electron module', async () => { diff --git a/packages/utils/core-utils/spec/package-manager.spec.ts b/packages/utils/core-utils/spec/package-manager.spec.ts index 91b8c282c4..38831f6a63 100644 --- a/packages/utils/core-utils/spec/package-manager.spec.ts +++ b/packages/utils/core-utils/spec/package-manager.spec.ts @@ -1,43 +1,92 @@ -import { detect } from 'detect-package-manager'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import findUp from 'find-up'; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; import { resolvePackageManager } from '../src/package-manager'; +vi.mock('find-up', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + default: vi.fn(), + }; +}); + describe('package-manager', () => { - let nodeInstaller: string | undefined; + describe('npm_config_user_agent', () => { + let originalUa: string | undefined; + beforeAll(() => { + originalUa = process.env.npm_config_user_agent; + }); + afterAll(() => { + process.env.npm_config_user_agent = originalUa; + }); + + it.each([ + { ua: 'yarn/1.22.22 npm/? node/v22.13.0 darwin arm64', pm: 'yarn', version: '1.22.22' }, + { ua: 'pnpm/10.0.0 npm/? node/v20.11.1 darwin arm64', pm: 'pnpm', version: '10.0.0' }, + { ua: 'npm/10.9.2 node/v22.13.0 darwin arm64 workspaces/false', pm: 'npm', version: '10.9.2' }, + ])('with $ua', async ({ ua, pm, version }) => { + process.env.npm_config_user_agent = ua; + await expect(resolvePackageManager()).resolves.toHaveProperty('executable', pm); + await expect(resolvePackageManager()).resolves.toHaveProperty('version', version); + }); - beforeEach(() => { - nodeInstaller = process.env.NODE_INSTALLER; - delete process.env.NODE_INSTALLER; + it('should return yarn if npm_config_user_agent=yarn', async () => { + process.env.npm_config_user_agent = 'yarn/1.22.22 npm/? node/v22.13.0 darwin arm64'; + await expect(resolvePackageManager()).resolves.toHaveProperty('executable', 'yarn'); + await expect(resolvePackageManager()).resolves.toHaveProperty('version', '1.22.22'); + }); + + it('should return pnpm if npm_config_user_agent=pnpm', async () => { + process.env.npm_config_user_agent = 'pnpm/10.0.0 npm/? node/v20.11.1 darwin arm64'; + await expect(resolvePackageManager()).resolves.toHaveProperty('executable', 'pnpm'); + }); + + it('should return npm if npm_config_user_agent=npm', async () => { + process.env.npm_config_user_agent = 'npm/10.9.2 node/v22.13.0 darwin arm64 workspaces/false'; + await expect(resolvePackageManager()).resolves.toHaveProperty('executable', 'npm'); + }); }); - afterEach(() => { - if (!nodeInstaller) { + describe('NODE_INSTALLER', () => { + let nodeInstaller: string | undefined; + + beforeEach(() => { + nodeInstaller = process.env.NODE_INSTALLER; delete process.env.NODE_INSTALLER; - } else { - process.env.NODE_INSTALLER = nodeInstaller; - } - }); + // NODE_INSTALLER is deprecated for Electron Forge 8 and throws a console.warn that we want to silence in tests + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + }); + afterEach(() => { + if (!nodeInstaller) { + delete process.env.NODE_INSTALLER; + } else { + process.env.NODE_INSTALLER = nodeInstaller; + } + vi.restoreAllMocks(); + }); - it('should by default equal the system package manager value', async () => { - const pm = await detect(); - await expect(resolvePackageManager()).resolves.toHaveProperty('executable', pm); - }); + it.each(['yarn', 'npm', 'pnpm'])('should return yarn if NODE_INSTALLER=yarn', async (pm) => { + process.env.NODE_INSTALLER = pm; + await expect(resolvePackageManager()).resolves.toHaveProperty('executable', pm); + }); - it('should return yarn if NODE_INSTALLER=yarn', async () => { - process.env.NODE_INSTALLER = 'yarn'; - await expect(resolvePackageManager()).resolves.toHaveProperty('executable', 'yarn'); + it('should return npm if package manager is unsupported', async () => { + process.env.NODE_INSTALLER = 'bun'; + console.warn = vi.fn(); + await expect(resolvePackageManager()).resolves.toHaveProperty('executable', 'npm'); + expect(console.warn).toHaveBeenCalledWith('⚠', expect.stringContaining('Package manager bun is unsupported')); + }); }); - it('should return npm if NODE_INSTALLER=npm', async () => { - process.env.NODE_INSTALLER = 'npm'; - await expect(resolvePackageManager()).resolves.toHaveProperty('executable', 'npm'); + it('should equal the system package manager value if above strategies failed', async () => { + vi.mocked(findUp).mockResolvedValue('yarn.lock'); + await expect(resolvePackageManager()).resolves.toHaveProperty('executable', 'yarn'); }); - it('should return npm if package manager is unsupported', async () => { - process.env.NODE_INSTALLER = 'bun'; - console.warn = vi.fn(); + it('should fall back to npm if no other strategy worked', async () => { + process.env.npm_config_user_agent = undefined; + vi.mocked(findUp).mockResolvedValue(undefined); await expect(resolvePackageManager()).resolves.toHaveProperty('executable', 'npm'); - expect(console.warn).toHaveBeenCalledWith('⚠', expect.stringContaining('Package manager bun is unsupported')); }); }); diff --git a/packages/utils/core-utils/src/electron-version.ts b/packages/utils/core-utils/src/electron-version.ts index cc117795f3..7fe5d959f0 100644 --- a/packages/utils/core-utils/src/electron-version.ts +++ b/packages/utils/core-utils/src/electron-version.ts @@ -1,10 +1,11 @@ import path from 'node:path'; import debug from 'debug'; -import findUp from 'find-up'; import fs from 'fs-extra'; import semver from 'semver'; +import { findLockFile } from './package-manager'; + const d = debug('electron-forge:electron-version'); const electronPackageNames = ['electron-nightly', 'electron']; @@ -20,7 +21,7 @@ function findElectronDep(dep: string): boolean { async function findAncestorNodeModulesPath(dir: string, packageName: string): Promise { d('Looking for a lock file to indicate the root of the repo'); - const lockPath = await findUp(['package-lock.json', 'yarn.lock', 'pnpm-lock.yaml'], { cwd: dir, type: 'file' }); + const lockPath = await findLockFile(dir); if (lockPath) { d(`Found lock file: ${lockPath}`); const nodeModulesPath = path.join(path.dirname(lockPath), 'node_modules', packageName); diff --git a/packages/utils/core-utils/src/package-manager.ts b/packages/utils/core-utils/src/package-manager.ts index 1d33d1b791..9f30de4085 100644 --- a/packages/utils/core-utils/src/package-manager.ts +++ b/packages/utils/core-utils/src/package-manager.ts @@ -1,10 +1,10 @@ import { CrossSpawnArgs, CrossSpawnOptions, spawn } from '@malept/cross-spawn-promise'; import chalk from 'chalk'; -import { detect } from 'detect-package-manager'; +import findUp from 'find-up'; import logSymbols from 'log-symbols'; export type SupportedPackageManager = 'yarn' | 'npm' | 'pnpm'; -export type PMDetails = { executable: SupportedPackageManager; install: string; dev: string; exact: string }; +export type PMDetails = { executable: SupportedPackageManager; version?: string; install: string; dev: string; exact: string }; const MANAGERS: Record = { yarn: { @@ -27,23 +27,74 @@ const MANAGERS: Record = { }, }; +const PM_FROM_LOCKFILE: Record = { + 'package-lock.json': 'npm', + 'yarn.lock': 'yarn', + 'pnpm-lock.yaml': 'pnpm', +}; + +/** + * Parses the `npm_config_user_agent` environment variable and returns its name and version. + * + * Code taken from {@link https://github.com/zkochan/packages/tree/main/which-pm-runs/ | which-pm-runs}. + */ +function pmFromUserAgent() { + const userAgent = process.env.npm_config_user_agent; + if (!userAgent) { + return undefined; + } + const pmSpec = userAgent.split(' ')[0]; + const separatorPos = pmSpec.lastIndexOf('/'); + const name = pmSpec.substring(0, separatorPos); + return { + name: name === 'npminstall' ? 'cnpm' : name, + version: pmSpec.substring(separatorPos + 1), + }; +} + /** - * Resolves the package manager to use. Prioritizes the `NODE_INSTALLER` environment variable. + * Finds a lockfile in the ancestor directories of the given directory + */ +export async function findLockFile(dir: string) { + return await findUp(['package-lock.json', 'yarn.lock', 'pnpm-lock.yaml'], { cwd: dir, type: 'file' }); +} + +/** + * Resolves the package manager to use. In order, it checks the following: + * + * 1. The value of the `NODE_INSTALLER` environment variable. + * 2. The `process.env.npm_config_user_agent` value set by the executing package manager. + * 3. The presence of a lockfile in an ancestor directory. + * 4. If an unknown package manager is used (or none of the above apply), then we fall back to `npm`. + * + * The version of the executing package manager is also returned if it is detected via user agent. + * * Supported package managers are `yarn`, `pnpm`, and `npm`. * - * If an unknown package manager is used, then a warning is logged and we fall back to `npm`. */ export const resolvePackageManager: () => Promise = async () => { - const system = await detect(); - const installer = process.env.NODE_INSTALLER || system; + const executingPM = pmFromUserAgent(); + const lockfile = await findLockFile(process.cwd()); + const lockfilePM = (lockfile && PM_FROM_LOCKFILE[lockfile]) ?? undefined; + const installer = process.env.NODE_INSTALLER || executingPM?.name || lockfilePM; + + // TODO(v8): Remove NODE_INSTALLER environment variable + if (typeof process.env.NODE_INSTALLER === 'string') { + console.warn(logSymbols.warning, chalk.yellow(`The NODE_INSTALLER environment variable is deprecated and will be removed in Electron Forge v8`)); + } switch (installer) { case 'yarn': case 'npm': case 'pnpm': - return MANAGERS[installer]; + return { ...MANAGERS[installer], version: executingPM?.version }; default: - console.warn(logSymbols.warning, chalk.yellow(`Package manager ${chalk.red(installer)} is unsupported. Falling back to ${chalk.green('npm')} instead.`)); + if (installer !== undefined) { + console.warn( + logSymbols.warning, + chalk.yellow(`Package manager ${chalk.red(installer)} is unsupported. Falling back to ${chalk.green('npm')} instead.`) + ); + } return MANAGERS['npm']; } }; diff --git a/yarn.lock b/yarn.lock index a9c013fc74..5c401ec82f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5322,13 +5322,6 @@ detect-node@^2.0.4: resolved "https://registry.yarnpkg.com/detect-node/-/detect-node-2.1.0.tgz#c9c70775a49c3d03bc2c06d9a73be550f978f8b1" integrity sha512-T0NIuQpnTvFDATNuHN5roPwSBG83rFsuO+MXXH9/3N1eFbn4wcPjttvjMLEPWJ0RGUYgQE7cGgS3tNxbqCGM7g== -detect-package-manager@^3.0.2: - version "3.0.2" - resolved "https://registry.yarnpkg.com/detect-package-manager/-/detect-package-manager-3.0.2.tgz#ca34261ab84198072580e93ae86582c575428da9" - integrity sha512-8JFjJHutStYrfWwzfretQoyNGoZVW1Fsrp4JO9spa7h/fBfwgTMEIy4/LBzRDGsxwVPHU0q+T9YvwLDJoOApLQ== - dependencies: - execa "^5.1.1" - dezalgo@^1.0.0: version "1.0.4" resolved "https://registry.yarnpkg.com/dezalgo/-/dezalgo-1.0.4.tgz#751235260469084c132157dfa857f386d4c33d81" From 7a5ab7056221b7e7a1df7ea83429a86713eb5a28 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 4 Feb 2025 22:58:59 -0800 Subject: [PATCH 18/30] Update .npmrc Co-authored-by: Kevin Cui --- .npmrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.npmrc b/.npmrc index acec48d1fc..18d60a49d1 100644 --- a/.npmrc +++ b/.npmrc @@ -1,2 +1,2 @@ # Required to run pnpm in tests because `packageManager` is set to `yarn` in `package.json` -package-manager-strict=false \ No newline at end of file +package-manager-strict=false From 3350dfeca57049c6d2b801480e93376277d4ac40 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 4 Feb 2025 22:59:13 -0800 Subject: [PATCH 19/30] Update packages/template/base/tmpl/.npmrc Co-authored-by: Kevin Cui --- packages/template/base/tmpl/.npmrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/template/base/tmpl/.npmrc b/packages/template/base/tmpl/.npmrc index 1d6a0d53cb..17633b501a 100644 --- a/packages/template/base/tmpl/.npmrc +++ b/packages/template/base/tmpl/.npmrc @@ -1 +1 @@ -node-linker = hoisted \ No newline at end of file +node-linker = hoisted From 65458b3377d3f51ff8915a6042797d6d94b0916e Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 5 Feb 2025 13:35:19 -0800 Subject: [PATCH 20/30] Update packages/utils/core-utils/src/package-manager.ts Co-authored-by: Kevin Cui --- packages/utils/core-utils/src/package-manager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/core-utils/src/package-manager.ts b/packages/utils/core-utils/src/package-manager.ts index 9f30de4085..a81f215526 100644 --- a/packages/utils/core-utils/src/package-manager.ts +++ b/packages/utils/core-utils/src/package-manager.ts @@ -43,7 +43,7 @@ function pmFromUserAgent() { if (!userAgent) { return undefined; } - const pmSpec = userAgent.split(' ')[0]; + const pmSpec = userAgent.split(' ', 1)[0]; const separatorPos = pmSpec.lastIndexOf('/'); const name = pmSpec.substring(0, separatorPos); return { From 1b084df7bd23d765c8ba7f76b865061b961a1a4d Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 5 Feb 2025 13:39:24 -0800 Subject: [PATCH 21/30] clarify todo with my name --- packages/utils/core-utils/src/package-manager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/core-utils/src/package-manager.ts b/packages/utils/core-utils/src/package-manager.ts index a81f215526..34f3c93e0a 100644 --- a/packages/utils/core-utils/src/package-manager.ts +++ b/packages/utils/core-utils/src/package-manager.ts @@ -78,7 +78,7 @@ export const resolvePackageManager: () => Promise = async () => { const lockfilePM = (lockfile && PM_FROM_LOCKFILE[lockfile]) ?? undefined; const installer = process.env.NODE_INSTALLER || executingPM?.name || lockfilePM; - // TODO(v8): Remove NODE_INSTALLER environment variable + // TODO(erickzhao): Remove NODE_INSTALLER environment variable for Forge 8 if (typeof process.env.NODE_INSTALLER === 'string') { console.warn(logSymbols.warning, chalk.yellow(`The NODE_INSTALLER environment variable is deprecated and will be removed in Electron Forge v8`)); } From 336662ce0293d090648a79efd9f5176c17caeb53 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 5 Feb 2025 13:51:45 -0800 Subject: [PATCH 22/30] Update packages/api/core/spec/slow/api.slow.spec.ts Co-authored-by: Kevin Cui --- packages/api/core/spec/slow/api.slow.spec.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/api/core/spec/slow/api.slow.spec.ts b/packages/api/core/spec/slow/api.slow.spec.ts index 2c785159b1..4e6469b1e8 100644 --- a/packages/api/core/spec/slow/api.slow.spec.ts +++ b/packages/api/core/spec/slow/api.slow.spec.ts @@ -39,10 +39,11 @@ describe.each([{ pm: 'npm' }, { pm: 'yarn' }, { pm: 'pnpm' }])(`init (with $pm)` if (pm === 'pnpm') { await spawnPackageManager('config set node-linker hoisted'.split(' ')); } - }); - - afterAll(async () => { - await spawnPackageManager(['run', 'link:remove']); + + return async () => { + await spawnPackageManager(['run', 'link:remove']); + delete process.env.NODE_INSTALLER; + } }); const beforeInitTest = (params?: Partial, beforeInit?: BeforeInitFunction) => { From 33454a5bfa5578b166e5525f2257fb92896ae172 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 5 Feb 2025 13:51:54 -0800 Subject: [PATCH 23/30] Update packages/utils/core-utils/spec/package-manager.spec.ts Co-authored-by: Kevin Cui --- packages/utils/core-utils/spec/package-manager.spec.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/utils/core-utils/spec/package-manager.spec.ts b/packages/utils/core-utils/spec/package-manager.spec.ts index 38831f6a63..1070cb5915 100644 --- a/packages/utils/core-utils/spec/package-manager.spec.ts +++ b/packages/utils/core-utils/spec/package-manager.spec.ts @@ -13,12 +13,12 @@ vi.mock('find-up', async (importOriginal) => { describe('package-manager', () => { describe('npm_config_user_agent', () => { - let originalUa: string | undefined; beforeAll(() => { - originalUa = process.env.npm_config_user_agent; - }); - afterAll(() => { - process.env.npm_config_user_agent = originalUa; + const originalUa = process.env.npm_config_user_agent; + + return () => { + process.env.npm_config_user_agent = originalUa; + } }); it.each([ From f3ff6236e34a7e5fffce96c307c5c8f1d14ac91d Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 5 Feb 2025 13:59:46 -0800 Subject: [PATCH 24/30] afterall --- packages/api/core/spec/slow/api.slow.spec.ts | 60 ++++++++++--------- .../core-utils/spec/package-manager.spec.ts | 6 +- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/packages/api/core/spec/slow/api.slow.spec.ts b/packages/api/core/spec/slow/api.slow.spec.ts index 4e6469b1e8..ff78287366 100644 --- a/packages/api/core/spec/slow/api.slow.spec.ts +++ b/packages/api/core/spec/slow/api.slow.spec.ts @@ -39,11 +39,11 @@ describe.each([{ pm: 'npm' }, { pm: 'yarn' }, { pm: 'pnpm' }])(`init (with $pm)` if (pm === 'pnpm') { await spawnPackageManager('config set node-linker hoisted'.split(' ')); } - + return async () => { - await spawnPackageManager(['run', 'link:remove']); - delete process.env.NODE_INSTALLER; - } + await spawnPackageManager(['run', 'link:remove']); + delete process.env.NODE_INSTALLER; + }; }); const beforeInitTest = (params?: Partial, beforeInit?: BeforeInitFunction) => { @@ -132,6 +132,10 @@ describe.each([{ pm: 'npm' }, { pm: 'yarn' }, { pm: 'pnpm' }])(`init (with $pm)` describe('init (with a templater sans required Forge version)', () => { beforeAll(async () => { dir = await ensureTestDirIsNonexistent(); + + return async () => { + await fs.promises.rm(dir, { recursive: true, force: true }); + }; }); it('should fail in initializing', async () => { @@ -142,15 +146,15 @@ describe.each([{ pm: 'npm' }, { pm: 'yarn' }, { pm: 'pnpm' }])(`init (with $pm)` }) ).rejects.toThrow(/it does not specify its required Forge version\.$/); }); - - afterAll(async () => { - await fs.promises.rm(dir, { recursive: true, force: true }); - }); }); describe('init (with a templater with a non-matching Forge version)', () => { beforeAll(async () => { dir = await ensureTestDirIsNonexistent(); + + return async () => { + await fs.promises.rm(dir, { recursive: true, force: true }); + }; }); it('should fail in initializing', async () => { @@ -161,15 +165,15 @@ describe.each([{ pm: 'npm' }, { pm: 'yarn' }, { pm: 'pnpm' }])(`init (with $pm)` }) ).rejects.toThrow(/is not compatible with this version of Electron Forge/); }); - - afterAll(async () => { - await fs.promises.rm(dir, { recursive: true, force: true }); - }); }); describe('init (with a nonexistent templater)', () => { beforeAll(async () => { dir = await ensureTestDirIsNonexistent(); + + return async () => { + await fs.promises.rm(dir, { recursive: true, force: true }); + }; }); it('should fail in initializing', async () => { @@ -180,10 +184,6 @@ describe.each([{ pm: 'npm' }, { pm: 'yarn' }, { pm: 'pnpm' }])(`init (with $pm)` }) ).rejects.toThrow('Failed to locate custom template'); }); - - afterAll(async () => { - await fs.promises.rm(dir, { recursive: true, force: true }); - }); }); describe('import', () => { @@ -193,6 +193,10 @@ describe.each([{ pm: 'npm' }, { pm: 'yarn' }, { pm: 'pnpm' }])(`init (with $pm)` execSync(`git clone https://github.com/electron/electron-quick-start.git . --quiet`, { cwd: dir, }); + + return async () => { + await fs.promises.rm(dir, { recursive: true, force: true }); + }; }); it('creates forge.config.js and can successfully package the application', async () => { @@ -215,10 +219,6 @@ describe.each([{ pm: 'npm' }, { pm: 'yarn' }, { pm: 'pnpm' }])(`init (with $pm)` expect(outDirContents).toHaveLength(1); expect(outDirContents[0]).toEqual(`ProductName-${process.platform}-${process.arch}`); }); - - afterAll(async () => { - await fs.promises.rm(dir, { recursive: true, force: true }); - }); }); describe('Electron Forge API', () => { @@ -254,6 +254,10 @@ describe.each([{ pm: 'npm' }, { pm: 'yarn' }, { pm: 'pnpm' }])(`init (with $pm)` packageJSON.homepage = 'http://www.example.com/'; packageJSON.author = 'Test Author'; }); + + return async () => { + await fs.promises.rm(dir, { recursive: true, force: true }); + }; }); it('throws an error when all is set', async () => { @@ -294,18 +298,18 @@ describe.each([{ pm: 'npm' }, { pm: 'yarn' }, { pm: 'pnpm' }])(`init (with $pm)` describe('with prebuilt native module deps installed', () => { beforeAll(async () => { await installDeps(dir, ['ref-napi']); + + return async () => { + await fs.promises.rm(path.resolve(dir, 'node_modules/ref-napi'), { recursive: true, force: true }); + await updatePackageJSON(dir, async (packageJSON) => { + delete packageJSON.dependencies['ref-napi']; + }); + }; }); it('can package without errors', async () => { await api.package({ dir }); }); - - afterAll(async () => { - await fs.promises.rm(path.resolve(dir, 'node_modules/ref-napi'), { recursive: true, force: true }); - await updatePackageJSON(dir, async (packageJSON) => { - delete packageJSON.dependencies['ref-napi']; - }); - }); }); it('can package without errors', async () => { @@ -479,8 +483,6 @@ describe.each([{ pm: 'npm' }, { pm: 'yarn' }, { pm: 'pnpm' }])(`init (with $pm)` }); }); }); - - afterAll(() => fs.promises.rm(dir, { recursive: true, force: true })); }); }); }); diff --git a/packages/utils/core-utils/spec/package-manager.spec.ts b/packages/utils/core-utils/spec/package-manager.spec.ts index 1070cb5915..e618bc306a 100644 --- a/packages/utils/core-utils/spec/package-manager.spec.ts +++ b/packages/utils/core-utils/spec/package-manager.spec.ts @@ -1,5 +1,5 @@ import findUp from 'find-up'; -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; import { resolvePackageManager } from '../src/package-manager'; @@ -15,10 +15,10 @@ describe('package-manager', () => { describe('npm_config_user_agent', () => { beforeAll(() => { const originalUa = process.env.npm_config_user_agent; - + return () => { process.env.npm_config_user_agent = originalUa; - } + }; }); it.each([ From 519aa9f31e288db61eb780193e39653b5156ae9a Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 5 Feb 2025 14:09:55 -0800 Subject: [PATCH 25/30] clarify cleanup function --- .../core-utils/spec/package-manager.spec.ts | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/packages/utils/core-utils/spec/package-manager.spec.ts b/packages/utils/core-utils/spec/package-manager.spec.ts index e618bc306a..541983526b 100644 --- a/packages/utils/core-utils/spec/package-manager.spec.ts +++ b/packages/utils/core-utils/spec/package-manager.spec.ts @@ -1,5 +1,5 @@ import findUp from 'find-up'; -import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; import { resolvePackageManager } from '../src/package-manager'; @@ -49,21 +49,25 @@ describe('package-manager', () => { }); describe('NODE_INSTALLER', () => { - let nodeInstaller: string | undefined; + let initialNodeInstallerValue: string | undefined; beforeEach(() => { - nodeInstaller = process.env.NODE_INSTALLER; + initialNodeInstallerValue = process.env.NODE_INSTALLER; delete process.env.NODE_INSTALLER; // NODE_INSTALLER is deprecated for Electron Forge 8 and throws a console.warn that we want to silence in tests vi.spyOn(console, 'warn').mockImplementation(() => undefined); - }); - afterEach(() => { - if (!nodeInstaller) { - delete process.env.NODE_INSTALLER; - } else { - process.env.NODE_INSTALLER = nodeInstaller; - } - vi.restoreAllMocks(); + + return () => { + // For cleanup, we want to restore process.env.NODE_INSTALLER. + // If it wasn't explicitly set before, we delete the value set during the test. + // Otherwise, we restore the initial value. + if (!initialNodeInstallerValue) { + delete process.env.NODE_INSTALLER; + } else { + process.env.NODE_INSTALLER = initialNodeInstallerValue; + } + vi.restoreAllMocks(); + }; }); it.each(['yarn', 'npm', 'pnpm'])('should return yarn if NODE_INSTALLER=yarn', async (pm) => { From 8fb93be2b8e13263977010418298b413bea0d78b Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 5 Feb 2025 23:30:15 -0800 Subject: [PATCH 26/30] Update packages/utils/core-utils/spec/package-manager.spec.ts Co-authored-by: Erik Moura --- packages/utils/core-utils/spec/package-manager.spec.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/utils/core-utils/spec/package-manager.spec.ts b/packages/utils/core-utils/spec/package-manager.spec.ts index 541983526b..a40b8fb4bf 100644 --- a/packages/utils/core-utils/spec/package-manager.spec.ts +++ b/packages/utils/core-utils/spec/package-manager.spec.ts @@ -58,14 +58,7 @@ describe('package-manager', () => { vi.spyOn(console, 'warn').mockImplementation(() => undefined); return () => { - // For cleanup, we want to restore process.env.NODE_INSTALLER. - // If it wasn't explicitly set before, we delete the value set during the test. - // Otherwise, we restore the initial value. - if (!initialNodeInstallerValue) { - delete process.env.NODE_INSTALLER; - } else { - process.env.NODE_INSTALLER = initialNodeInstallerValue; - } + process.env.NODE_INSTALLER = initialNodeInstallerValue; vi.restoreAllMocks(); }; }); From 12168acb1eaa7e9cd723d51bba105877d3891e68 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Thu, 6 Feb 2025 12:08:20 -0800 Subject: [PATCH 27/30] Revert "Update packages/utils/core-utils/spec/package-manager.spec.ts" This reverts commit 8fb93be2b8e13263977010418298b413bea0d78b. --- packages/utils/core-utils/spec/package-manager.spec.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/utils/core-utils/spec/package-manager.spec.ts b/packages/utils/core-utils/spec/package-manager.spec.ts index a40b8fb4bf..541983526b 100644 --- a/packages/utils/core-utils/spec/package-manager.spec.ts +++ b/packages/utils/core-utils/spec/package-manager.spec.ts @@ -58,7 +58,14 @@ describe('package-manager', () => { vi.spyOn(console, 'warn').mockImplementation(() => undefined); return () => { - process.env.NODE_INSTALLER = initialNodeInstallerValue; + // For cleanup, we want to restore process.env.NODE_INSTALLER. + // If it wasn't explicitly set before, we delete the value set during the test. + // Otherwise, we restore the initial value. + if (!initialNodeInstallerValue) { + delete process.env.NODE_INSTALLER; + } else { + process.env.NODE_INSTALLER = initialNodeInstallerValue; + } vi.restoreAllMocks(); }; }); From f19e1cb7cebadd05c78ce3bf843a3c1adf12235f Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Thu, 6 Feb 2025 12:58:28 -0800 Subject: [PATCH 28/30] Update packages/utils/core-utils/spec/package-manager.spec.ts Co-authored-by: Erik Moura --- packages/utils/core-utils/spec/package-manager.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/core-utils/spec/package-manager.spec.ts b/packages/utils/core-utils/spec/package-manager.spec.ts index a40b8fb4bf..030a2eafff 100644 --- a/packages/utils/core-utils/spec/package-manager.spec.ts +++ b/packages/utils/core-utils/spec/package-manager.spec.ts @@ -63,7 +63,7 @@ describe('package-manager', () => { }; }); - it.each(['yarn', 'npm', 'pnpm'])('should return yarn if NODE_INSTALLER=yarn', async (pm) => { + it.each([{ pm: 'yarn' }, { pm: 'npm' }, { pm: 'pnpm' }])('should return $pm if NODE_INSTALLER=$pm', async ({ pm }) => { process.env.NODE_INSTALLER = pm; await expect(resolvePackageManager()).resolves.toHaveProperty('executable', pm); }); From 2e96421a6c6b8fec8fe6dfc7545a5e2705bccbc4 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Thu, 6 Feb 2025 14:33:13 -0800 Subject: [PATCH 29/30] adjust tests and findUp --- .../core-utils/spec/package-manager.spec.ts | 2 +- .../utils/core-utils/src/electron-version.ts | 5 ++--- .../utils/core-utils/src/package-manager.ts | 17 +++++++++-------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/utils/core-utils/spec/package-manager.spec.ts b/packages/utils/core-utils/spec/package-manager.spec.ts index 240c8fd5f5..b0a02a772d 100644 --- a/packages/utils/core-utils/spec/package-manager.spec.ts +++ b/packages/utils/core-utils/spec/package-manager.spec.ts @@ -83,7 +83,7 @@ describe('package-manager', () => { }); }); - it('should equal the system package manager value if above strategies failed', async () => { + it('should use the package manager for the nearest ancestor lockfile if detected', async () => { vi.mocked(findUp).mockResolvedValue('yarn.lock'); await expect(resolvePackageManager()).resolves.toHaveProperty('executable', 'yarn'); }); diff --git a/packages/utils/core-utils/src/electron-version.ts b/packages/utils/core-utils/src/electron-version.ts index 7fe5d959f0..cc117795f3 100644 --- a/packages/utils/core-utils/src/electron-version.ts +++ b/packages/utils/core-utils/src/electron-version.ts @@ -1,11 +1,10 @@ import path from 'node:path'; import debug from 'debug'; +import findUp from 'find-up'; import fs from 'fs-extra'; import semver from 'semver'; -import { findLockFile } from './package-manager'; - const d = debug('electron-forge:electron-version'); const electronPackageNames = ['electron-nightly', 'electron']; @@ -21,7 +20,7 @@ function findElectronDep(dep: string): boolean { async function findAncestorNodeModulesPath(dir: string, packageName: string): Promise { d('Looking for a lock file to indicate the root of the repo'); - const lockPath = await findLockFile(dir); + const lockPath = await findUp(['package-lock.json', 'yarn.lock', 'pnpm-lock.yaml'], { cwd: dir, type: 'file' }); if (lockPath) { d(`Found lock file: ${lockPath}`); const nodeModulesPath = path.join(path.dirname(lockPath), 'node_modules', packageName); diff --git a/packages/utils/core-utils/src/package-manager.ts b/packages/utils/core-utils/src/package-manager.ts index 34f3c93e0a..a873a8a97c 100644 --- a/packages/utils/core-utils/src/package-manager.ts +++ b/packages/utils/core-utils/src/package-manager.ts @@ -1,8 +1,11 @@ import { CrossSpawnArgs, CrossSpawnOptions, spawn } from '@malept/cross-spawn-promise'; import chalk from 'chalk'; +import debug from 'debug'; import findUp from 'find-up'; import logSymbols from 'log-symbols'; +const d = debug('electron-forge:package-manager'); + export type SupportedPackageManager = 'yarn' | 'npm' | 'pnpm'; export type PMDetails = { executable: SupportedPackageManager; version?: string; install: string; dev: string; exact: string }; @@ -52,13 +55,6 @@ function pmFromUserAgent() { }; } -/** - * Finds a lockfile in the ancestor directories of the given directory - */ -export async function findLockFile(dir: string) { - return await findUp(['package-lock.json', 'yarn.lock', 'pnpm-lock.yaml'], { cwd: dir, type: 'file' }); -} - /** * Resolves the package manager to use. In order, it checks the following: * @@ -74,7 +70,7 @@ export async function findLockFile(dir: string) { */ export const resolvePackageManager: () => Promise = async () => { const executingPM = pmFromUserAgent(); - const lockfile = await findLockFile(process.cwd()); + const lockfile = await findUp(['package-lock.json', 'yarn.lock', 'pnpm-lock.yaml', 'pnpm-workspace.yaml'], { type: 'file' }); const lockfilePM = (lockfile && PM_FROM_LOCKFILE[lockfile]) ?? undefined; const installer = process.env.NODE_INSTALLER || executingPM?.name || lockfilePM; @@ -87,6 +83,9 @@ export const resolvePackageManager: () => Promise = async () => { case 'yarn': case 'npm': case 'pnpm': + d( + `Resolved package manager to ${installer}. (Derived from NODE_INSTALLER: ${process.env.NODE_INSTALLER}, npm_config_user_agent: ${executingPM}, lockfile: ${lockfilePM}.)` + ); return { ...MANAGERS[installer], version: executingPM?.version }; default: if (installer !== undefined) { @@ -94,6 +93,8 @@ export const resolvePackageManager: () => Promise = async () => { logSymbols.warning, chalk.yellow(`Package manager ${chalk.red(installer)} is unsupported. Falling back to ${chalk.green('npm')} instead.`) ); + } else { + d(`No package manager detected. Falling back to npm.`); } return MANAGERS['npm']; } From 782502201602aa596e61c10289f968465fe46cc6 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Fri, 7 Feb 2025 15:57:09 -0800 Subject: [PATCH 30/30] support `hoist-pattern` and `public-hoist-pattern` as well --- packages/api/cli/spec/check-system.spec.ts | 29 ++++++++++++++++-- packages/api/cli/src/util/check-system.ts | 34 ++++++++++++++++++---- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/packages/api/cli/spec/check-system.spec.ts b/packages/api/cli/spec/check-system.spec.ts index 62b79575c9..7cc30c3ea0 100644 --- a/packages/api/cli/spec/check-system.spec.ts +++ b/packages/api/cli/spec/check-system.spec.ts @@ -59,7 +59,7 @@ describe('checkPackageManager', () => { await expect(checkPackageManager()).rejects.toThrow(); }); - it('should throw if using pnpm without node-linker=hoisted', async () => { + it('should throw if using pnpm without node-linker=hoisted or custom hoist-pattern', async () => { vi.mocked(resolvePackageManager).mockResolvedValue({ executable: 'pnpm', install: 'add', @@ -69,6 +69,10 @@ describe('checkPackageManager', () => { vi.mocked(spawnPackageManager).mockImplementation((args) => { if (args?.join(' ') === 'config get node-linker') { return Promise.resolve('isolated'); + } else if (args?.join(' ') === 'config get hoist-pattern') { + return Promise.resolve('undefined'); + } else if (args?.join(' ') === 'config get public-hoist-pattern') { + return Promise.resolve('undefined'); } else if (args?.join(' ') === '--version') { return Promise.resolve('10.0.0'); } else { @@ -76,10 +80,31 @@ describe('checkPackageManager', () => { } }); await expect(checkPackageManager()).rejects.toThrow( - 'When using pnpm, `node-linker` must be set to "hoisted". Run `pnpm config set node-linker hoisted` to set this config value.' + 'When using pnpm, `node-linker` must be set to "hoisted" (or a custom `hoist-pattern` or `public-hoist-pattern` must be defined). Run `pnpm config set node-linker hoisted` to set this config value, or add it to your project\'s `.npmrc` file.' ); }); + it.each(['hoist-pattern', 'public-hoist-pattern'])('should pass without validation if user has set %s in their pnpm config', async (cfg) => { + vi.mocked(resolvePackageManager).mockResolvedValue({ + executable: 'pnpm', + install: 'add', + dev: '--dev', + exact: '--exact', + }); + vi.mocked(spawnPackageManager).mockImplementation((args) => { + if (args?.join(' ') === 'config get node-linker') { + return Promise.resolve('isolated'); + } else if (args?.join(' ') === `config get ${cfg}`) { + return Promise.resolve('["*eslint*","*babel*"]'); + } else if (args?.join(' ') === '--version') { + return Promise.resolve('10.0.0'); + } else { + return Promise.resolve('undefined'); + } + }); + await expect(checkPackageManager()).resolves.not.toThrow(); + }); + // resolvePackageManager optionally returns a `version` if `npm_config_user_agent` was used to // resolve the package manager being used. it('should not shell out to child process if version was already parsed via npm_config_user_agent', async () => { diff --git a/packages/api/cli/src/util/check-system.ts b/packages/api/cli/src/util/check-system.ts index 30f2eac0a8..17de67482a 100644 --- a/packages/api/cli/src/util/check-system.ts +++ b/packages/api/cli/src/util/check-system.ts @@ -27,15 +27,37 @@ async function checkNodeVersion() { return process.versions.node; } -async function checkPnpmNodeLinker() { - const nodeLinkerValue = await spawnPackageManager(['config', 'get', 'node-linker']); +/** + * Packaging an app with Electron Forge requires `node_modules` to be on disk. + * With `pnpm`, this can be done in a few different ways. + * + * `node-linker=hoisted` replicates the behaviour of npm and Yarn Classic, while + * users may choose to set `public-hoist-pattern` or `hoist-pattern` for advanced + * configuration purposes. + */ +async function checkPnpmConfig() { + const hoistPattern = await spawnPackageManager(['config', 'get', 'hoist-pattern']); + const publicHoistPattern = await spawnPackageManager(['config', 'get', 'public-hoist-pattern']); - if (nodeLinkerValue !== 'hoisted') { - throw new Error('When using pnpm, `node-linker` must be set to "hoisted". Run `pnpm config set node-linker hoisted` to set this config value.'); + if (hoistPattern !== 'undefined' || publicHoistPattern !== 'undefined') { + d( + `Custom hoist pattern detected ${JSON.stringify({ + hoistPattern, + publicHoistPattern, + })}, assuming that the user has configured pnpm to package dependencies.` + ); + return; + } + + const nodeLinker = await spawnPackageManager(['config', 'get', 'node-linker']); + if (nodeLinker !== 'hoisted') { + throw new Error( + 'When using pnpm, `node-linker` must be set to "hoisted" (or a custom `hoist-pattern` or `public-hoist-pattern` must be defined). Run `pnpm config set node-linker hoisted` to set this config value, or add it to your project\'s `.npmrc` file.' + ); } } -// TODO(v8): Drop antiquated versions of npm +// TODO(erickzhao): Drop antiquated versions of npm for Forge v8 const ALLOWLISTED_VERSIONS: Record> = { npm: { all: '^3.0.0 || ^4.0.0 || ~5.1.0 || ~5.2.0 || >= 5.4.2', @@ -65,7 +87,7 @@ export async function checkPackageManager() { } if (pm.executable === 'pnpm') { - await checkPnpmNodeLinker(); + await checkPnpmConfig(); } return `${pm.executable}@${versionString}`;