From 58cf435e79b0851ed65477d430ed921628c5adbb Mon Sep 17 00:00:00 2001 From: richarddavison <89518095+richarddavison@users.noreply.github.com> Date: Thu, 26 Sep 2024 11:00:18 +0200 Subject: [PATCH] Fix & simplify CJS support (#609) --- fixtures/e.cjs | 1 + llrt/src/main.rs | 18 ++--- llrt_core/src/modules/js/@llrt/std.ts | 19 ------ llrt_core/src/vm.rs | 97 ++++++++++++++------------- llrt_modules/src/modules/process.rs | 11 ++- llrt_utils/src/object.rs | 51 +++++++++++++- tests/unit/require.test.ts | 6 ++ 7 files changed, 117 insertions(+), 86 deletions(-) create mode 100644 fixtures/e.cjs diff --git a/fixtures/e.cjs b/fixtures/e.cjs new file mode 100644 index 0000000000..f85a21900d --- /dev/null +++ b/fixtures/e.cjs @@ -0,0 +1 @@ +module.exports.prop = "a"; diff --git a/llrt/src/main.rs b/llrt/src/main.rs index 9a32902ca7..0c931a3a0c 100644 --- a/llrt/src/main.rs +++ b/llrt/src/main.rs @@ -170,18 +170,12 @@ async fn start_cli(vm: &Vm) { let filename = Path::new(arg); let file_exists = filename.exists(); + let global = ext == ".cjs"; + match ext { - ".cjs" => { - if file_exists { - return vm.run_cjs_file(filename, false, true).await; - } else { - eprintln!("No such file: {}", arg); - exit(1); - } - }, - ".js" | ".mjs" => { + ".cjs" | ".js" | ".mjs" => { if file_exists { - return vm.run_esm_file(filename, true, false).await; + return vm.run_file(filename, true, global).await; } else { eprintln!("No such file: {}", arg); exit(1); @@ -189,7 +183,7 @@ async fn start_cli(vm: &Vm) { }, _ => { if file_exists { - return vm.run_esm_file(filename, true, false).await; + return vm.run_file(filename, true, false).await; } eprintln!("Unknown command: {}", arg); usage(); @@ -199,7 +193,7 @@ async fn start_cli(vm: &Vm) { } } } else if let Some(filename) = get_js_path("index") { - vm.run_esm_file(&filename, true, false).await; + vm.run_file(&filename, true, false).await; } } diff --git a/llrt_core/src/modules/js/@llrt/std.ts b/llrt_core/src/modules/js/@llrt/std.ts index 8e8a5c4ac4..3030382582 100644 --- a/llrt_core/src/modules/js/@llrt/std.ts +++ b/llrt_core/src/modules/js/@llrt/std.ts @@ -2,23 +2,4 @@ // SPDX-License-Identifier: Apache-2.0 import "./init"; -Object.defineProperty(globalThis, "module", { - value: new Proxy( - {}, - { - set: __bootstrap.moduleExport, - } - ), - configurable: false, -}); -Object.defineProperty(globalThis, "exports", { - value: new Proxy( - {}, - { - set: __bootstrap.exports, - } - ), - configurable: false, -}); - Object.freeze(__bootstrap); diff --git a/llrt_core/src/vm.rs b/llrt_core/src/vm.rs index 89f0093de2..bec37cb0f0 100644 --- a/llrt_core/src/vm.rs +++ b/llrt_core/src/vm.rs @@ -17,7 +17,11 @@ use llrt_modules::{ path::{join_path_with_separator, resolve_path, resolve_path_with_separator}, timers::{self, poll_timers}, }; -use llrt_utils::{bytes::ObjectBytes, error::ErrorExtensions, object::ObjectExt}; +use llrt_utils::{ + bytes::ObjectBytes, + error::ErrorExtensions, + object::{ObjectExt, Proxy}, +}; use once_cell::sync::Lazy; use ring::rand::SecureRandom; use rquickjs::{ @@ -415,26 +419,14 @@ impl Vm { .await; } - pub async fn run_esm_file(&self, filename: &Path, strict: bool, global: bool) { - self.run( - [ - r#"import(""#, - &filename.to_string_lossy(), - r#"").catch((e) => {{console.error(e);process.exit(1)}})"#, - ] - .concat(), - strict, - global, - ) - .await; - } - pub async fn run_cjs_file(&self, filename: &Path, strict: bool, global: bool) { - let source = std::fs::read(filename).unwrap_or_else(|_| { - panic!( - "{}", - ["No such file: ", &filename.to_string_lossy()].concat() - ) - }); + pub async fn run_file(&self, filename: &Path, strict: bool, global: bool) { + let source = [ + r#"try{require(""#, + &filename.to_string_lossy(), + r#"")}catch(e){console.error(e);process.exit(1)}"#, + ] + .concat(); + self.run(source, strict, global).await; } @@ -540,33 +532,44 @@ fn init(ctx: &Ctx<'_>, module_names: HashSet<&'static str>) -> Result<()> { let require_exports: Arc>> = Arc::new(Mutex::new(None)); let require_exports_ref = require_exports.clone(); let require_exports_ref_2 = require_exports.clone(); + let require_exports_ref_3 = require_exports.clone(); + + let module_proxy_target = Object::new(ctx.clone())?; + let module_proxy = Proxy::with_target(ctx.clone(), module_proxy_target.into_value())?; + module_proxy.setter(Func::from(move |ctx, obj, prop, value| { + let ExportArgs(_, _, _, value) = ExportArgs(ctx, obj, prop, value); + let mut exports = require_exports.lock().unwrap(); + exports.replace(value); + Result::Ok(true) + }))?; + + module_proxy.getter(Func::from(move |ctx, target, prop, receiver| { + let ExportArgs(_, target, _, _) = ExportArgs(ctx, target, prop, receiver); + let mut exports = require_exports_ref_3.lock().unwrap(); + exports.replace(target.as_value().clone()); + Result::Ok(target) + }))?; + + globals.prop("module", module_proxy)?; + + let exports_proxy_target = Object::new(ctx.clone())?; + let exports_proxy = Proxy::with_target(ctx.clone(), exports_proxy_target.into_value())?; + exports_proxy.setter(Func::from(move |ctx, obj, prop, value| { + let ExportArgs(ctx, _, prop, value) = ExportArgs(ctx, obj, prop, value); + let mut exports = require_exports_ref.lock().unwrap(); + let exports = if exports.is_some() { + exports.as_ref().unwrap() + } else { + exports.replace(Object::new(ctx.clone())?.into_value()); + exports.as_ref().unwrap() + }; + exports.as_object().unwrap().set(prop, value)?; + Result::Ok(true) + }))?; - let js_bootstrap = Object::new(ctx.clone())?; - js_bootstrap.set( - "moduleExport", - Func::from(move |ctx, obj, prop, value| { - let ExportArgs(_, _, _, value) = ExportArgs(ctx, obj, prop, value); - let mut exports = require_exports.lock().unwrap(); - exports.replace(value); - Result::Ok(true) - }), - )?; - js_bootstrap.set( - "exports", - Func::from(move |ctx, obj, prop, value| { - let ExportArgs(ctx, _, prop, value) = ExportArgs(ctx, obj, prop, value); - let mut exports = require_exports_ref.lock().unwrap(); - let exports = if exports.is_some() { - exports.as_ref().unwrap() - } else { - exports.replace(Object::new(ctx.clone())?.into_value()); - exports.as_ref().unwrap() - }; - exports.as_object().unwrap().set(prop, value)?; - Result::Ok(true) - }), - )?; - globals.set("__bootstrap", js_bootstrap)?; + globals.prop("exports", exports_proxy)?; + + globals.set("__bootstrap", Object::new(ctx.clone())?)?; globals.set( "require", diff --git a/llrt_modules/src/modules/process.rs b/llrt_modules/src/modules/process.rs index e1c8e498ef..2bdda2f7e4 100644 --- a/llrt_modules/src/modules/process.rs +++ b/llrt_modules/src/modules/process.rs @@ -3,11 +3,10 @@ use std::env; use std::{collections::HashMap, sync::atomic::Ordering}; +use llrt_utils::object::Proxy; use llrt_utils::{module::export_default, result::ResultExt}; use rquickjs::{ - atom::PredefinedAtom, convert::Coerced, - function::Constructor, module::{Declarations, Exports, ModuleDef}, object::Property, prelude::Func, @@ -83,12 +82,10 @@ pub fn init(ctx: &Ctx<'_>) -> Result<()> { } } - let proxy_ctor = globals.get::<_, Constructor>(PredefinedAtom::Proxy)?; - let env_obj = env_map.into_js(ctx)?; - let env_proxy_cfg = Object::new(ctx.clone())?; - env_proxy_cfg.set(PredefinedAtom::Setter, Func::from(env_proxy_setter))?; - let env_proxy = proxy_ctor.construct::<_, Value>((env_obj, env_proxy_cfg))?; + + let env_proxy = Proxy::with_target(ctx.clone(), env_obj)?; + env_proxy.setter(Func::from(env_proxy_setter))?; process.set("env", env_proxy)?; process.set("cwd", Func::from(cwd))?; diff --git a/llrt_utils/src/object.rs b/llrt_utils/src/object.rs index 623f11a4a4..da864e8390 100644 --- a/llrt_utils/src/object.rs +++ b/llrt_utils/src/object.rs @@ -1,6 +1,11 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -use rquickjs::{atom::PredefinedAtom, FromJs, Function, IntoAtom, Object, Result, Symbol, Value}; +use rquickjs::{ + atom::PredefinedAtom, + function::{Constructor, IntoJsFunc}, + prelude::Func, + Ctx, FromJs, Function, IntoAtom, IntoJs, Object, Result, Symbol, Undefined, Value, +}; pub trait ObjectExt<'js> { fn get_optional + Clone, V: FromJs<'js>>(&self, k: K) -> Result>; @@ -35,3 +40,47 @@ impl<'js> CreateSymbol<'js> for Symbol<'js> { for_function.call((description,)) } } + +pub struct Proxy<'js> { + target: Value<'js>, + options: Object<'js>, +} + +impl<'js> IntoJs<'js> for Proxy<'js> { + fn into_js(self, ctx: &Ctx<'js>) -> Result> { + let proxy_ctor = ctx.globals().get::<_, Constructor>(PredefinedAtom::Proxy)?; + + proxy_ctor.construct::<_, Value>((self.target, self.options)) + } +} + +impl<'js> Proxy<'js> { + pub fn new(ctx: Ctx<'js>) -> Result { + let options = Object::new(ctx.clone())?; + Ok(Self { + target: Undefined.into_value(ctx), + options, + }) + } + + pub fn with_target(ctx: Ctx<'js>, target: Value<'js>) -> Result { + let options = Object::new(ctx)?; + Ok(Self { target, options }) + } + + pub fn setter(&self, setter: Func) -> Result<()> + where + T: IntoJsFunc<'js, P> + 'js, + { + self.options.set(PredefinedAtom::Setter, setter)?; + Ok(()) + } + + pub fn getter(&self, getter: Func) -> Result<()> + where + T: IntoJsFunc<'js, P> + 'js, + { + self.options.set(PredefinedAtom::Getter, getter)?; + Ok(()) + } +} diff --git a/tests/unit/require.test.ts b/tests/unit/require.test.ts index 3002f98c9d..0a886e13ec 100644 --- a/tests/unit/require.test.ts +++ b/tests/unit/require.test.ts @@ -30,6 +30,12 @@ it("should handle cjs requires", () => { expect(a.c).toEqual("c"); }); +it("should handle cjs requires", () => { + const a = _require(`${CWD}/fixtures/e.cjs`); + + expect(a.prop).toEqual("a"); +}); + it("should be able to use node module with prefix `node:` with require", () => { let { Console } = require("node:console"); const consoleObj = new Console({