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.

Sign up for GitHub

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

Jump to bottom

Add the ability to use non-literal string as attribute names #2593

Merged
merged 5 commits into from Apr 12, 2022
Merged

Add the ability to use non-literal string as attribute names #2593

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9f98b7f
Use AttrValue instead of &'static str in Attributes::IndexMap
hamza1311 Apr 8, 2022
6a24780
avoid monomorphization to reduce binary size
hamza1311 Apr 11, 2022
6b1a1fc
fmt & `#[inline(always)]`s
hamza1311 Apr 11, 2022
d621406
more attempts to reduce the code size
hamza1311 Apr 12, 2022
6139274
make clippy happy
hamza1311 Apr 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 15 additions & 18 deletions packages/yew/src/dom_bundle/btag/attributes.rs
View file
Open in desktop
Expand Up @@ -7,6 +7,7 @@ use std::collections::HashMap;
use std::iter;
use std::ops::Deref;
use web_sys::{Element, HtmlInputElement as InputElement, HtmlTextAreaElement as TextAreaElement};
use yew::AttrValue;

impl<T: AccessValue> Apply for Value<T> {
type Element = T;
Expand Down Expand Up @@ -83,24 +84,20 @@ impl Apply for InputFields {

impl Attributes {
#[cold]
fn apply_diff_index_maps<'a, A, B>(
fn apply_diff_index_maps<'a>(
el: &Element,
// this makes it possible to diff `&'a IndexMap<_, A>` and `IndexMap<_, &'a A>`.
mut new_iter: impl Iterator<Item = (&'static str, &'a str)>,
new: &IndexMap<&'static str, A>,
old: &IndexMap<&'static str, B>,
) where
A: AsRef<str>,
B: AsRef<str>,
{
new: &IndexMap<AttrValue, AttrValue>,
old: &IndexMap<AttrValue, AttrValue>,
) {
let mut old_iter = old.iter();
let mut new_iter = new.iter();
loop {
match (new_iter.next(), old_iter.next()) {
(Some((new_key, new_value)), Some((old_key, old_value))) => {
if new_key != *old_key {
if new_key != old_key {
break;
}
if new_value != old_value.as_ref() {
if new_value != old_value {
Self::set_attribute(el, new_key, new_value);
}
}
Expand All @@ -109,7 +106,7 @@ impl Attributes {
for (key, value) in iter::once(attr).chain(new_iter) {
match old.get(key) {
Some(old_value) => {
if value != old_value.as_ref() {
if value != old_value {
Self::set_attribute(el, key, value);
}
}
Expand All @@ -123,6 +120,7 @@ impl Attributes {
// removed attributes
(None, Some(attr)) => {
for (key, _) in iter::once(attr).chain(old_iter) {
let key = key;
if !new.contains_key(key) {
Self::remove_attribute(el, key);
}
Expand All @@ -138,7 +136,7 @@ impl Attributes {
/// Works with any [Attributes] variants.
#[cold]
fn apply_diff_as_maps<'a>(el: &Element, new: &'a Self, old: &'a Self) {
fn collect<'a>(src: &'a Attributes) -> HashMap<&'static str, &'a str> {
fn collect(src: &Attributes) -> HashMap<&str, &str> {
use Attributes::*;

match src {
Expand All @@ -148,7 +146,7 @@ impl Attributes {
.zip(values.iter())
.filter_map(|(k, v)| v.as_ref().map(|v| (*k, v.as_ref())))
.collect(),
IndexMap(m) => m.iter().map(|(k, v)| (*k, v.as_ref())).collect(),
IndexMap(m) => m.iter().map(|(k, v)| (k.as_ref(), v.as_ref())).collect(),
}
}

Expand Down Expand Up @@ -233,8 +231,8 @@ impl Apply for Attributes {
},
) if ptr_eq(new_k, old_k) => {
// Double zipping does not optimize well, so use asserts and unsafe instead
assert!(new_k.len() == new_v.len());
assert!(new_k.len() == old_v.len());
assert_eq!(new_k.len(), new_v.len());
assert_eq!(new_k.len(), old_v.len());
for i in 0..new_k.len() {
macro_rules! key {
() => {
Expand Down Expand Up @@ -263,8 +261,7 @@ impl Apply for Attributes {
}
// For VTag's constructed outside the html! macro
(Self::IndexMap(new), Self::IndexMap(ref old)) => {
let new_iter = new.iter().map(|(k, v)| (*k, v.as_ref()));
Self::apply_diff_index_maps(el, new_iter, new, old);
Self::apply_diff_index_maps(el, &*new, &*old);
}
// Cold path. Happens only with conditional swapping and reordering of `VTag`s with the
// same tag and no keys.
Expand Down
45 changes: 38 additions & 7 deletions packages/yew/src/virtual_dom/mod.rs
View file
Open in desktop
Expand Up @@ -39,8 +39,9 @@ pub use self::vtag::VTag;
pub use self::vtext::VText;

use indexmap::IndexMap;
use std::borrow::Cow;
use std::borrow::{Borrow, Cow};
use std::fmt::Formatter;
use std::hash::{Hash, Hasher};
use std::ops::Deref;
use std::rc::Rc;
use std::{fmt, hint::unreachable_unchecked};
Expand All @@ -57,6 +58,7 @@ pub enum AttrValue {
impl Deref for AttrValue {
type Target = str;

#[inline(always)]
Copy link
Member

Choose a reason for hiding this comment

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

I don't think theses inline annotations actually effect anything, might as well remove them again.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should just keep them as they hint the compiler. The stdlib also has them

fn deref(&self) -> &Self::Target {
match self {
AttrValue::Static(s) => *s,
Expand All @@ -65,25 +67,36 @@ impl Deref for AttrValue {
}
}

impl Hash for AttrValue {
#[inline(always)]
fn hash<H: Hasher>(&self, state: &mut H) {
self.as_ref().hash(state);
}
}

impl From<&'static str> for AttrValue {
#[inline(always)]
fn from(s: &'static str) -> Self {
AttrValue::Static(s)
}
}

impl From<String> for AttrValue {
#[inline(always)]
fn from(s: String) -> Self {
AttrValue::Rc(Rc::from(s))
}
}

impl From<Rc<str>> for AttrValue {
#[inline(always)]
fn from(s: Rc<str>) -> Self {
AttrValue::Rc(s)
}
}

impl From<Cow<'static, str>> for AttrValue {
#[inline(always)]
fn from(s: Cow<'static, str>) -> Self {
match s {
Cow::Borrowed(s) => s.into(),
Expand All @@ -102,11 +115,18 @@ impl Clone for AttrValue {
}

impl AsRef<str> for AttrValue {
#[inline(always)]
fn as_ref(&self) -> &str {
&*self
}
}

impl Borrow<str> for AttrValue {
fn borrow(&self) -> &str {
&*self
}
}

impl fmt::Display for AttrValue {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
Expand All @@ -117,6 +137,7 @@ impl fmt::Display for AttrValue {
}

impl PartialEq for AttrValue {
#[inline(always)]
fn eq(&self, other: &Self) -> bool {
self.as_ref() == other.as_ref()
}
Expand Down Expand Up @@ -292,7 +313,7 @@ pub enum Attributes {

/// IndexMap is used to provide runtime attribute deduplication in cases where the html! macro
/// was not used to guarantee it.
IndexMap(IndexMap<&'static str, AttrValue>),
IndexMap(IndexMap<AttrValue, AttrValue>),
}

impl Attributes {
Expand All @@ -303,21 +324,21 @@ impl Attributes {

/// Return iterator over attribute key-value pairs.
/// This function is suboptimal and does not inline well. Avoid on hot paths.
pub fn iter<'a>(&'a self) -> Box<dyn Iterator<Item = (&'static str, &'a str)> + 'a> {
pub fn iter<'a>(&'a self) -> Box<dyn Iterator<Item = (&'a str, &'a str)> + 'a> {
match self {
Self::Static(arr) => Box::new(arr.iter().map(|kv| (kv[0], kv[1] as &'a str))),
Self::Dynamic { keys, values } => Box::new(
keys.iter()
.zip(values.iter())
.filter_map(|(k, v)| v.as_ref().map(|v| (*k, v.as_ref()))),
),
Self::IndexMap(m) => Box::new(m.iter().map(|(k, v)| (*k, v.as_ref()))),
Self::IndexMap(m) => Box::new(m.iter().map(|(k, v)| (k.as_ref(), v.as_ref()))),
}
}

/// Get a mutable reference to the underlying `IndexMap`.
/// If the attributes are stored in the `Vec` variant, it will be converted.
pub fn get_mut_index_map(&mut self) -> &mut IndexMap<&'static str, AttrValue> {
pub fn get_mut_index_map(&mut self) -> &mut IndexMap<AttrValue, AttrValue> {
macro_rules! unpack {
() => {
match self {
Expand All @@ -331,15 +352,15 @@ impl Attributes {
match self {
Self::IndexMap(m) => m,
Self::Static(arr) => {
*self = Self::IndexMap(arr.iter().map(|kv| (kv[0], kv[1].into())).collect());
*self = Self::IndexMap(arr.iter().map(|kv| (kv[0].into(), kv[1].into())).collect());
unpack!()
}
Self::Dynamic { keys, values } => {
*self = Self::IndexMap(
std::mem::take(values)
.iter_mut()
.zip(keys.iter())
.filter_map(|(v, k)| v.take().map(|v| (*k, v)))
.filter_map(|(v, k)| v.take().map(|v| (AttrValue::from(*k), v)))
.collect(),
);
unpack!()
Expand All @@ -348,8 +369,18 @@ impl Attributes {
}
}

impl From<IndexMap<AttrValue, AttrValue>> for Attributes {
fn from(v: IndexMap<AttrValue, AttrValue>) -> Self {
Self::IndexMap(v)
}
}

impl From<IndexMap<&'static str, AttrValue>> for Attributes {
fn from(v: IndexMap<&'static str, AttrValue>) -> Self {
let v = v
.into_iter()
.map(|(k, v)| (AttrValue::Static(k), v))
.collect();
Self::IndexMap(v)
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/yew/src/virtual_dom/vtag.rs
View file
Open in desktop
Expand Up @@ -351,7 +351,7 @@ impl VTag {
pub fn add_attribute(&mut self, key: &'static str, value: impl Into<AttrValue>) {
self.attributes
.get_mut_index_map()
.insert(key, value.into());
.insert(AttrValue::Static(key), value.into());
}

/// Sets attributes to a virtual node.
Expand All @@ -366,7 +366,7 @@ impl VTag {
pub fn __macro_push_attr(&mut self, key: &'static str, value: impl IntoPropValue<AttrValue>) {
self.attributes
.get_mut_index_map()
.insert(key, value.into_prop_value());
.insert(AttrValue::from(key), value.into_prop_value());
}

/// Add event listener on the [VTag]'s [Element](web_sys::Element).
Expand Down