Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Electron dependency path for PnP #3622

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/api/core/src/api/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export default autoTrace(
}

if (!electronExecPath) {
electronExecPath = await locateElectronExecutable(dir, packageJSON);
electronExecPath = locateElectronExecutable(dir, packageJSON);
}

d('Electron binary path:', electronExecPath);
Expand Down
4 changes: 2 additions & 2 deletions packages/api/core/src/util/electron-executable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import logSymbols from 'log-symbols';

type PackageJSON = Record<string, unknown>;

export default async function locateElectronExecutable(dir: string, packageJSON: PackageJSON): Promise<string> {
const electronModulePath: string | undefined = await getElectronModulePath(dir, packageJSON);
export default function locateElectronExecutable(dir: string, packageJSON: PackageJSON): string {
const electronModulePath: string | undefined = getElectronModulePath(dir, packageJSON);

// eslint-disable-next-line @typescript-eslint/no-var-requires
let electronExecPath = require(electronModulePath || path.resolve(dir, 'node_modules/electron'));
Expand Down
2 changes: 1 addition & 1 deletion packages/api/core/test/fast/make_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('make', () => {
const electronPath = path.resolve(__dirname, 'node_modules/electron');
stubbedMake = proxyquire.noCallThru().load('../../src/api/make', {
'@electron-forge/core-utils': {
getElectronModulePath: () => Promise.resolve(electronPath),
getElectronModulePath: () => electronPath,
getElectronVersion: () => Promise.resolve('1.0.0'),
},
}).default;
Expand Down
49 changes: 12 additions & 37 deletions packages/utils/core-utils/src/electron-version.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import path from 'path';

import debug from 'debug';
import findUp from 'find-up';
import fs from 'fs-extra';
import semver from 'semver';

Expand All @@ -20,28 +19,6 @@ function findElectronDep(dep: string): boolean {
return electronPackageNames.includes(dep);
}

async function findAncestorNodeModulesPath(dir: string, packageName: string): Promise<string | undefined> {
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' });
if (lockPath) {
d(`Found lock file: ${lockPath}`);
const nodeModulesPath = path.join(path.dirname(lockPath), 'node_modules', packageName);
if (await fs.pathExists(nodeModulesPath)) {
return nodeModulesPath;
}
}

return Promise.resolve(undefined);
}

async function determineNodeModulesPath(dir: string, packageName: string): Promise<string | undefined> {
const nodeModulesPath: string | undefined = path.join(dir, 'node_modules', packageName);
if (await fs.pathExists(nodeModulesPath)) {
return nodeModulesPath;
}
return findAncestorNodeModulesPath(dir, packageName);
}

export class PackageNotFoundError extends Error {
constructor(packageName: string, dir: string) {
super(`Cannot find the package "${packageName}". Perhaps you need to run "${safeYarnOrNpm()} install" in "${dir}"?`);
Expand All @@ -63,23 +40,21 @@ function getElectronModuleName(packageJSON: PackageJSONWithDeps): string {
return packageName;
}

async function getElectronPackageJSONPath(dir: string, packageName: string): Promise<string | undefined> {
const nodeModulesPath = await determineNodeModulesPath(dir, packageName);
if (!nodeModulesPath) {
throw new PackageNotFoundError(packageName, dir);
}

const electronPackageJSONPath = path.join(nodeModulesPath, 'package.json');
if (await fs.pathExists(electronPackageJSONPath)) {
return electronPackageJSONPath;
function getElectronPackageJSONPath(dir: string, packageName: string): string {
if (packageName) {
try {
// eslint-disable-next-line node/no-missing-require
return require.resolve(`${packageName}/package.json`, { paths: [dir] });
} catch {
// Ignore
}
}

return undefined;
throw new PackageNotFoundError(packageName, dir);
}

export async function getElectronModulePath(dir: string, packageJSON: PackageJSONWithDeps): Promise<string | undefined> {
export function getElectronModulePath(dir: string, packageJSON: PackageJSONWithDeps): string | undefined {
const moduleName = getElectronModuleName(packageJSON);
const packageJSONPath = await getElectronPackageJSONPath(dir, moduleName);
const packageJSONPath = getElectronPackageJSONPath(dir, moduleName);
if (packageJSONPath) {
return path.dirname(packageJSONPath);
}
Expand All @@ -95,7 +70,7 @@ export async function getElectronVersion(dir: string, packageJSON: PackageJSONWi
let version = packageJSON.devDependencies![packageName];
if (!semver.valid(version)) {
// It's not an exact version, find it in the actual module
const electronPackageJSONPath = await getElectronPackageJSONPath(dir, packageName);
const electronPackageJSONPath = getElectronPackageJSONPath(dir, packageName);
if (electronPackageJSONPath) {
const electronPackageJSON = await fs.readJson(electronPackageJSONPath);
version = electronPackageJSON.version;
Expand Down
10 changes: 5 additions & 5 deletions packages/utils/core-utils/test/electron-version_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ describe('getElectronModulePath', () => {
devDependencies: { electron: '^4.0.4' },
};

expect(await getElectronModulePath(fixtureDir, packageJSON)).to.be.equal(path.join(workspaceDir, 'node_modules', 'electron'));
expect(getElectronModulePath(fixtureDir, packageJSON)).to.be.equal(path.join(workspaceDir, 'node_modules', 'electron'));
});

after(() => {
Expand All @@ -150,7 +150,7 @@ describe('getElectronModulePath', () => {
devDependencies: { electron: '^4.0.4' },
};

expect(await getElectronModulePath(fixtureDir, packageJSON)).to.be.equal(path.join(workspaceDir, 'node_modules', 'electron'));
expect(getElectronModulePath(fixtureDir, packageJSON)).to.be.equal(path.join(workspaceDir, 'node_modules', 'electron'));
});

it('finds the top-level electron module despite the additional node_modules folder inside the package', async () => {
Expand All @@ -160,7 +160,7 @@ describe('getElectronModulePath', () => {
devDependencies: { electron: '^4.0.4' },
};

expect(await getElectronModulePath(fixtureDir, packageJSON)).to.be.equal(path.join(workspaceDir, 'node_modules', 'electron'));
expect(getElectronModulePath(fixtureDir, packageJSON)).to.be.equal(path.join(workspaceDir, 'node_modules', 'electron'));
});

it('finds the correct electron module in nohoist mode', async () => {
Expand All @@ -170,8 +170,8 @@ describe('getElectronModulePath', () => {
devDependencies: { electron: '^13.0.0' },
};

expect(await getElectronModulePath(fixtureDir, packageJSON)).to.be.equal(path.join(fixtureDir, 'node_modules', 'electron'));
expect(await getElectronModulePath(fixtureDir, packageJSON)).not.to.be.equal(path.join(workspaceDir, 'node_modules', 'electron'));
expect(getElectronModulePath(fixtureDir, packageJSON)).to.be.equal(path.join(fixtureDir, 'node_modules', 'electron'));
expect(getElectronModulePath(fixtureDir, packageJSON)).not.to.be.equal(path.join(workspaceDir, 'node_modules', 'electron'));
});

after(() => {
Expand Down