Skip to content

Exotic methods support#567

Merged
richarddd merged 19 commits intomasterfrom
ef-exotic-class
Apr 16, 2026
Merged

Exotic methods support#567
richarddd merged 19 commits intomasterfrom
ef-exotic-class

Conversation

@Sytten
Copy link
Copy Markdown
Collaborator

@Sytten Sytten commented Dec 4, 2025

  • Add support for exotic methods
  • Add a macro to more easily implement those
    • it's a bit ugly but it works, I will improve it over time
    • Unsure about the get,set,has,delete names
  • Remove send bound for async_with when not in parallel mode

Finishes #497

@Sytten Sytten requested a review from richarddd December 4, 2025 01:49
@Sytten Sytten linked an issue Dec 6, 2025 that may be closed by this pull request
Why does async_with need the future to implement Send? #495
Open
richarddd
richarddd approved these changes Dec 8, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@richarddd richarddd left a comment

Choose a reason for hiding this comment

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

Minor suggestions

Comment thread macro/src/exotic.rs

let crate_name = format_ident!("{}", crate_ident()?);
let class_name = get_class_name(&self_ty);
let module_name = format_ident!("__impl_exotic_{}__", class_name);
Copy link
Copy Markdown
Collaborator

@richarddd richarddd Dec 8, 2025

Choose a reason for hiding this comment

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

Should we consolidate this with the one in class to avoid duplication?

Copy link
Copy Markdown
Collaborator Author

@Sytten Sytten Dec 8, 2025

Choose a reason for hiding this comment

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

I will rework this I am not satisfied. I think I can use the same trick we are using for the constructor with the double ref specialization.

Comment thread core/src/result.rs Outdated
DelSkayn
DelSkayn reviewed Jan 18, 2026
View reviewed changes
Copy link
Copy Markdown
Owner

@DelSkayn DelSkayn left a comment

Choose a reason for hiding this comment

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

Two suggestions.

Comment thread core/src/class/ffi.rs Outdated
Comment thread core/src/class.rs
@irvanservia irvanservia mentioned this pull request Jan 19, 2026
Open
@xaptronic
Copy link
Copy Markdown

xaptronic commented Apr 13, 2026

hey, came across this while optimizing Rust/JS interop patterns. will this PR include support for get_own_property and get_own_property_names? I noticed they're TODO in ExoticMethodsHolder — those are the main callbacks we'd need. happy to help with the remaining work if useful.

@Sytten
Copy link
Copy Markdown
Collaborator Author

Sytten commented Apr 14, 2026

Honestly I dont have time to work on that right now, so @richarddd if you want to take it over or have someone else take it off my plate it works for me.

@richarddd
Address PR review comments
34db552 
- Remove unused for<'a> lifetime bounds on exotic function type aliases
- Replace CALLABLE/EXOTIC bool consts with ClassKind enum
- Deduplicate handle_panic and handle_panic_exotic into shared inner method
- Remove unused ToString import
@xaptronic
Copy link
Copy Markdown

xaptronic commented Apr 14, 2026

NP - are these get_own_property and get_own_property_names sensible / possible to do as part of this? I noticed they also don't exist on the proxy objects - would this be more sensible to do in a follow up on both?

@richarddd
Copy link
Copy Markdown
Collaborator

richarddd commented Apr 15, 2026

NP - are these get_own_property and get_own_property_names sensible / possible to do as part of this? I noticed they also don't exist on the proxy objects - would this be more sensible to do in a follow up on both?

@xaptronic seems in scope. will add!

@Sytten
Copy link
Copy Markdown
Collaborator Author

Sytten commented Apr 15, 2026

The reason I didnt do it previously is that the vtable is starting to get pretty big. I am not as fan that all classes use the same quickjs class id since it makes not nice to extend a rust class in js.

@richarddd richarddd merged commit aa311a3 into master Apr 16, 2026
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@richarddd richarddd richarddd approved these changes

@DelSkayn DelSkayn DelSkayn left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Why does async_with need the future to implement Send?

4 participants

@Sytten @xaptronic @richarddd @DelSkayn