Skip to content

feat: Improve JSClass and add JSClassBuilder #23

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

Conversation

Hywan
Copy link
Contributor

@Hywan Hywan commented Oct 11, 2023

This PR should be reviewed commit-by-commit.

Basically:

  • it adds a new procedural macro: #[constructor_callback]
  • it adds a new type JSClassBuilder
  • it adds JSClass::builder and JSClass::new_object.

So far, JSClassBuilder only allows to set the class name and the class constructor, but this pattern is easily extendable to cover all features provided by sys::JSClassDefinition.

@Hywan Hywan changed the title feat: Implement JSClass::new and JSClass::instantiate feat: Improve JSClass and add JSClassBuilder Oct 12, 2023
@Hywan Hywan marked this pull request as ready for review October 12, 2023 09:02
@Hywan
Copy link
Contributor Author

Hywan commented Oct 12, 2023

CI has been broken by apt-get. Please, can you re-run the jobs?

@waywardmonkeys
Copy link
Contributor

CI has been broken by apt-get. Please, can you re-run the jobs?

It was a bug in the CI script. I've pushed to main with a fix. You can rebase this atop current main and it should be good to go.

Hywan added 3 commits October 12, 2023 14:01
This patch is twofold. First off, it implements `JSClass::new`,
along with the `constructor_callback` procedural macro for a better
developer experience. Second, it implements `JSClass::instantiate`
to instantiate the object (note that it doesn't call the constructor,
`JSObject::call_as_constructor` must be used).
This patch improves the tests about `new_function` can checking a
function can run from within a script too.
@Hywan Hywan force-pushed the feat-value-new-construction-callback branch from 1608188 to 4ac4bb8 Compare October 12, 2023 12:02
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.

I had a number of questions while reading these changes and then went back over them and decided against them ... I'm inclined to merge this now and maybe then lay out what some of the next steps might be to doing a release with it.

I'd like to see some of the APIs get more ergonomic and less boilerplate ...

@waywardmonkeys
Copy link
Contributor

I guess if you have the time, rebasing the clippy fix back into the originating commit (ea94cab right now) would be nice, so that the whole history is cleanly building.

Hywan added 2 commits October 12, 2023 16:24
This patch revamps `JSClass` by introducing `JSClassBuilder`. A
`JSClass` is kind of rich, and providing a clean API is difficult, hence
the `JSClassBuilder` which brings a nice API for building any kind of
`JSClass`. For now, only the classname and the constructor callback can
be set, but this pattern is easily extendable.

This patch also adds documentation and improves the tests a little bit.
This patch implements `Display` and `Error` for `JSException`.
@Hywan Hywan force-pushed the feat-value-new-construction-callback branch from 5ead2c1 to 9db000e Compare October 12, 2023 14:24
@Hywan
Copy link
Contributor Author

Hywan commented Oct 12, 2023

Don't want to push you. If you want to improve this, please share your ideas. It can be done as a next PR though.

@waywardmonkeys waywardmonkeys merged commit 2e79f46 into endoli:main Oct 12, 2023
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