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

Add fixed size array rendering #3174

Merged
merged 2 commits into from
Nov 4, 2024
Merged

Add fixed size array rendering #3174

merged 2 commits into from
Nov 4, 2024

Conversation

LouisDISPA
Copy link
Contributor

Hello, I was using fixed size array and the render method is not implemented on them.
I find it weird that it's implemented on Vec but not arrays. So I wrote a quick implementation.
Is there a reason it's not implemented ?

This allow to pass the array directly to the view method without converting it into a vec.

// before
view! { <div> {array.to_vec()} </div> }

// after
view! { <div> {array} </div> }

It's mostly a copy of the Vec implementation with minor adjustments :

  • I removed the marker since it's used to insert new items on size change
pub struct VecState<T>
where
    T: Mountable,
{
    states: Vec<T>,
    // Vecs keep a placeholder because they have the potential to add additional items,
    // after their own items but before the next neighbor. It is much easier to add an
    // item before a known placeholder than to add it after the last known item, so we
    // just leave a placeholder here unlike zero-or-one iterators (Option, Result, etc.)
    marker: crate::renderer::types::Placeholder,
}
  • I used the <[]>::map function most of the time.
    The only exception is for the future::join_all that takes and return an iterator.

  • For the Render::rebuild method I don't know if there is a smart way of handling it. So I just wrote a naive implementation.

}

fn html_len(&self) -> usize {
self.iter().map(RenderHtml::html_len).sum::<usize>() + 2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I removed 1 from the constant compared to the Vec implementation. I did this guessing that the placeholder was counted in. But I have no idea what the constant is for.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The 3 was the length of the comment node <!> :-) So this can just be 0, but it's also just an estimate/heuristic

for child in self.into_iter() {
child.to_html_with_buf(buf, position, escape, mark_branches);
}
buf.push_str("<!>");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Inserting the placeholder here will cause an issue, as it's not used during hydration (so children after this will hydrate incorrectly because there's an extra node

mark_branches,
);
}
buf.push_sync("<!>");
Copy link
Collaborator

Choose a reason for hiding this comment

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

(See above)

@gbj
Copy link
Collaborator

gbj commented Nov 4, 2024

Thanks so much! This looks great over all, I added a couple comments about the placeholders for information, but I just added a quick commit to fix it, so I don't think there's any action needed on your part.

@gbj gbj merged commit d9f52da into leptos-rs:main Nov 4, 2024
54 of 73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants