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

fix: wasm functions return undefined #11

Closed
wants to merge 5 commits into from
Closed

fix: wasm functions return undefined #11

wants to merge 5 commits into from

Conversation

sigoden
Copy link

@sigoden sigoden commented Sep 7, 2022

This pr solve #1.

Why some wasm functions return undefined?

If you import a wasm js module:

import { parse } from "@jsona/openapi";
console.log(parse("{}"));

vite will compile this to:

import { parse } from "/node_modules/@jsona/openapi/index_bg.js?v=bb17ff0c"; 

NOTE: vite append a version hash to module path.

vite-wasm-plugin will resolve this to:

import __vite__initWasm from "/__vite-plugin-wasm-helper"
import { __wbindgen_json_parse as __vite__wasmImport_0_0 } from "/node_modules/@jsona/openapi/index_bg.js";
const __vite__wasmModule = await __vite__initWasm({ "./index_bg.js": { __wbindgen_json_parse: __vite__wasmImport_0_0 } }, "/node_modules/@jsona/openapi/index_bg.wasm?init");
export const memory = __vite__wasmModule.memory;
export const parse = __vite__wasmModule.parse;
export const __wbindgen_malloc = __vite__wasmModule.__wbindgen_malloc;
export const __wbindgen_realloc = __vite__wasmModule.__wbindgen_realloc;

vite-wasm-plugin imports the importer module /node_modules/@jsona/openapi/index_bg.js without a version hash.

That's the problem. They are two different modules because their paths are different.

If you run non-local wasm js module(vite don't add version hash to non-node_module files), call a function depends on __wbindgen* function, it will return undefined.

src/index.ts Outdated
@@ -33,14 +35,20 @@ export default function wasm(): Plugin {
// Make a call to Vite's internal `fileToUrl` function by calling Vite's original WASM plugin's load()
const originalLoadResult = (await originalWasmPlugin.load.call(this, id + "?init")) as string;
const url = JSON.parse(/".+"/g.exec(originalLoadResult.trim().split("\n")[1])[0]) as string;
const importUrls = await Promise.all(
imports.map(async ({ from }) => {
const importerPath = id.slice(0, id.lastIndexOf("/")) + from.slice(from.lastIndexOf("/"));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand this path concatenation for importerPath, what would happen if the WASM file imports a JS file that are not in the same directory? e.g. when from is ../../../a/b/c/xxx.js.

Copy link
Author

@sigoden sigoden Sep 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just replace file name:

id: 'C:/t/test-wasm2/node_modules/@jsona/openapi/index_bg.wasm'
from: "./index_bg.js"
importerPath: "C:/t/test-wasm2/node_modules/@jsona/openapi/index_bg.js"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if

id: 'C:/t/test-wasm2/node_modules/@jsona/openapi/a/b/c/d/e/index_bg.wasm'
from: "../../f/g/index_bg.js"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The index_bg.wasm always point to index_bg.js. in same directory.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only appies to wasm-bindgen, you can't assume all WASM files are generated by wasm-bindgen. I don't see in the WASM ESM spec it has any restrictions on the imported files' directory.

@Menci
Copy link
Owner

Menci commented Sep 7, 2022

Could you add some test that fails without this PR and passes with it? I personally couldn't reproduce this issue.

@sigoden
Copy link
Author

sigoden commented Sep 7, 2022

This happend in dev mode, I'm afraid the tests need to be refactored.

Files to reproduce this issue.

package.json

{
  "scripts": {
    "dev": "vite",
    "build": "vite build",
    "preview": "vite preview"
  },
  "dependencies": {
    "@jsona/openapi": "^0.2.1",
    "vite": "^3.1.0",
    "vite-plugin-wasm": "^2.1.0",
    "vite-plugin-top-level-await": "^1.1.1"
  }
}

vite.config.js

import { defineConfig } from "vite";
import wasm from "vite-plugin-wasm";
import topLevelAwait from "vite-plugin-top-level-await";

export default defineConfig({
  plugins: [
    wasm(),
    topLevelAwait(),
  ]
})

index.html

<body>
  <script type="module" src="/main.js"></script>
</body>

main.js

import { parse } from "@jsona/openapi";
console.log(parse("{}"));

src/index.ts Outdated Show resolved Hide resolved
@Menci
Copy link
Owner

Menci commented Sep 21, 2022

I merged your code with latest main changes and added unit test. Some circular dependency happened in @syntect/wasm was found. Could you please check the error?

@sigoden
Copy link
Author

sigoden commented Sep 22, 2022

vitejs/vite#10141 fix version query.

The easiest way to fix this problem now is to exclude wasm js module from pre-bundling (vite >= 3.1.2)

  optimizeDeps: {
    exclude: [
      "@jsona/openapi"
    ]
  },

In order to solve this problem perfectly, vite needs to provide api reverse lookup importer.

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

Successfully merging this pull request may close these issues.

None yet

2 participants