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

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

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


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
7 checks passed

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

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 commented Nov 30, 2023 •

edited

Loading

MarshallOfSound Nov 30, 2023

Sambyah9595 Jan 16, 2024

felixrieseberg Nov 30, 2023

MarshallOfSound Nov 30, 2023

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

Conversation

MarshallOfSound commented Nov 30, 2023 • edited Loading

MarshallOfSound Nov 30, 2023

Choose a reason for hiding this comment

Sambyah9595 Jan 16, 2024

Choose a reason for hiding this comment

felixrieseberg Nov 30, 2023

Choose a reason for hiding this comment

MarshallOfSound Nov 30, 2023

Choose a reason for hiding this comment

MarshallOfSound commented Nov 30, 2023 •

edited

Loading