Mutability in captured value with NativeFunction closure

[theoparis]

I've wrapped boa's context in a "Engine" struct because I want to add features similar to Deno's allow-env and allow-net. However, I'm encountering the following error when I call NativeFunction::from_copy_closure_with_captures as it seems to expect a non-mutable reference to my engine, which won't work because then I can't get a mutable boa context from it.

error[E0308]: mismatched types
  --> crates/engine/require.rs:80:34
   |
80 |                     let module = require_modu...
   |                                  -------------- ^^^^^^ types differ in mutability
   |                                  |
   |                                  arguments to this function are incorrect
   |
   = note: expected mutable reference `&mut Engine<'_>`
                      found reference `&&mut Engine<'_>`
note: function defined here
  --> crates/engine/require.rs:8:8
   |
8  | pub fn require_module(
   |        ^^^^^^^^^^^^^^
9  |     engine: &mut Engine<'_>,
   |     -----------------------

Code for the require function:

use boa_engine::{
	prelude::JsObject, property::Attribute, JsError, JsResult, JsValue,
	NativeFunction, Source,
};

use crate::Engine;

pub fn require_module(engine: &mut Engine<'_>, uri: &str) -> JsResult<JsValue> {
	let mut ctx = &mut engine.context;

	// If the uri contains file://, then we can load the file
	// otherwise we can use reqwest to load the file from the internet
	// or we can use the std::fs::read_to_string to load the file from the local filesystem
	if uri.starts_with("file://") && engine.allow_filesystem {
		let path = uri.replace("file://", "");
		let buffer = std::fs::read_to_string(&path)
			.map_err(|e| JsError::from_opaque(JsValue::from(e.to_string())))?;

		setup_module(&mut engine)?;

		ctx.eval_script(Source::from_bytes(&buffer))?;

		let global_obj = ctx.global_object().to_owned();
		let module = global_obj.get("module", ctx)?;
		module.as_object().unwrap().get("exports", ctx)
	} else if uri.starts_with("http://")
		|| uri.starts_with("https://") && engine.allow_network
	{
		let response = reqwest::blocking::get(uri).map_err(|err| {
			JsError::from_opaque(JsValue::from(err.to_string()))
		})?;
		let buffer = response.text().map_err(|err| {
			JsError::from_opaque(JsValue::from(err.to_string()))
		})?;

		setup_module(&mut engine)?;

		ctx.eval_script(Source::from_bytes(&buffer))?;

		let global_obj = ctx.global_object().to_owned();
		let module = global_obj.get("module", ctx)?;
		module.as_object().unwrap().get("exports", ctx)
	} else {
		Err(JsError::from_opaque(JsValue::from("Invalid URI provided")))
	}
}

pub fn setup_module(engine: &mut Engine) -> JsResult<()> {
	let mut ctx = &mut engine.context;

	unsafe {
		ctx.register_global_callable(
			"require",
			0,
			NativeFunction::from_copy_closure_with_captures(
				move |_: &JsValue, args: &[JsValue], engine, ctx| {
					let arg = args.get(0).unwrap();

					let uri = arg
						.to_string(ctx)
						.expect("Failed to convert to string")
						.to_std_string()
						.map_err(|e| {
							JsError::from_opaque(JsValue::from(e.to_string()))
						})?;

					let module = require_module(engine, &uri)?;

					Ok(module)
				},
				engine,
			),
		);
	}

	let moduleobj = JsObject::default();
	moduleobj.set("exports", JsValue::from(" "), false, ctx)?;
	ctx.register_global_property(
		"module",
		JsValue::from(moduleobj),
		Attribute::default(),
	);

	Ok(())
}

It seems like &T needs to be &mut T 🤔

[jedel1043]

I'm fairly certain we can't lift the requirement of immutable references to the inner captures of a closure, mainly because our current definition of NativeClosure uses only a Gc to store the inner function:

boa/boa_engine/src/native_function.rs

Lines 84 to 88 in f793a27

	#[derive(Clone)]
	enum Inner {
	PointerFn(NativeFunctionPointer),
	Closure(Gc<dyn TraceableClosure>),
	}

If we lifted that requirement, we would need to wrap our closures in a GcRefCell which would add more overhead to every call of the closure for every user, including users who don't need to store mutable state on their closures. Because of this, we opted for storing only Gc<dyn TraceableClosure>s (which forces only immutable references to the captures) and left the option to use interior mutability to our users.

Having said that, one option to solve this would be to use GcRefCell to enable interior mutability in the closure, but that seems suboptimal for this usecase in particular; mainly because Engine is shared by all calls to require(); so maybe we could provide a way to append additional data to every Context. Would that solve your specific usecase?