-
Notifications
You must be signed in to change notification settings - Fork 31
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
deps: update to latest starlingmonkey #111
Conversation
Thank you for this PR! This is great to see how to update starlingmonkey. When building this PR and running the tests using
|
@karthik2804 make sure you are definitely not using a debug build or referencing the debug build in this project, as that error is only associated with debug builds. |
I just attempted it with a fresh clone of the project with the following commands and I get the same error git clone https://github.com/bytecodealliance/ComponentizeJS
cd ComponentizeJS
gh pr checkout 111
npm install
git submodule update --init --recursive
npm run build && npm run test Builtins
1) console-object
✔ console-simple (2177ms)
✔ crypto-random-disabled (2222ms)
✔ crypto-random (2230ms)
2) globals
✔ math-random-disabled (2206ms)
✔ math-random (2173ms)
✔ now-disabled (2211ms)
✔ now (2215ms)
✔ pure (2152ms)
✔ timeout-disabled (2183ms)
3) timeout
Bindings
✔ args (2116ms)
Splicer error: unable to read prelude sequence, continuing for debug build but note binding functions will not work!
4) char
Splicer error: unable to read prelude sequence, continuing for debug build but note binding functions will not work!
5) conventions
✔ empty (2116ms)
Splicer error: unable to read prelude sequence, continuing for debug build but note binding functions will not work!
6) flags
Splicer error: unable to read prelude sequence, continuing for debug build but note binding functions will not work!
flavorful.bindings.js:147:32 RangeError: invalid or out-of-range index
Redirecting call to abort() to mozalloc_abort
7) flavorful
Splicer error: unable to read prelude sequence, continuing for debug build but note binding functions will not work!
8) floats
Splicer error: unable to read prelude sequence, continuing for debug build but note binding functions will not work!
hello.bindings.js:111:32 RangeError: offset is outside the bounds of the DataView
Redirecting call to abort() to mozalloc_abort
9) hello
Splicer error: unable to read prelude sequence, continuing for debug build but note binding functions will not work!
http-request.js:24:45 TypeError: futureIncomingResponse.get() is undefined
Redirecting call to abort() to mozalloc_abort
10) http-request
Splicer error: unable to read prelude sequence, continuing for debug build but note binding functions will not work!
✔ import-func (3910ms) |
Ahh thanks for spotting that, the splicing functions might need to be updated to the latest binary format here then due to the Spidermonkey upgrade. |
I did a little print line debugging on
The snippet below is a part of the output. InstrSeq {
...
instrs: [
(
LocalGet(
LocalGet {
local: Id {
idx: 21481,
},
},
),
...
} |
@karthik2804 I fixed up the splicing portion of this PR so those tests should be passing now, if that helps. |
@guybedford Thank you! I can confirm that the tests are passing now. I will now attempt to see if I can get fetch to work when building an app with componentize-js. |
@guybedford Looking at the diff with respect to starlingmonkey it no longer includes the fetch rework unless I am missing something. |
@karthik2804 it needs a merge of that work against bytecodealliance/StarlingMonkey#56 now, I've tried to add that, let's see how it looks in CI. |
Fetch seems to be supported but it does look like // hello.js
export function hello(name) {
console.log("in here")
test()
return `Hello ${name}`;
}
async function test() {
console.log("in test")
await new Promise(resolve => setTimeout(resolve, 0));
console.log("after await")
} The result of running in here
in test
Hello ComponentizeJS The output when running from in here
in test
after await
Hello ComponentizeJS This is possibly a |
@karthik2804 I've posted #112 with an outline of an approach, happy to discuss further if it helps. |
@karthik2804 here's what I do to implement async in the mean time - a |
@guybedford Thank you for the example implementation. I am trying to target the |
In that case, an implementation along the lines of #112 sounds like the right kind of approach to me. |
e12ec9e
to
90ec48e
Compare
Updates to the latest version of StarlingMonkey as of bytecodealliance/StarlingMonkey@d7c6348.