Fix: Vite Build Error (vite:worker-import-meta-url)

[icheered]

I encountered the same issue mentioned by #11 where emscripten does not specify that the web worker is a module and thus during build time vite cannot resolve the package. This PR achieves exactly the same as #11 but now during a post processing step in the build script so that type: 'module' is always added to the import of the web worker module, also in future versions of this package.

[RReverser]

Hm this seems like a bug in Vite tbh. After all, the generated Web Worker is not an ES module and doesn't need {type: 'module'} - it's a regular script that, like any regular script, uses dynamic import(). Only static imports need parent to be ESM.

[RReverser]

Is this essentially https://github.com/vitejs/vite/issues/8427? If so, does adding the mentioned workaround from that issue in your app also solve the problem?

I'd rather not add bundler-specific workarounds, especially with rewriting code like this, if it can be solved somewhere higher up.

[icheered]

It does appear to be a bug in Vite, but seeing as issues are open going back to 2021 this bug appears to be so niche that it is unlikely to get fixed soon. I understand wanting to avoid bundler-specific workarounds but Vite is quite popular with modern frameworks and I tested this fix with your demo and it still worked fine. In other words: It's not breaking stuff (AFAIK), but it is fixing a breaking bug in an upstream tool.

The workaround mentioned in the referenced issue (https://github.com/vitejs/vite/issues/8427#issuecomment-1149775516) works but only during development, during build it still give the mentioned error.

I also have to admit it feels kind of dirty using sed to fix the problem, but since the problem is not solvable through emscripten (I tried, even with newer versions) and the only other solution appears to be Vite fixing their stuff, it feels like this is the only option we have other than telling people to manually do a find-and-replace their node_modules/web-gphoto2/build/libapi.mjs which feels even worse.

[RReverser]

but since the problem is not solvable through emscripten (I tried, even with newer versions)

Well it's not solved yet but certainly solvable :) I submitted a PR upstream. https://github.com/emscripten-core/emscripten/pull/20452

I'd suggest using sed workaround in your own app for now, and then it will be a matter of compiling with 3.1.48 when it's merged and out.

I'll keep this PR open for others who stumble upon this issue and also in case upstream PR is rejected for some reason, but I don't think it will be a problem.

[RReverser]

The upstream PR was merged; I decided not to wait for 3.1.48 for now, and upgraded to 3.1.47 and had to fix bunch of build issues in the process (I believe you mentioned them in your npm package PR).

Then I manually pulled newer Emscripten for this Worker fix, as well as removed the old gpp_rethrow helper as it's no longer necessary with new Emscripten.

Since it's a fairly large version gap with lots of changes, I'd welcome some testing before I publish it to npm. (I pushed rebuilt files to the main branch.)

[RReverser]

@icheered Just checking in - did this solve your issue?

[icheered]

@RReverser updating the emscripten results in the same error I ran into (also the reason why the build is currently failing). This is after running npm run build:wasm

emmake: error: 'make -j4' failed (returned 2)
Error: Process completed with exit code 1.

I'm not sure what the cause is, even after a few hours of troubleshooting :/

[RReverser]

Ah weird, maybe related to the em-config thing I tried. Can you try previous 758b5bb95b95f2029d0e69f4a8d44805b5c2bbe7 which is successful on CI?

[icheered]

Weirdly enough, when running the passing build locally I still get the same error. This happens both on my laptop (running PopOS) and my desktop (running Fedora). This is a part of the log:

lib/m4sugar/m4sh.m4:699: AS_IF is expanded from...
./lib/autoconf/general.m4:2894: _AC_LINK_IFELSE is expanded from...
./lib/autoconf/general.m4:2911: AC_LINK_IFELSE is expanded from...
./lib/autoconf/general.m4:2920: AC_TRY_LINK is expanded from...
libgphoto2_port/gphoto-m4/gp-va-copy.m4:18: GP_VA_COPY is expanded from...
configure.ac:235: the top level
configure.ac:397: warning: The macro `AC_HEADER_STDC' is obsolete.
configure.ac:397: You should run autoupdate.
./lib/autoconf/headers.m4:704: AC_HEADER_STDC is expanded from...
configure.ac:397: the top level
autoreconf: running: /usr/bin/autoheader --force
autoreconf: running: automake --add-missing --copy --force-missing
configure.ac:110: installing 'auto-aux/compile'
configure.ac:17: installing 'auto-aux/missing'
camlibs/Makefile.am: installing 'auto-aux/depcomp'
autoreconf: Leaving directory '.'
rm deps/libgphoto2/configure
emmake: error: 'make -j16' failed (returned 2)

Does building locally work for you?

Eitherway, when I try to take the build files from the repo (last updated in https://github.com/GoogleChromeLabs/web-gphoto2/commit/850fbaecbff5004ef5a279e6c860a8ae7934c308 ) and build it using Vite I still get the same error as before (but that was before you updated emscripten so that isn't surprising).

[RReverser]

Does building locally work for you?

Yeah it worked for me locally and works on CI, so that's surprising.

Did you clean your build folder? Try rm -rf deps and build again maybe.

[icheered]

rm -rf deps fixed the build issue. We are very close. Building using Vite still fails with the same error as before. Inspecting libapi.mjs shows that the file now generates: worker = new Worker(new URL("libapi.worker.js", import.meta.url), { type: "module" })

Moving the closing bracket after import.meta.url to AFTER the {type: "module"} fixes the vite build issue... worker = new Worker(new URL("libapi.worker.js", import.meta.url, { type: "module" }))

[RReverser]

Um that's invalid new URL invocation though. {type: "module"} is new Worker's option, not new URL's. It doesn't make any sense...

[icheered]

I am just as confused as you are because it is indeed an incorrect function call... I created a 'minimal' reproduction here: https://github.com/icheered/gphoto2-vite-build-error (instructions in readme).

I genuinely do not understand why it doesn't work, and even less why it works with a fix that shouldn't be valid...

[RReverser]

That seems like something to discuss at Vite repo. I definitely don't want to ship an invocation that would be invalid in browsers & other bundlers just to work around an issue in one bundler. Sorry.

[RReverser]

FWIW the official Vite docs also show that they should accept regular new Worker(new URL(..., import.meta.url)), with or without type: 'module' so this definitely looks like a Vite bug that's worth reporting. https://vitejs.dev/guide/features#import-with-constructors

[icheered]

I created an issue at Vite, but after some testing I'm still unsure whether this problem is on our or their side. The invalid new URL invocation is definitely on their side, but it seems that the libapi.mjs and libapi.worker.js are in a circular dependency. I narrowed the content of the files down to the following:

// main.js
import Module from './libapi.mjs';

// libapi.mjs
var Module = (() => {
    return function () {
        new Worker(new URL("libapi.worker.js", import.meta.url));
    };
})();

export default Module;

// libapi.worker.js
var Module = {};

self.onmessage = event => {
    if (event.data.cmd === "load") {
        import("./libapi.mjs");
    }
};

In Vite 4.4.2 this gives the error discussed before, in Vite 5 (released this week) the build process gets into an infinite loop until it crashes due to a memory leak.

[RReverser]

but it seems that the libapi.mjs and libapi.worker.js are in a circular dependency

Sigh, +1 to the number of times I had to discuss this 😅

Yes, it's sort of circular dependency, but it's instantiating a Web Worker, which is basically a separate entry point that runs on a separate thread, and not the same as importing a circular module. Even module circular dependencies are fine and are supported by ESM and the only reason they're discouraged is because it's easy to make mistakes with execution order if one is not careful.

But with Workers in particular there is no actual cycle since code is executed separately on different threads so execution order is always deterministic as each thread has only one possible entry point.

[RReverser]

This comment from linked original / duplicate issue is spot-on: https://github.com/vitejs/vite/issues/7015#issuecomment-1751479906

The problem is that some bundlers tend to take a shortcut and treat new Worker in the same way they would import(...), when they should be treating new Worker as a separate entry point.

[RReverser]

That said, I don't understand why your fix with incorrect syntax helps with this particular issue... Is it maybe that, because it's invalid, it's just ignored and not bundled by Vite at all, so resolution just works in runtime then?