feat: add inheritance support for Rust classes#587
feat: add inheritance support for Rust classes#587ajihyf wants to merge 3 commits intoDelSkayn:masterfrom
Conversation
ae55ef9 to
2705011
Compare
|
Not a bad addition though the primary usecase is to extend rust from a js class to avoid the shenanigans we currently have to do to create classes like Buffer. Any solution we merge will need to support that. Also from a larger perspective I dont like the vtable design we currently have with a unique class ID for all rust classes. I would much rather we find a way to use the engine directly, we switched to this system as a patch to avoid clashes of class ID but nobody is really happy with the solution. |
|
Thanks for the feedback. To give some context on my motivation: I am currently working on a DOM-like library, so implementing Rust-to-Rust inheritance (e.g., Node -> Element -> HTMLElement) was my immediate priority. As for the "extending JS classes" requirement, I have a proposal: we could store the Rust struct (as a hidden object) inside the native JS object using a Symbol. This would incur a small extra lookup cost but would solve the binding issue without requiring a massive refactor right now. |
I don't think it has to be this complicated. We just have to call this function: With newTarget being the class we want to extend. I added some comments here: |
As I understand it, passing For QuickJS internal/built-in classes: The opaque field is already occupied by the engine (e.g., |
|
Sorry for the huge delay and thanks again for this PR @ajihyf ! Code looks fine, just a final think i want to test is that how does polymorphism work across rust and JS? For instance extending a class in Rust then extending from JS? Similarly If a class is extended in JS and we want to extend it from Rust. |
Closes DelSkayn#116 (partly), since extending JS classes is excluded from this MR as it requires significant refactoring of class methods.
|
No worries about the delay! @richarddd I've just added a test case for JS extending a Rust class, which works straightforwardly. However, extending a JS class from Rust is still tricky, as it requires storing the Rust pointer (state) inside the JS object. Also, I no longer have an immediate need for this inheritance feature in my project. Please feel free to decide on the status of this PR based on your roadmap. :) |
I’m very interested in this feature and would like to see it supported. My only concern is whether the current approach might turn into a footgun over time. It may be worth considering changes in quickjs-ng that allow for a cleaner design, preferably one that avoids storing Rust state on JavaScript objects. From your perspective @ajihyf, what changes or additions to |
Closes #116 (partly), extending JS classes is excluded from this MR as it requires significant refactoring of class methods.
Description of changes
class.rsto exposeparent_vtablemethod for inheritance.inherits.rsto defineHasParenttrait.ffi.rsto include parent vtable in theVTablestruct.opaque.rsto handle constructors for inherited classes.extendsattribute.Checklist