Skip to content

Commit

Permalink
fix: ensure that webpack is run once per arch for universal builds (#…
Browse files Browse the repository at this point in the history
…3433)

* fix: ensure that webpack is run once per arch for universal builds

On universal webpack builds we capture native_modules from node_modules
and store them in .webpack, these means the packager level rebuild hook
doesn't rebuild anything.  We need to rebuild once-per-arch which means
we also need to webpack once-per-arch.

Currently this is fairly naive and missing some things but is safe to land
in it's current form and we can improve on it later.
* We build webpack runs sequentially, this is because they share a node_modules
  folder, we could probably instead run webpack once and map native_modules to
  their resolved webpack locations and rebuild N times but only run webpack once
* We hardcode the universal -> arm64/x64 mapping, this can probably be extended to
  resolving all packager matrixes

* build: fix webpack test tsc
  • Loading branch information
MarshallOfSound authored Nov 30, 2023
1 parent a7efb59 commit f3cd9c6
Show file tree
Hide file tree
Showing 4 changed files with 273 additions and 165 deletions.
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[]));
} 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'];
}

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

0 comments on commit f3cd9c6

Please sign in to comment.