Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

photon compatibility #19

Open
mukundshah opened this issue Mar 7, 2024 · 8 comments
Open

photon compatibility #19

mukundshah opened this issue Mar 7, 2024 · 8 comments
Assignees

Comments

@mukundshah
Copy link

Environment

  • Operating System: Darwin
  • Node Version: v21.6.2
  • Nitro Version: 2.9.1
  • Package Manager: [email protected]

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

code += `\n ${moduleName}: {`;
for (const name of importNames) {
code += pkgImport
? `\n ${name}: _imports_${moduleName}.${name},\n`
: `\n ${name}: () => { throw new Error("\`${moduleName}.${name}\` is not provided!")},\n`;
}
code += " },\n";

Additional context

No response

Logs

No response

@mukundshah
Copy link
Author

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
Copy link
Member

pi0 commented Mar 7, 2024

I would highly appreciate a runnable reproduction 🙏

@Zn4rK
Copy link

Zn4rK commented Mar 7, 2024

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
Copy link
Author

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

@pi0 pi0 changed the title unwasm breaks if module name is a relative path photon compatibility Mar 8, 2024
@pi0
Copy link
Member

pi0 commented Mar 8, 2024

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 ~> #21)

@pi0 pi0 self-assigned this Mar 8, 2024
@mukundshah
Copy link
Author

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
Copy link
Author

Repro: mukundshah/nitro-wasm#photon

@pi0
Copy link
Member

pi0 commented Mar 14, 2024

Thanks for nice reproduction @mukundshah. Added a fix via #24 keeping this issue open until make sure photon is fully compatible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants