bytecodealliance / ComponentizeJS

JS -> WebAssembly Component
Apache License 2.0
242 stars 32 forks source link

add option to enable weval #134

Closed karthik2804 closed 2 months ago

karthik2804 commented 2 months ago

closes #133

I am not sure how we want to modify the tests, Do we want to run all the tests for the weval build as well? I am happy to update it as necessary.

karthik2804 commented 2 months ago

I am not sure why the tests for the weval build fail in the way they are failing. The generated .d.ts files from the transpile step are different. Adding only the difference in the types

regular build

export function run(): void;

weval build

import { WasiCliRun } from './interfaces/wasi-cli-run.js';
import { WasiHttpIncomingHandler } from './interfaces/wasi-http-incoming-handler.js';
export const run: typeof WasiCliRun;
export const incomingHandler: typeof WasiHttpIncomingHandler;

I will try to investigate this more.

I originally was passing in the original engine and not the spliced one which leads to a different error message

TypeError: 0 is not a function
Redirecting call to abort() to mozalloc_abort

wasm://wasm/048817aa:1

RuntimeError: unreachable
    at  (wasm://wasm/048817aa:wasm-function[14041]:0x8c8986)
    at  (wasm://wasm/048817aa:wasm-function[14791]:0x8ccbc6)
    at  (wasm://wasm/048817aa:wasm-function[470]:0x36e8d1)
    at run (wasm://wasm/048817aa:wasm-function[14790]:0x8ccbba)
    at run (file:///Users/karthik_ganeshram/Work/wasm-tools-contrib/forks/fermyon/ComponentizeJS/test/output/console-object/console-object.js:5668:12)
    at file:///Users/karthik_ganeshram/Work/wasm-tools-contrib/forks/fermyon/ComponentizeJS/test/output/console-object/run.js:3:9

Node.js v22.6.0

    1) console-object
karthik2804 commented 2 months ago

@guybedford I believe I have the same arguments being passed to both but I still run into the error. I feel like I am missing something obvious.

guybedford commented 2 months ago

The failing test remaining seems like it is due to Weval taking slightly longer to build.

Updating the test time diff from 5 seconds to 10 or 15 seconds like the following should fix:

  // verify now was taken at build time (within than 10 seconds ago)
  ok(Number(times[0]) < curNow);
  ok(Number(times[0]) > curNow - 10_000);
}
karthik2804 commented 2 months ago

@guybedford thank you for the final test fix and helping get this PR over the line!