wasm-bindgen icon indicating copy to clipboard operation
wasm-bindgen copied to clipboard

fix: revert deno wasm loading template

Open kallebysantos opened this issue 2 months ago • 12 comments

Description

Reverts the Deno wasm loading template

  • closes #4794
  • external closes https://github.com/supabase/edge-runtime/issues/640

Checklist

  • [x] Verified changelog requirement

kallebysantos avatar Nov 10 '25 20:11 kallebysantos

Hi @guybedford 💚

please add a changelog entry for this as well.

Is my first time here 😅 Should I manually update the CHANGELOG.md file?

kallebysantos avatar Nov 10 '25 20:11 kallebysantos

Yes, just manually add a new line noting the change.

The tests also need to be updated here via just test-ui-overwrite. There also seems to be an issue with the one test in the examples exposed by this change although it seems like it is a test suite issue and not the change itself although I'm not 100% sure.

guybedford avatar Nov 10 '25 21:11 guybedford

Seems there is also just test-ui-overwrite to run here as well.

guybedford avatar Nov 10 '25 21:11 guybedford

Seems there is also just test-ui-overwrite to run here as well.

I already did this one ada1037, not really sure if its correctly. Perhaps I need to do some playwright setup?

My `just test` did fail at this point:
Running `/Users/kallebysantos/projects/community/wasm-bindgen/wasm-bindgen/target/debug/wasm-bindgen-test-runner /Users/kallebysantos/projects/community/wasm-bindgen/wasm-bindgen/target/wasm32-unknown-unknown/debug/deps/tests-298ca02a98050a29.wasm`
Running headless tests in Safari on `http://127.0.0.1:52467/`
Try find `webdriver.json` for configure browser's capabilities:
Not found
driver status: signal: 9 (SIGKILL)
Error: http status: 500

kallebysantos avatar Nov 10 '25 21:11 kallebysantos

I already did this one ada1037, not really sure if its correctly.

This is MacOs / Linux / toolchain differences, I think down to spacing.

We likely have a genuine issue with the example, perhaps it's running Deno code in the browser or Node.js. If you're unable to look into it I can try and track it down on Friday.

guybedford avatar Nov 10 '25 22:11 guybedford

Sure! I'm macos based, so its a different environment from the gh test runner

We likely have a genuine issue with the example, perhaps it's running Deno code in the browser

I though the same, I was expecting to Deno.readFile() only be available while generating for deno environment. But seems that is being called from playwright example.

If you're unable to look into it I can try and track it down on Friday.

Please 💚 It will make simpler for both of us, I tried to spent some time installing playwright but got no success 😅 I could run tests inside /examples folder but not the just test-wasm-bindgen-futures

kallebysantos avatar Nov 10 '25 22:11 kallebysantos

Ah I see what you mean re:CI, that log does look weird as if coming from the browser, which is pretty weird as Playwright should just invoke the Deno CLI for these tests.

I'll look into this more tomorrow.

RReverser avatar Nov 11 '25 00:11 RReverser

Sorry for the delay. I looked at it, and looks like this test was just declared with --target deno even though it's not Deno-specific (presumably, it's from time when Symbol.dispose wasn't available in browsers). It passed quietly until this PR because Deno output used standard Web APIs so it was accidentally compatible with browser too.

You should be able to just change this line to --target web:

https://github.com/wasm-bindgen/wasm-bindgen/blob/018641b4dd6752d6d27910661f80e6646f776ce2/examples/explicit-resource-management/package.json#L3


That aside, the earlier question still stands:

Sounds good as a hotfix, but would love to understand why it fails in your scenario but works eg on our CI as well for me locally.

Are you using older Deno version before it added support for fetching local files?

Proper Node.js and Deno tests are in https://github.com/wasm-bindgen/wasm-bindgen/tree/main/examples/nodejs_and_deno and the current Deno output still works fine both locally and on CI, both the one that uses fetch / Web APIs (which is what --target deno produces) and via Wasm ESM integration (--target bundler). You can try those yourself in that folder.

Here's my Deno version just in case:

deno --version deno 2.4.5 (stable, release, x86_64-pc-windows-msvc) v8 13.7.152.14-rusty typescript 5.8.3

RReverser avatar Nov 14 '25 13:11 RReverser

Ah, looking through the linked issues, sounds like this is a workaround for a Deno bug https://github.com/denoland/deno/issues/28129 where it can't fetch files specifically bundled into a compiled executable, even though it works fine for real files on the filesystem?

If so, adding a workaround sounds good for now if unfortunate, but we should add a regression test specifically for that scenario.

RReverser avatar Nov 14 '25 13:11 RReverser

Hi @RReverser 💚 Yes! this PR is to revert the fetch() change, that causes this error on Deno.

I'm not really sure how to add theses tests 😕 I can't even run everything on my machine cause I'm missing some playwright setup

Would be happy If may you guys could help with it?

kallebysantos avatar Nov 14 '25 13:11 kallebysantos

I can't even run everything on my machine cause I'm missing some playwright setup

You can run those without playwright by just doing pnpm test in said directory.

I'm not really sure how to add theses tests 😕

I never used Deno's compiled executables, so not sure either, at least not right away. Maybe someone else can help.

RReverser avatar Nov 14 '25 13:11 RReverser

Guys 😕 I don't really sure if theses CI errors could be related with my PR changes, I did already add the web parameter suggested here https://github.com/wasm-bindgen/wasm-bindgen/pull/4795#issuecomment-3532672672

1) playwright.spec.ts:84:3 › explicit-resource-management ────────────────────────────────────────

    TypeError: Cannot read properties of undefined (reading 'current_allocation')
        at current_allocation (http://localhost/explicit-resource-management/explicit_resource_management.js:36:22)
        at http://localhost/explicit-resource-management/index.html:4:27

    Error: page.goto: Test ended.

kallebysantos avatar Nov 14 '25 13:11 kallebysantos