Skip to content

Commit

Permalink
Refactor the structure of the Config/APIs (#646)
Browse files Browse the repository at this point in the history
* Refactor the structure of the `Config`/APIs

This change primarily rewrites the current APIs using "Intrinsics" as defined by rquickjs.

This change has several objectives:

Refactor the structure of the Config and its relationship with the Runtime to pave the way for a truly extensible and configurable engine. The way Intrinsics are exposed in rquickjs simplifies (i) enabling a granular configuration experience of the native intrinsics, which was not entirely clear how to achieve before, and (ii) adding custom APIs as intrinsics.
Enhance how our configuration operates: the previous model assumed an almost static configuration setup, allowing only limited, non-straightforward customizability from the CLI in the future. This was mostly modeled per API and not as a global aspect of the runtime. Our recommended approach for users needing functionality beyond what was provided was to fork or recreate their own version of the CLI/APIs. A telling sign of this issue is that the configuration parameters were almost never used in any of the APIs. This PR paves the way for greater extensibility, which will be facilitated by threading configuration options from the CLI in a follow-up PR; this method will also allow for opting-in to non-standard functionality via a CLI flag, helping us avoid issues like the one noted here: #628.

Along the way,  I had to make several changes to make all this work; the most relevant ones  are: (i)  deprecating the `javy-apis` crate (ii) enabling all the APIs by default and (iii) reworking a bit the `Console` implementation. 

The reasoning to deprecate the `javy-apis` crate is mostly around: (i) the fact that we're revamping our extensilbity model in the near fuiture and (ii)  the complexity around cyclical dependencies:  `javy` exposes rquickjs and given that the cofiguration is tied to the runtime, `javy` needed to depend on `javy-apis` but the other way around was also true. I decided not to spend too much cycles on this mostly given point (i) above. 

There still things to follow-up on after this PR, namely 

- [ ] Update all the documentation under  `docs` 
- [ ] A PR to thread the engine configuration from the CLI. NB that the engine behaviour is not  altered in this PR.

* Fix typos

* Add a comment in the apis/lib.rs 

This makes `cargo fmt -- --check` happy

* Review comments
  • Loading branch information
saulecabrera authored May 14, 2024
1 parent 821b15f commit 30c21d0
Show file tree
Hide file tree
Showing 22 changed files with 489 additions and 414 deletions.
8 changes: 2 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 1 addition & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ docs:
test-javy:
cargo wasi test --package=javy --features json,messagepack -- --nocapture

test-apis:
cargo hack wasi test --package=javy-apis --each-feature -- --nocapture

test-core:
cargo wasi test --package=javy-core -- --nocapture

Expand All @@ -44,7 +41,7 @@ test-wpt:
npm install --prefix wpt
npm test --prefix wpt

tests: test-javy test-apis test-core test-cli test-wpt
tests: test-javy test-core test-cli test-wpt

fmt: fmt-quickjs-wasm-sys fmt-quickjs-wasm-rs fmt-javy fmt-apis fmt-core fmt-cli

Expand Down
3 changes: 3 additions & 0 deletions crates/apis/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Mark the crate as deprecated.
- Fold APIs under the [Javy crate](https://crates.io/crates/javy) to enable more
ergonomic runtime configuration.
- Rewrite the APIs on top of Javy v3.0.0, which drops support for
`quickjs-wasm-rs` in favor of `rquickjs`

Expand Down
12 changes: 3 additions & 9 deletions crates/apis/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,7 @@ homepage = "https://github.com/bytecodealliance/javy/tree/main/crates/apis"
repository = "https://github.com/bytecodealliance/javy/tree/main/crates/apis"
categories = ["wasm"]

[features]
console = []
random = ["dep:fastrand"]
stream_io = []
text_encoding = []

[dependencies]
anyhow = { workspace = true }
fastrand = { version = "2.1.0", optional = true }
javy = { workspace = true }

[badges]
maintenance = { status = "deprecated" }
37 changes: 2 additions & 35 deletions crates/apis/README.md
Original file line number Diff line number Diff line change
@@ -1,36 +1,3 @@
<div align="center">
<h1><code>Javy APIs</code></h1>
<p>
<strong>A collection of APIs for Javy</strong>
</p>
# This crate is deprecated.

<p>
<a href="https://docs.rs/javy-apis"><img src="https://docs.rs/javy-apis/badge.svg" alt="Documentation Status" /></a>
<a href="https://crates.io/crates/javy-apis"><img src="https://img.shields.io/crates/v/javy-apis.svg" alt="crates.io status" /></a>
</p>
</div>

Refer to the [crate level documentation](https://docs.rs/javy-apis) to learn more.

Example usage:

```rust
// With the `console` feature enabled.
use javy::{Runtime, from_js_error};
use javy_apis::RuntimeExt;
use anyhow::Result;

fn main() -> Result<()> {
let runtime = Runtime::new_with_defaults()?;
let context = runtime.context();
context.with(|cx| {
cx.eval_with_options(Default::default(), "console.log('hello!');")
.map_err(|e| to_js_error(cx.clone(), e))?
});
Ok(())
}
```

## Publishing to crates.io

To publish this crate to crates.io, run `./publish.sh`.
Javy APIs have been folded into the [Javy crate](https://crates.io/crates/javy)
12 changes: 0 additions & 12 deletions crates/apis/src/api_config.rs

This file was deleted.

51 changes: 0 additions & 51 deletions crates/apis/src/console/config.rs

This file was deleted.

89 changes: 2 additions & 87 deletions crates/apis/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,87 +1,2 @@
//! A collection of APIs for Javy.
//!
//! APIs are enabled through cargo features.
//!
//! Example usage:
//! ```rust
//!
//! //With the `console` feature enabled.
//! use javy::{Runtime, from_js_error};
//! use javy_apis::RuntimeExt;
//! use anyhow::Result;
//!
//! fn main() -> Result<()> {
//! let runtime = Runtime::new_with_defaults()?;
//! let context = runtime.context();
//! context.with(|cx| {
//! cx.eval_with_options(Default::default(), "console.log('hello!');")
//! .map_err(|e| to_js_error(cx.clone(), e))?
//! });
//! Ok(())
//! }
//!
//! ```
//!
//! If you want to customize the runtime or the APIs, you can use the
//! [`Runtime::new_with_apis`] method instead to provide a [`javy::Config`]
//! for the underlying [`Runtime`] or an [`APIConfig`] for the APIs.
//!
//! ## Features
//! * `console`: Adds an implementation of the `console.log` and `console.error`,
//! enabling the configuration of the standard streams.
//! * `text_encoding`: Registers implementations of `TextEncoder` and `TextDecoder`.
//! * `random`: Overrides the implementation of `Math.random` to one that seeds
//! the RNG on first call to `Math.random`. This is helpful to enable when using
//! using a tool like Wizer to snapshot a [`Runtime`] so that the output of
//! `Math.random` relies on the WASI context used at runtime and not the WASI
//! context used when Wizening. Enabling this feature will increase the size of
//! the Wasm module that includes the Javy Runtime and will introduce an
//! additional hostcall invocation when `Math.random` is invoked for the first
//! time.
//! * `stream_io`: Adds an implementation of `Javy.IO.readSync` and `Javy.IO.writeSync`.
use anyhow::Result;
use javy::Runtime;

pub use api_config::APIConfig;
#[cfg(feature = "console")]
pub use console::LogStream;
pub use runtime_ext::RuntimeExt;

mod api_config;
#[cfg(feature = "console")]
mod console;
#[cfg(feature = "random")]
mod random;
mod runtime_ext;
#[cfg(feature = "stream_io")]
mod stream_io;
#[cfg(feature = "text_encoding")]
mod text_encoding;

pub(crate) trait JSApiSet {
fn register(&self, runtime: &Runtime, config: &APIConfig) -> Result<()>;
}

/// Adds enabled JS APIs to the provided [`Runtime`].
///
/// ## Example
/// ```
/// # use anyhow::Error;
/// # use javy::Runtime;
/// # use javy_apis::APIConfig;
/// let runtime = Runtime::default();
/// javy_apis::add_to_runtime(&runtime, APIConfig::default())?;
/// # Ok::<(), Error>(())
/// ```
pub fn add_to_runtime(runtime: &Runtime, config: APIConfig) -> Result<()> {
#[cfg(feature = "console")]
console::Console::new().register(runtime, &config)?;
#[cfg(feature = "random")]
random::Random.register(runtime, &config)?;
#[cfg(feature = "stream_io")]
stream_io::StreamIO.register(runtime, &config)?;
#[cfg(feature = "text_encoding")]
text_encoding::TextEncoding.register(runtime, &config)?;
Ok(())
}
//! Javy APIs -- Deprecated
//! Javy APIs moved to the main Javy crate.
36 changes: 0 additions & 36 deletions crates/apis/src/runtime_ext.rs

This file was deleted.

1 change: 0 additions & 1 deletion crates/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ crate-type = ["cdylib"]
[dependencies]
anyhow = { workspace = true }
javy = { workspace = true, features = ["export_alloc_fns"] }
javy-apis = { path = "../apis", features = ["console", "text_encoding", "random", "stream_io"] }
once_cell = { workspace = true }

[features]
Expand Down
11 changes: 7 additions & 4 deletions crates/core/src/runtime.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use anyhow::Result;
use javy::{Config, Runtime};
use javy_apis::{APIConfig, LogStream, RuntimeExt};

pub(crate) fn new_runtime() -> Result<Runtime> {
let mut api_config = APIConfig::default();
api_config.log_stream(LogStream::StdErr);
Runtime::new_with_apis(Config::default(), api_config)
let mut config = Config::default();
let config = config
.text_encoding(true)
.redirect_stdout_to_stderr(true)
.javy_stream_io(true);

Runtime::new(std::mem::take(config))
}
2 changes: 2 additions & 0 deletions crates/javy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ rmp-serde = { version = "^1.3", optional = true }
# TODO: cargo doesn't seem to pickup the fact that quickcheck is only used for
# tests.
quickcheck = "1"
bitflags = "2.5.0"
fastrand = "2.1.0"

[features]
export_alloc_fns = []
Expand Down
Loading

0 comments on commit 30c21d0

Please sign in to comment.