search

denoland / rusty_v8

Rust bindings for the V8 JavaScript engine
https://crates.io/crates/v8
MIT License
3.27k stars 307 forks source link

api improvement #1470

Closed dev-ardi closed 5 months ago

dev-ardi commented 5 months ago

This addresses https://github.com/denoland/rusty_v8/issues/1467

This new API design has the following advantages

  1. It makes it sound to initialize isolates
  2. It uses builder pattern to create a platform. This is better because it doesn't require users to explicitly think about uncommon options, and better documents existing options in case that users might need them.
  3. It makes it impossible at compile time to create an isolate without having initialized the platform
  4. It's uncommon to create an isolate with options so make the default way of creating them not take any options.
  5. This leads to nicer code and it's more rusty: 
    let isolate = v8::V8Options::new()
.build() // This hints that it's builder pattern and that there are more options that could be used, but doesn't forces them to know them.
.isolate();
CLAassistant commented 5 months ago

CLA assistant check
All committers have signed the CLA.

mmastrac commented 5 months ago

I think it's better if we address https://github.com/denoland/rusty_v8/issues/1467 separately from the builder pattern for init. While I don't think it's a bad thing to readdress isolate initialization, that's a totally different topic.

dev-ardi commented 5 months ago

The idea is there. Feel free to do with it what you will.