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

add option to retain fetchEvent handler #116

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion crates/spidermonkey-embedding-splicer/src/bindgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,12 @@ pub struct Componentization {
pub resource_imports: Vec<(String, String, u32)>,
}

pub fn componentize_bindgen(resolve: &Resolve, id: WorldId, name: &str) -> Componentization {
pub fn componentize_bindgen(
resolve: &Resolve,
id: WorldId,
name: &str,
retain_fetch_event: bool,
) -> Componentization {
let mut bindgen = JsBindgen {
src: Source::default(),
esm_bindgen: EsmBindgen::default(),
Expand Down Expand Up @@ -342,6 +347,7 @@ pub fn componentize_bindgen(resolve: &Resolve, id: WorldId, name: &str) -> Compo
"$source_mod",
&mut bindgen.local_names,
name,
retain_fetch_event,
);

let js_intrinsics = render_intrinsics(&mut bindgen.all_intrinsics, false, true);
Expand Down Expand Up @@ -1001,6 +1007,7 @@ impl EsmBindgen {
imports_object: &str,
_local_names: &mut LocalNames,
source_name: &str,
retain_fetch_event: bool,
) {
// TODO: bring back these validations of imports
// including using the flattened bindings
Expand Down Expand Up @@ -1043,6 +1050,10 @@ impl EsmBindgen {
");
}
for (export_name, binding) in &self.exports {
// If we are retaining the fetch event, then we should just let StarlingMonkey handle the event directly.
if retain_fetch_event && export_name == "wasi:http/[email protected]" {
continue;
}
match binding {
Binding::Interface(bindings) => {
uwrite!(output, "const ");
Expand Down
27 changes: 23 additions & 4 deletions crates/spidermonkey-embedding-splicer/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use anyhow::{bail, Context, Result};
use bindgen::BindingItem;
use std::path::{Path, PathBuf};
use std::{
collections::HashSet,
path::{Path, PathBuf},
};

mod bindgen;
mod splice;
Expand Down Expand Up @@ -111,6 +114,7 @@ impl Guest for SpidermonkeyEmbeddingSplicerComponent {
wit_path: Option<String>,
world_name: Option<String>,
debug: bool,
mut retain_fetch_event: bool,
) -> Result<SpliceResult, String> {
let source_name = source_name.unwrap_or("source.js".to_string());

Expand All @@ -131,7 +135,22 @@ impl Guest for SpidermonkeyEmbeddingSplicerComponent {
.map_err(|e| e.to_string())?;

let mut wasm_bytes = wit_component::dummy_module(&resolve, world);
let componentized = bindgen::componentize_bindgen(&resolve, world, &source_name);
let componentized =
bindgen::componentize_bindgen(&resolve, world, &source_name, retain_fetch_event);

let target_world = &resolve.worlds[world];
let mut target_world_exports = HashSet::new();

for (key, _) in &target_world.exports {
target_world_exports.insert(resolve.name_world_key(key));
}

// Do not retain fetch event if the target world does not export the
// incomingHandler Interface as that would make it incompatible with the
// target world
if !target_world_exports.contains("wasi:http/[email protected]") {
retain_fetch_event = false;
}

// merge the engine world with the target world, retaining the engine producers
let producers = if let Ok((
Expand Down Expand Up @@ -330,8 +349,8 @@ impl Guest for SpidermonkeyEmbeddingSplicerComponent {
// println!("{:?}", &imports);
// println!("{:?}", &componentized.imports);
// println!("{:?}", &exports);
let mut wasm =
splice::splice(engine, imports, exports, debug).map_err(|e| format!("{:?}", e))?;
let mut wasm = splice::splice(engine, imports, exports, debug, retain_fetch_event)
.map_err(|e| format!("{:?}", e))?;

// add the world section to the spliced wasm
wasm.push(section.id());
Expand Down
24 changes: 16 additions & 8 deletions crates/spidermonkey-embedding-splicer/src/splice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub fn splice(
imports: Vec<(String, String, CoreFn, Option<i32>)>,
exports: Vec<(String, CoreFn)>,
debug: bool,
retain_fetch_event: bool,
) -> Result<Vec<u8>> {
let mut config = walrus::ModuleConfig::new();
if debug {
Expand All @@ -53,13 +54,16 @@ pub fn splice(
module.exports.delete(expt.id());
module.funcs.delete(run);
}
if let Ok(serve) = module
.exports
.get_func("wasi:http/[email protected]#handle")
{
let expt = module.exports.get_exported_func(serve).unwrap();
module.exports.delete(expt.id());
module.funcs.delete(serve);

if !retain_fetch_event {
if let Ok(serve) = module
.exports
.get_func("wasi:http/[email protected]#handle")
{
let expt = module.exports.get_exported_func(serve).unwrap();
module.exports.delete(expt.id());
module.funcs.delete(serve);
}
}

// we reencode the WASI world component data, so strip it out from the
Expand All @@ -79,7 +83,7 @@ pub fn splice(
synthesize_import_functions(&mut module, &imports)?;

// create the exported functions as wrappers around the "cabi_call" function
synthesize_export_functions(&mut module, &exports)?;
synthesize_export_functions(&mut module, &exports, retain_fetch_event)?;

Ok(module.emit_wasm())
}
Expand Down Expand Up @@ -530,6 +534,7 @@ fn synthesize_import_functions(
fn synthesize_export_functions(
module: &mut walrus::Module,
exports: &Vec<(String, CoreFn)>,
retain_fetch_event: bool,
) -> Result<()> {
let cabi_realloc = get_export_fid(
module,
Expand Down Expand Up @@ -561,6 +566,9 @@ fn synthesize_export_functions(
let arg_ptr = module.locals.add(ValType::I32);
let ret_ptr = module.locals.add(ValType::I32);
for (export_num, (expt_name, expt_sig)) in exports.iter().enumerate() {
if retain_fetch_event && expt_name == "wasi:http/[email protected]#handle" {
continue;
}
// Export function synthesis
{
// add the function type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,5 @@ world spidermonkey-embedding-splicer {

export stub-wasi: func(engine: list<u8>, features: list<features>, wit-world: option<string>, wit-path: option<string>, world-name: option<string>) -> result<list<u8>, string>;

export splice-bindings: func(source-name: option<string>, spidermonkey-engine: list<u8>, wit-world: option<string>, wit-path: option<string>, world-name: option<string>, debug: bool) -> result<splice-result, string>;
export splice-bindings: func(source-name: option<string>, spidermonkey-engine: list<u8>, wit-world: option<string>, wit-path: option<string>, world-name: option<string>, debug: bool, retain-fetch-event: bool ) -> result<splice-result, string>;
}
4 changes: 3 additions & 1 deletion src/componentize.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export async function componentize(jsSource, witWorld, opts) {
worldName,
disableFeatures = [],
enableFeatures = [],
retainFetchEvent = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it would make sense to integrate this into the enableFeatures model and call it fetch-event?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oooh! I like the idea of adding it via the enableFeatures. I will also add it to the readme.

} = opts || {};

let { wasm, jsBindings, importWrappers, exports, imports } = spliceBindings(
Expand All @@ -53,7 +54,8 @@ export async function componentize(jsSource, witWorld, opts) {
witWorld,
maybeWindowsPath(witPath),
worldName,
false
false,
retainFetchEvent
);

// we never disable a feature that is already in the target world usage
Expand Down
Loading