unjs / unwasm

🇼 WebAssembly tools for JavaScript
MIT License
178 stars 6 forks source link

photon compatibility #19

Open mukundshah opened 8 months ago

mukundshah commented 8 months ago

Environment

Reproduction

N/A

Describe the bug

If a wasm contains imports from relative path, the module name will have foreign unintended characters.

For context, photon-rs has some imports from relative path, and using it causes the following error:

(inject plugin) rollup-plugin-inject: failed to parse ~/Developer/play/nitro-wasm/routes/wasm/photon_rs_bg.wasm. Consider restricting the plugin to particular files via options.include
routes/wasm/photon_rs_bg.wasm (6:2) Error when using sourcemap for reporting an error: Can't resolve original location of error.

[nitro 6:22:17 PM]  ERROR  RollupError: Unexpected token .. Expected identifier, string literal, numeric literal or [ for the computed key (Note that you need plugins to import files that are not JavaScript)

4: import { base64ToUint8Array } from "unwasm:helpers";
5: const _imports = {
6:   ./photon_rs_bg.js: {
     ^
7:     __wbindgen_object_drop_ref: () => { throw new Error("./photon_rs_bg.js.__wbindgen_object_drop_ref is not provid...

This issue is resolved when moduleName is json-stringified at L180 in https://github.com/unjs/unwasm/blob/67a94a9d4f203db9a9864fcb0b8c2d0912c575c1/src/plugin/runtime.ts#L180-L186

Additional context

No response

Logs

No response

mukundshah commented 8 months ago

I patched it temporarily:

diff --git a/dist/plugin.cjs b/dist/plugin.cjs
index a85e47a4ab4fefb6b25755a78c1dd631e764f040..a76f3b0716d89d2bde8c3d2a9f00064424281061 100644
--- a/dist/plugin.cjs
+++ b/dist/plugin.cjs
@@ -161,7 +161,7 @@ ${code}`;
       resolved = false;
     }
     code += `
-  ${moduleName}: {`;
+  ${JSON.stringify(moduleName)}: {`;
     for (const name of importNames2) {
       code += pkgImport ? `
     ${name}: _imports_${moduleName}.${name},
diff --git a/dist/plugin.mjs b/dist/plugin.mjs
index 6561b2e5906f7a676f68517ae2be6d7f79ecf88b..15226a90b79b1fcee97829b9ebe1051887d9dede 100644
--- a/dist/plugin.mjs
+++ b/dist/plugin.mjs
@@ -153,7 +153,7 @@ ${code}`;
       resolved = false;
     }
     code += `
-  ${moduleName}: {`;
+  ${JSON.stringify(moduleName)}: {`;
     for (const name of importNames2) {
       code += pkgImport ? `
     ${name}: _imports_${moduleName}.${name},
pi0 commented 8 months ago

I would highly appreciate a runnable reproduction 🙏

Zn4rK commented 8 months ago

There are probably easier ways to include a repro, but I ran in to the same problem with Prismas new database adapters.

Here's a repro-link codesandbox.io.

If you try yourself with prisma, it does require you to have generated the prisma client before building:

node_modules/.bin/prisma generate
mukundshah commented 8 months ago

I'm not able to reproduce this at the moment; but @Zn4rK 's reproduction aligns with the issue I had

pi0 commented 8 months ago

Okay, i will try to assmble a reproduction for photon as well to track both separately 👍🏼 There might be same common issues but we always need reproduction to make sure all root causes are fixed.

(prisma ~> https://github.com/unjs/unwasm/issues/21)

mukundshah commented 8 months ago

I will be able to provide a repro for photon by Sunday, as it happened on my work pc; And i couldn't make it happen again on my personal one.

The issue so far is that key in imports object should have been enclosed by quotes.

mukundshah commented 8 months ago

Repro: mukundshah/nitro-wasm#photon

pi0 commented 7 months ago

Thanks for nice reproduction @mukundshah. Added a fix via https://github.com/unjs/unwasm/pull/24 keeping this issue open until make sure photon is fully compatible.