dioxus icon indicating copy to clipboard operation
dioxus copied to clipboard

Create `Visit` trait

Open matthunz opened this issue 1 year ago • 2 comments

Implements a zero-cost visitor for VNodes via a trait pattern instead of an iterator.

Alternative to https://github.com/DioxusLabs/dioxus/pull/2722 to solve https://github.com/DioxusLabs/dioxus/issues/1177.

Exports the Visit trait:

pub trait Visit {
    /// Visit a [`VNode`].
    fn visit_vnode(&mut self, vdom: &VirtualDom, vnode: &VNode) { ... }

    /// Visit a template node.
    fn visit_node(&mut self, vdom: &VirtualDom, vnode: &VNode, node: TemplateNode) { ... }

    /// Visit an element.
    fn visit_element(
        &mut self,
        vdom: &VirtualDom,
        vnode: &VNode,
        tag: &'static str,
        namespace: Option<&'static str>,
        attrs: &'static [TemplateAttribute],
        children: &'static [TemplateNode],
    ) { ... }

    /// Visit an element attribute.
    fn visit_attr(&mut self, vdom: &VirtualDom, vnode: &VNode, attr: &TemplateAttribute) { ... }

    /// Visit a dynamic node.
    fn visit_dynamic(
        &mut self,
        vdom: &VirtualDom,
        vnode: &VNode,
        index: usize,
        node: &DynamicNode,
    ) { ... }

    /// Visit a placeholder.
    fn visit_placeholder(&mut self, vdom: &VirtualDom, vnode: &VNode, index: usize) { ... }

    /// Visit a fragment.
    fn visit_fragment(&mut self, vdom: &VirtualDom, vnode: &VNode, roots: &[VNode]) { ... }

    /// Visit a text node.
    fn visit_text(&mut self, vdom: &VirtualDom, vnode: &VNode, text: &str) { ... }
}

And default methods under the visit module:

/// Default method to visit a [`VNode`].
pub fn visit_vnode<V: Visit + ?Sized>(visitor: &mut V, vdom: &VirtualDom, vnode: &VNode) { ... }

/// Default method to visit a template node.
pub fn visit_node<V: Visit + ?Sized>(
    visitor: &mut V,
    vdom: &VirtualDom,
    vnode: &VNode,
    node: TemplateNode,
) { ... }

/// Default method to visit an element.
pub fn visit_element<V: Visit + ?Sized>(
    visitor: &mut V,
    vdom: &VirtualDom,
    vnode: &VNode,
    tag: &'static str,
    namespace: Option<&'static str>,
    attrs: &'static [TemplateAttribute],
    children: &'static [TemplateNode],
) { ... }

/// Default method to visit a dynamic node.
pub fn visit_dynamic<V: Visit + ?Sized>(
    visitor: &mut V,
    vdom: &VirtualDom,
    vnode: &VNode,
    index: usize,
    node: &DynamicNode,
) { ... }

/// Default method to visit a placeholder.
pub fn visit_placeholder<V: Visit + ?Sized>(
    visitor: &mut V,
    vdom: &VirtualDom,
    vnode: &VNode,
    index: usize,
) { ... }

/// Default method to visit a fragment.
pub fn visit_fragment<V: Visit + ?Sized>(
    visitor: &mut V,
    vdom: &VirtualDom,
    vnode: &VNode,
    roots: &[VNode],
) { ... }

matthunz avatar Sep 09 '24 17:09 matthunz

I see two main use cases for #1177:

  1. Renderers like dioxus-tui need to traverse the virtual dom and don't care about caching too much
  2. Components like head::Title need to look through their children and find a specific node. This could also allow more lints like warning if an outlet is not found in the children of a router

Right now any general version of vnode traversal only really useful for 1 because it requires direct access to the VirtualDom. I don't think we can create a unified solution to both until we move component state to the runtime

The visitor approach lets you avoid matching over the node type which is nice, but the API is a bit odd if you don't know about templates which seems common for 2. You still need to deal with static and dynamic attribute values separately and there are indexes floating around the methods to link Template and VNode state together

I think the visitor API works better for 1. It fits an immediate mode renderer very nicely. Still a bit rough around the edges because of templates, but a lot better than how you would implement this today. Renders like dioxus-ssr also seem like a natural fit, but I don't think we would use this in practice because we need to cache the template and avoid traversing it every time

What do you think about combining the two APIs? The visitor could visit the more abstract version of nodes that the cursor API provides. Then for use case 2 you can just use the cursor directly

ealmloff avatar Sep 13 '24 01:09 ealmloff

Right now any general version of vnode traversal only really useful for 1 because it requires direct access to the VirtualDom. I don't think we can create a unified solution to both until we move component state to the runtime

Makes sense 👍 If we can move component state I feel like this could easily be used without the VirtualDom

The visitor approach lets you avoid matching over the node type which is nice, but the API is a bit odd if you don't know about templates which seems common for 2.

Totally agree, I am wondering if we could ease that with docs like the syn crate https://docs.rs/syn/latest/syn/visit/index.html

You still need to deal with static and dynamic attribute values separately and there are indexes floating around the methods to link Template and VNode state together

You make an interesting point here, I feel like we could change the Visit trait to visit both the same. Maybe visit_dynamic can delegate to visit_attribute or visit_element?

What do you think about combining the two APIs? The visitor could visit the more abstract version of nodes that the cursor API provides. Then for use case 2 you can just use the cursor directly

That makes a lot of sense to me 👍 I personally think it would be nice to have an iterator for more dynamic situations (and easier children.map(...) etc) and a visitor for static situations (like renderers, debugging, and maybe lints now that you mention it)

matthunz avatar Sep 13 '24 03:09 matthunz

Closing this since I'm not very committed to the structure of the rsx!{} call output (vnodes/nodes/etc) and am hesitant to support any iterators/traits until I feel like the rest is stable.

Currently it's possible to traverse the tree manually - as we do in a variety of places - but not very ergonomic.

jkelleyrtp avatar Oct 26 '24 00:10 jkelleyrtp