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

feat: Implement JSValue::new_function #20

Merged
merged 5 commits into from
Oct 9, 2023

Conversation

Hywan
Copy link
Contributor

@Hywan Hywan commented Oct 9, 2023

This PR is twofold: first it creates JSValue::new_function, second it creates a procedural macro named function_callback to create function callbacks more easily.

Check out this test:

#[test]
fn function_with_macros() -> Result<(), JSException> {
    use crate as javascriptcore;
    use crate::function_callback;

    let ctx = JSContext::default();

    #[function_callback]
    fn sum(
        ctx: &JSContext,
        _function: Option<&JSObject>,
        _this_object: Option<&JSObject>,
        arguments: &[JSValue],
    ) -> Result<JSValue, JSException> {
        if arguments.len() != 2 {
            return Err(JSValue::new_string(ctx, "must receive 2 arguments").into());
        }

        let x = arguments[0].as_number()?;
        let y = arguments[1].as_number()?;

        Ok(JSValue::new_number(ctx, x + y))
    }

    let sum = JSValue::new_function(&ctx, "awesome_sum", Some(sum)).as_object()?;

    // Correct call.
    {
        let result = sum.call_as_function(
            None,
            &[JSValue::new_number(&ctx, 1.), JSValue::new_number(&ctx, 2.)],
        )?;

        assert_eq!(result.as_number()?, 3.);
    }

    // Invalid call: Not enough arguments.
    {
        let result = sum.call_as_function(None, &[]);

        assert!(result.is_err());
    }

    Ok(())
}

Hywan added 4 commits October 9, 2023 13:55
This patch adds a new `JSContext::new_inner` method as a quick
constructor for `JSContext`.
This patch adds `JSObject::new_inner` to build a `JSObject` from its raw value.
Copy link
Contributor

@waywardmonkeys waywardmonkeys left a comment

Choose a reason for hiding this comment

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

Should the new_inner to from_raw be a separate commit?

If it is hard, then let's not worry about it, I guess.

The clippy fix commit should get smashed back into the commit that started the problem so that the commit history remains fairly clean.

I'm out at the moment and want to look at the actual changes a bit more in about 2 hours when I'm back home.

@waywardmonkeys
Copy link
Contributor

(But I should say that I'm excited about this!)

@Hywan
Copy link
Contributor Author

Hywan commented Oct 9, 2023

The change from new_inner to from_raw is part of the same commit and honestly… if it's OK for you, I would prefer to avoid having a new rebase :-).

I've changed to pub unsafe fn from_raw because I need them in the proc-macro.

This patch implements `JSValue::new_function`.

In addition, it creates a new `javascriptcore_macros` crate, to expose
a `function_callback` procedural macro. It's helpful to easily create a
function callback.
Copy link
Contributor

@waywardmonkeys waywardmonkeys left a comment

Choose a reason for hiding this comment

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

Going to go with this. Have some things that we should discuss, but for subsequent commits.

@waywardmonkeys waywardmonkeys merged commit 3126711 into endoli:main Oct 9, 2023
@waywardmonkeys
Copy link
Contributor

Thanks a ton for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants