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

icheered · 2023-10-09T10:06:14Z

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 · 2023-10-09T11:49:27Z

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 · 2023-10-09T11:56:47Z

Is this essentially vitejs/vite#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 · 2023-10-09T12:38:40Z

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 (vitejs/vite#8427 (comment)) 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.

Even in ESM mode, we use dynamic import to load main code from the Web Worker, so, technically, our Workers dont *have* to be ESM as well - they're compatible with regular script mode too. Unfortunately, unlike browsers, some bundlers care about the distinction and don't bundle Workers correctly when `new Worker(...)` is used without `{type: 'module'}`. Some recent examples where I've seen this: - issue with parcel bundler: parcel-bundler/parcel#8727 (comment) - issue with esbuild bundler used by popular vite toolchain: GoogleChromeLabs/web-gphoto2#12 In both cases adding `{type: 'module'}` fixes the build. One difference this change will make is that `{type: 'module'}` forces Worker to be loaded as a module which always runs in strict JS mode. Our main code should already be compatible with `use strict`, or it wouldn't work as ESM at all, and our `src/worker.js` code has `use strict` at the top, so it's compatible too, so I don't expect any issues, but it's something worth keeping in mind if we get breakage reports. Also note that older browsers without Module Workers support will ignore `{type: 'module'}` and run Workers in script mode, so as long as we don't use static imports or `import.meta.url` in `src/worker.js`, this is a backward-compatible change.

RReverser · 2023-10-13T17:44:51Z

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. emscripten-core/emscripten#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.

Even in ESM mode, we use dynamic import to load main code from the Web Worker, so, technically, our Workers dont *have* to be ESM as well - they're compatible with regular script mode too. Unfortunately, unlike browsers, some bundlers care about the distinction and don't bundle Workers correctly when `new Worker(...)` is used without `{type: 'module'}`. Some recent examples where I've seen this: - issue with parcel bundler: parcel-bundler/parcel#8727 (comment) - issue with esbuild bundler used by popular vite toolchain: GoogleChromeLabs/web-gphoto2#12 In both cases adding `{type: 'module'}` fixes the build. One difference this change will make is that `{type: 'module'}` forces Worker to be loaded as a module which always runs in strict JS mode. Our main code should already be compatible with `use strict`, or it wouldn't work as ESM at all, and our `src/worker.js` code has `use strict` at the top, so it's compatible too, so I don't expect any issues, but it's something worth keeping in mind if we get breakage reports. Also note that older browsers without Module Workers support will ignore `{type: 'module'}` and run Workers in script mode, so as long as we don't use static imports or `import.meta.url` in `src/worker.js`, this is a backward-compatible change.

RReverser · 2023-10-17T02:48:05Z

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 · 2023-11-15T00:49:23Z

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

icheered · 2023-11-20T12:03:55Z

@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 · 2023-11-20T12:54:51Z

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

icheered · 2023-11-20T13:35:14Z

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 850fbae ) 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 · 2023-11-20T13:45:47Z

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 · 2023-11-20T14:03:52Z

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 · 2023-11-20T14:09:08Z

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 · 2023-11-20T14:43:26Z

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 · 2023-11-20T14:55:34Z

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 · 2023-11-20T22:40:22Z

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 · 2023-11-21T11:24:51Z

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 · 2023-11-21T13:41:09Z

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 · 2023-11-21T13:44:24Z

This comment from linked original / duplicate issue is spot-on: vitejs/vite#7015 (comment)

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 · 2023-11-21T13:45:26Z

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?

Add post processing step in build script

fc03494

icheered mentioned this pull request Oct 9, 2023

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

Closed

RReverser mentioned this pull request Oct 13, 2023

Load Workers with {type:'module'} in ESM mode emscripten-core/emscripten#20452

Merged

RReverser closed this in 5652da7 Oct 17, 2023

icheered mentioned this pull request Nov 21, 2023

Running vite build throws error when creating new Worker vitejs/vite#15056

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

icheered commented Oct 9, 2023

RReverser commented Oct 9, 2023 •

edited

Loading

RReverser commented Oct 9, 2023

icheered commented Oct 9, 2023

RReverser commented Oct 13, 2023

RReverser commented Oct 17, 2023

RReverser commented Nov 15, 2023

icheered commented Nov 20, 2023

RReverser commented Nov 20, 2023

icheered commented Nov 20, 2023 •

edited

Loading

RReverser commented Nov 20, 2023

icheered commented Nov 20, 2023 •

edited

Loading

RReverser commented Nov 20, 2023

icheered commented Nov 20, 2023 •

edited

Loading

RReverser commented Nov 20, 2023

RReverser commented Nov 20, 2023

icheered commented Nov 21, 2023

RReverser commented Nov 21, 2023 •

edited

Loading

RReverser commented Nov 21, 2023

RReverser commented Nov 21, 2023

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

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

Conversation

icheered commented Oct 9, 2023

RReverser commented Oct 9, 2023 • edited Loading

RReverser commented Oct 9, 2023

icheered commented Oct 9, 2023

RReverser commented Oct 13, 2023

RReverser commented Oct 17, 2023

RReverser commented Nov 15, 2023

icheered commented Nov 20, 2023

RReverser commented Nov 20, 2023

icheered commented Nov 20, 2023 • edited Loading

RReverser commented Nov 20, 2023

icheered commented Nov 20, 2023 • edited Loading

RReverser commented Nov 20, 2023

icheered commented Nov 20, 2023 • edited Loading

RReverser commented Nov 20, 2023

RReverser commented Nov 20, 2023

icheered commented Nov 21, 2023

RReverser commented Nov 21, 2023 • edited Loading

RReverser commented Nov 21, 2023

RReverser commented Nov 21, 2023

RReverser commented Oct 9, 2023 •

edited

Loading

icheered commented Nov 20, 2023 •

edited

Loading

icheered commented Nov 20, 2023 •

edited

Loading

icheered commented Nov 20, 2023 •

edited

Loading

RReverser commented Nov 21, 2023 •

edited

Loading