bytecodealliance / javy

JS to WebAssembly toolchain
Apache License 2.0
2.1k stars 101 forks source link

Add escape hatches to and from `quickjs_wasm_sys` #610

Closed surma closed 5 months ago

surma commented 5 months ago

While quickjs_wasm_rs provides a much more ergonomic API, there are some features of QuickJS that are not exposed. For example, exotic object behaviors or QuickJS’s execution timeout.

In the long-run, it would be great to have those in quickjs_wasm_rs, but in the meantime, it seems like a good idea regardless to me expose escape hatches to the low-level crate.

Specifically:

surma commented 5 months ago

I'd be good though to have a list of features that you think we're missing in quickjs_wasm_rs to be able to get get rid of these "escape hatches", so I'd appreciate if you could list those more concretely if you can.

I mean the short answer is: All QuickJS features 😅

Specifically for my explorations right now, I want to make use of the following QuickJS features:

In general I think these escape hatches are valuable and I don’t think they really pose a risk, but it’s totally your call.

saulecabrera commented 5 months ago

In general I think these escape hatches are valuable and I don’t think they really pose a risk, but it’s totally your call.

I think they are too IMO, and we should land this, given that we currently don't cover the entire surface area of the QuickJS API. But in the future, ideally we should support all the features in a safe way, hence my comment about not making any stability guarantees. And as mentioned above, these new methods are are not forcing anyone to use any unsafe operations.

Exotic Methods, which are a part of a QuickJS class definition. It effectively allows you to implement a JS class whose shape and implementation is backed by Rust

Oh ok, that makes sense. This is probably going to be useful for https://github.com/bytecodealliance/javy/issues/605

jeffcharles commented 5 months ago

Would there be any value in putting the re-export of quickjs-wasm-sys and the escape hatches behind a cargo feature? That might help communicate the stability guarantees in code as well as in documentation.

surma commented 5 months ago

@jeffcharles I don’t think I have seen that be done before, but I like the suggestion.

saulecabrera commented 5 months ago

It seems like one of the bindings tests is failing, I think it could be related to the fix in as_str, because all the other changes seem unrelated.

jeffcharles commented 5 months ago

Please update the CHANGELOG file for quickjs-wasm-rs and update the version of the crate to 3.1.0-alpha.1 since this adds meaningful new features.

surma commented 5 months ago

@jeffcharles @saulecabrera I think this is good to go