-
Notifications
You must be signed in to change notification settings - Fork 9
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
RFC: add pyo3-like attribute macros to define nodejs classes and methods #43
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Joe Zhou <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for this proposal! Py03
is a great example because a lot of the problem space are shared.
name: string, | ||
age: u8, | ||
} | ||
#[neon::methods] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these should be namespaced to help prevent conflicts with other macros. This could be named neon::class_methods
or we could have everything in a neon::class
module.
#[neon::class::data]
#[neon::class::methods]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently neon only has one proc/attribute macro, right? main
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually prefer the simplicity of #[neon::methods], #[neon::classes]
if in a foreseeable future there won't be any namespace conflicts.
Ok(cx.string(format!("Hello, my name is {}", self.name))) | ||
} | ||
|
||
#[neon::constructor] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current declare_types
macro has a two-part initialization with the second part being optional.
- (Required). Constructor. This creates the Rust struct that gets wrapped into the JS class.
- (Optional). Initialization. This has access to the created class and can perform actions like assigning properties. This can be thought of as using
this
in a JS constructor.
I think this is valuable and it would be nice to maintain that functionality. I can think of a couple ways to implement it:
- Another macro attribute like
#[neon::class_init]
for that phase - Need to call a function and return a class instead of the data type.
create_class(Person { name, age })
- What other designs have been considered and what is the rationale for not choosing them? | ||
- What is the impact of not doing this? | ||
|
||
# Unresolved questions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some unresolved questions that I have.
- What is the name of the type? Do we generate complete new types or do we have something like
JsClass<Person>
? - Do we implicitly create a
RefCell
or do we only allow&self
? I think we should start simple and class methods can only take&self
. If they take a different form of&self
we fail to compile and if they don't takeself
at all we create a static method. - Are there any other critical features? In my opinion we should start simple and not try to add everything.
- What happens if the class is called as a function instead of a constructor?
Signed-off-by: Joe Zhou <[email protected]>
Updated RFC with the comments. |
Would love to see this added back. Very much missing it from the recent versions |
Any update on this RFC? |
@louneskmt No update. This RFC itself is blocked by being able to create classes (i.e., a type safe wrappers around While this feature is highly desirable, it's not considered a blocker for Neon 1.0 because the functionality can be accomplished with some JavaScript glue code. Blockers for 1.0 have taken priority. Contributions would be welcome. I have spent a bit of time thinking about what Neon's
|
Signed-off-by: Joe Zhou [email protected]