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

[Merged by Bors] - Implement JsPromise wrapper #2758

Closed
wants to merge 2 commits into from
Closed

Conversation

jedel1043
Copy link
Member

This Pull Request closes #2687.

It changes the following:

  • Adds a JsPromise wrapper to manipulate promises from Rust.
  • Cleans up the promise module to ease the integration of JsPromise with it.

cc @lastmjs

@jedel1043 jedel1043 added enhancement New feature or request API labels Mar 28, 2023
@jedel1043 jedel1043 added this to the v0.17.0 milestone Mar 28, 2023
@github-actions
Copy link

Test262 conformance changes

Test result main count PR count difference
Total 94,277 94,277 0
Passed 71,094 71,094 0
Ignored 17,324 17,324 0
Failed 5,859 5,859 0
Panics 0 0 0
Conformance 75.41% 75.41% 0.00%

@nekevss
Copy link
Member

nekevss commented Mar 29, 2023

This is great! On other built-in wrappers, I believe an example was typically added to boa_example to provide a basic reference outside of the docs. Any thought on doing adding that to this PR? Given the size, we could always file it as an issue, since implementing an example of JsPromise might be a great first issue for someone.

@jedel1043
Copy link
Member Author

Given the size, we could always file it as an issue, since implementing an example of JsPromise might be a great first issue for someone.

Sounds good! I think it's better to have some low hanging fruit to introduce new contributors to the repo, and adding an usage example sounds like the perfect issue for that. I'll open an issue for that after we merge this.

Copy link
Member

@nekevss nekevss left a comment

Choose a reason for hiding this comment

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

Looks good to me! 😄 Nice work!

Copy link
Member

@HalidOdat HalidOdat left a comment

Choose a reason for hiding this comment

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

Nice! Looks good to me!

@jedel1043
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Mar 29, 2023
This Pull Request closes #2687.

It changes the following:

- Adds a `JsPromise` wrapper to manipulate promises from Rust.
- Cleans up the `promise` module to ease the integration of `JsPromise` with it.

cc @lastmjs
@bors
Copy link

bors bot commented Mar 29, 2023

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Implement JsPromise wrapper [Merged by Bors] - Implement JsPromise wrapper Mar 29, 2023
@bors bors bot closed this Mar 29, 2023
@bors bors bot deleted the promises-api branch March 29, 2023 18:40
@lastmjs
Copy link
Contributor

lastmjs commented Mar 31, 2023

I'll just share some of the janky stuff I had to do, and please let me know if these will be fixed with this PR:

To move the promise queue forward (not sure if that's the right way to describe it, but this was the suggestion and it worked), I had to just eval nothing:

// This runs all pending promises to completion
// TODO use the better Boa API once it's available
_azle_boa_context.eval("");

Making a promise in Rust:

// TODO make this promise in a better way once Boa allows it or you can figure it out
let promise_js_value = _context.eval("new Promise(() => {})").unwrap();

Fulfilling a promise in Rust:

let promise_js_object = promise_js_value.as_object().unwrap();
let mut promise_object = promise_js_object.borrow_mut();
let mut promise = promise_object.as_promise_mut().unwrap();

promise.fulfill_promise(&call_result_js_value, &mut *_azle_boa_context);

I also had to make some things public in my fork of Boa:

pub(crate) use if_abrupt_reject_promise;

/// PromiseState
#[derive(Debug, Clone, Trace, Finalize)]
pub enum PromiseState {
    /// Pending
    Pending,
    /// Fulfilled
    Fulfilled(JsValue),
    /// Rejected
    Rejected(JsValue),
}

/// The internal representation of a `Promise` object.
#[derive(Debug, Clone, Trace, Finalize)]
pub struct Promise {
    /// promise_state
    pub promise_state: PromiseState,
    promise_fulfill_reactions: Vec<ReactionRecord>,
    promise_reject_reactions: Vec<ReactionRecord>,
    promise_is_handled: bool,
	@@ -1439,7 +1444,7 @@ impl Promise {
    /// # Panics
    ///
    /// Panics if `Promise` is not pending.
    pub fn fulfill_promise(&mut self, value: &JsValue, context: &mut Context) {
        // 1. Assert: The value of promise.[[PromiseState]] is pending.
        assert!(
            matches!(self.promise_state, PromiseState::Pending),

@jedel1043
Copy link
Member Author

jedel1043 commented Mar 31, 2023

Yep! Some usage examples:

To move the microtask queue forward:

context.run_jobs();

To create a new, pending promise:

let (promise, resolvers) = JsPromise::new_pending(context);

To resolve a promise:

resolvers.resolve.call(this, &[value], context)?;

// or

resolvers.reject.call(this, &[error], context)?;

To inspect the state of a promise:

match promise.state() {
    PromiseState::Pending => {},
    PromiseState::Fulfilled(v) => {},
    PromiseState::Rejected(e) => {},
}

The JsPromise docs has more detailed examples :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Safe wrapper for JsPromise
4 participants