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: ensure that webpack is run once per arch for universal builds #3433

Merged
merged 2 commits into from
Nov 30, 2023
Merged
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/util/plugin-interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export default class PluginInterface implements IForgePluginInterface {
async (_, __, task) => {
if ((hook as any).__hookName) {
// Also give it the task
await (hook as any).call(task, this.config, ...(hookArgs as any[]));
return await (hook as any).call(task, this.config, ...(hookArgs as any[]));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows internal hooks to return sub-listrs to do some pretty terminal stuff

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cc

} else {
await hook(this.config, ...hookArgs);
}
Expand Down
89 changes: 73 additions & 16 deletions packages/plugin/webpack/src/WebpackPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import path from 'path';

import { getElectronVersion, listrCompatibleRebuildHook } from '@electron-forge/core-utils';
import { namedHookWithTaskFn, PluginBase } from '@electron-forge/plugin-base';
import { ForgeMultiHookMap, ResolvedForgeConfig, StartResult } from '@electron-forge/shared-types';
import { ForgeListrTaskDefinition, ForgeMultiHookMap, ResolvedForgeConfig, StartResult } from '@electron-forge/shared-types';
import Logger, { Tab } from '@electron-forge/web-multi-logger';
import chalk from 'chalk';
import debug from 'debug';
Expand Down Expand Up @@ -158,20 +158,67 @@ export default class WebpackPlugin extends PluginBase<WebpackPluginConfig> {

this.isProd = true;
await fs.remove(this.baseDir);
await listrCompatibleRebuildHook(
this.projectDir,
await getElectronVersion(this.projectDir, await fs.readJson(path.join(this.projectDir, 'package.json'))),
platform,
arch,
config.rebuildConfig,
task,
chalk.cyan('[plugin-webpack] ')
);
}, 'Preparing native dependencies'),
namedHookWithTaskFn<'prePackage'>(async () => {
await this.compileMain();
await this.compileRenderers();
}, 'Building webpack bundles'),

// TODO: Figure out how to get matrix from packager
let arches: string[] = [arch];
if (arch === 'universal') {
arches = ['arm64', 'x64'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh hardcoding this is fine, making it dynamic is probably not worth the squeeze

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is defs the 99% case, the 1% case is folks running package --arch=all but idk if that even works at the moment let alone with webpack 😅

}

return task.newListr(
arches.map(
(pArch): ForgeListrTaskDefinition => ({
title: `Building webpack bundle for ${chalk.magenta(pArch)}`,
task: async (_, task) => {
return task.newListr(
[
{
title: 'Preparing native dependencies',
task: async (_, innerTask) => {
await listrCompatibleRebuildHook(
this.projectDir,
await getElectronVersion(this.projectDir, await fs.readJson(path.join(this.projectDir, 'package.json'))),
platform,
pArch,
config.rebuildConfig,
innerTask
);
},
options: {
persistentOutput: true,
bottomBar: Infinity,
showTimer: true,
},
},
{
title: 'Building webpack bundles',
task: async () => {
await this.compileMain();
await this.compileRenderers();
// Store it in a place that won't get messed with
// We'll restore the right "arch" in the afterCopy hook further down
const targetDir = path.resolve(this.baseDir, pArch);
await fs.mkdirp(targetDir);
for (const child of await fs.readdir(this.baseDir)) {
if (!arches.includes(child)) {
await fs.move(path.resolve(this.baseDir, child), path.resolve(targetDir, child));
}
}
},
options: {
showTimer: true,
},
},
],
{ concurrent: false }
);
},
})
),
{ concurrent: false }
// eslint-disable-next-line @typescript-eslint/no-explicit-any
) as any;
}, 'Preparing webpack bundles'),
],
postStart: async (_config, child) => {
d('hooking electron process exit');
Expand All @@ -181,7 +228,17 @@ export default class WebpackPlugin extends PluginBase<WebpackPluginConfig> {
});
},
resolveForgeConfig: this.resolveForgeConfig,
packageAfterCopy: this.packageAfterCopy,
packageAfterCopy: [
async (_forgeConfig: ResolvedForgeConfig, buildPath: string, _electronVersion: string, _platform: string, pArch: string): Promise<void> => {
// Restore the correct 'arch' build of webpack
// Steal the correct arch, wipe the folder, move it back to pretend to be ".webpack" root
const tmpWebpackDir = path.resolve(buildPath, '.webpack.tmp');
await fs.move(path.resolve(buildPath, '.webpack', pArch), tmpWebpackDir);
await fs.remove(path.resolve(buildPath, '.webpack'));
await fs.move(tmpWebpackDir, path.resolve(buildPath, '.webpack'));
},
this.packageAfterCopy,
],
};
}

Expand Down
8 changes: 3 additions & 5 deletions packages/plugin/webpack/test/WebpackConfig_spec.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
import path from 'path';

import { expect } from 'chai';
import { Compiler, Configuration, Entry, WebpackPluginInstance } from 'webpack';
import { Configuration, Entry } from 'webpack';

import { WebpackConfiguration, WebpackPluginConfig, WebpackPluginEntryPoint } from '../src/Config';
import AssetRelocatorPatch from '../src/util/AssetRelocatorPatch';
import WebpackConfigGenerator, { ConfigurationFactory } from '../src/WebpackConfig';

const mockProjectDir = process.platform === 'win32' ? 'C:\\path' : '/path';

type WebpackPlugin = ((this: Compiler, compiler: Compiler) => void) | WebpackPluginInstance;

function hasAssetRelocatorPatchPlugin(plugins?: WebpackPlugin[]): boolean {
return (plugins || []).some((plugin: WebpackPlugin) => plugin instanceof AssetRelocatorPatch);
function hasAssetRelocatorPatchPlugin(plugins?: Required<Configuration>['plugins']): boolean {
return (plugins || []).some((plugin) => plugin && typeof plugin === 'object' && plugin instanceof AssetRelocatorPatch);
}

const sampleWebpackConfig = {
Expand Down
Loading