fix: ensure that webpack is run once per arch for universal builds by MarshallOfSound · Pull Request #3433 · electron/forge

MarshallOfSound · 2023-11-30T10:03:52Z

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

MarshallOfSound · 2023-11-30T10:05:02Z

packages/api/core/src/util/plugin-interface.ts

                  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[]));


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

felixrieseberg · 2023-11-30T17:38:08Z

packages/plugin/webpack/src/WebpackPlugin.ts

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


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

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 😅

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

MarshallOfSound requested a review from a team as a code owner November 30, 2023 10:03

MarshallOfSound commented Nov 30, 2023

View reviewed changes

erikian approved these changes Nov 30, 2023

View reviewed changes

felixrieseberg reviewed Nov 30, 2023

View reviewed changes

felixrieseberg approved these changes Nov 30, 2023

View reviewed changes

MarshallOfSound added 2 commits November 30, 2023 11:07

build: fix webpack test tsc

97497de

MarshallOfSound force-pushed the cry branch from f0db9a1 to 97497de Compare November 30, 2023 19:07

MarshallOfSound merged commit f3cd9c6 into main Nov 30, 2023

MarshallOfSound deleted the cry branch November 30, 2023 20:36

TheRealCuran mentioned this pull request Dec 4, 2023

[REGRESSION] webpack-plugin generates different output for arm64 and x64 builds on Mac OS, breaking universal builds #3436

Closed

3 tasks

MarshallOfSound mentioned this pull request Dec 4, 2023

refactor: only run webpack once for multi-arch packages #3437

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

fix: ensure that webpack is run once per arch for universal builds#3433
MarshallOfSound merged 2 commits intomainfrom
cry

MarshallOfSound commented Nov 30, 2023 •

edited

Loading

Uh oh!

MarshallOfSound Nov 30, 2023

Uh oh!

Sambyah9595 Jan 16, 2024

Uh oh!

felixrieseberg Nov 30, 2023

Uh oh!

MarshallOfSound Nov 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

MarshallOfSound commented Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MarshallOfSound Nov 30, 2023

Choose a reason for hiding this comment

Uh oh!

Sambyah9595 Jan 16, 2024

Choose a reason for hiding this comment

Uh oh!

felixrieseberg Nov 30, 2023

Choose a reason for hiding this comment

Uh oh!

MarshallOfSound Nov 30, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MarshallOfSound commented Nov 30, 2023 •

edited

Loading