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

Return the add_listener method for VTag #2298

Closed
XX opened this issue Dec 24, 2021 · 6 comments · Fixed by #2300
Closed

Return the add_listener method for VTag #2298

XX opened this issue Dec 24, 2021 · 6 comments · Fixed by #2300
Labels

Comments

@XX
Copy link
Contributor

XX commented Dec 24, 2021

The add_listener function is needed for my project https://github.com/noogen-projects/yew-mdc-widgets, but it was removed in the #1542 PR: https://github.com/yewstack/yew/pull/1542/files?authenticity_token=Tuuykd78jnZ%2BAdLNBHI59D2wWoJnpGuCUvZPLJvjNMkmLvrDf%2FM0Ap0XHwA0TZAGVPz2sH8w8OU6s1696gisMA%3D%3D&file-filters%5B%5D=.rs#diff-ca7fccc825bdf79bdab83f034636dee4d6727ba13ee5578dc8eb784b1d136478L503

But why? Can I return this method or is it impossible?

@voidpumpkin
Copy link
Member

@bakape Do you remember why it got removed?

@bakape
Copy link
Contributor

bakape commented Dec 26, 2021

@voidpumpkin The listener vector was converted into a boxed array to reduce construction overhead. This is a microptimasation that should not break almost all use cases. I also have a PR in the works, that enables allocating said arrays on the stack (among other things), so I don't see this changing. If dynamic size listener arrays are needed, best build vectors, convert them to boxed arrays and then pass that to the VTag.

@voidpumpkin
Copy link
Member

@bakape I think your explanation ended up being a little bit too complex for me. By boxed array, I presume, you mean its a slice now (i see the same in the code). And by that it means you have to set all setters at once as its size is not dynamic.

@voidpumpkin
Copy link
Member

voidpumpkin commented Dec 26, 2021

@XX Could you explain more about what add_listener does and why it is needed? Its hard to grasp what it its and why its needed just from "I use it in X project, please add it back".
I cant imagine personally anyone adding listeners not via DOM api or html! macro, so i would love to understand the core of the issue.

@voidpumpkin
Copy link
Member

voidpumpkin commented Dec 26, 2021

@bakape Also I see the creator of the issue implemented exactly what you suggested, could you give #2300 a review?

@bakape
Copy link
Contributor

bakape commented Dec 26, 2021

By boxed array, I presume, you mean its a slice now

A boxed array is an owning reference to a heap-allocated static size array. That reference is a Box and implements dereferencing (via std::ops::Deref) to a slice.

I cant imagine personally anyone adding listeners not via DOM api or html! macro, so i would love to understand the core of the issue.

You would need it for writing libraries (or other abstractions), that work on individual VDOM types - lower level API than the html! macro.

Also I see the creator of the issue implemented exactly what you suggested, could you give #2300 a review?

Sure, I'll take a look.

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

Successfully merging a pull request may close this issue.

3 participants