From aaded7981fbad2a9e11438fb3d61ec56c80cebc6 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Tue, 8 Aug 2023 10:56:41 -0700 Subject: [PATCH 1/3] fix boolean attribute rendering in SSR --- packages/ssr/src/cache.rs | 6 ++ packages/ssr/src/renderer.rs | 100 +++++++++++++++++++++++++------- packages/ssr/tests/bool_attr.rs | 38 ++++++++++++ 3 files changed, 122 insertions(+), 22 deletions(-) create mode 100644 packages/ssr/tests/bool_attr.rs diff --git a/packages/ssr/src/cache.rs b/packages/ssr/src/cache.rs index a67fd5f22..564be68a2 100644 --- a/packages/ssr/src/cache.rs +++ b/packages/ssr/src/cache.rs @@ -1,6 +1,8 @@ use dioxus_core::prelude::*; use std::fmt::Write; +use crate::renderer::{str_truthy, BOOL_ATTRS}; + #[derive(Debug)] pub struct StringCache { pub segments: Vec, @@ -86,6 +88,10 @@ impl StringCache { inner_html = Some(value); } else if let Some("style") = namespace { styles.push((name, value)); + } else if BOOL_ATTRS.contains(&name) { + if str_truthy(value) { + write!(chain, " {name}=\"{value}\"",)?; + } } else { write!(chain, " {name}=\"{value}\"")?; } diff --git a/packages/ssr/src/renderer.rs b/packages/ssr/src/renderer.rs index f23e654fb..873d0a036 100644 --- a/packages/ssr/src/renderer.rs +++ b/packages/ssr/src/renderer.rs @@ -1,5 +1,6 @@ use super::cache::Segment; use crate::cache::StringCache; + use dioxus_core::{prelude::*, AttributeValue, DynamicNode, RenderReturn}; use std::collections::HashMap; use std::fmt::Write; @@ -83,18 +84,14 @@ impl Renderer { inner_html = Some(attr); } else if attr.namespace == Some("style") { accumulated_dynamic_styles.push(attr); + } else if BOOL_ATTRS.contains(&attr.name) { + if truthy(&attr.value) { + write!(buf, " {}=", attr.name)?; + write_value(buf, &attr.value)?; + } } else { - match attr.value { - AttributeValue::Text(value) => { - write!(buf, " {}=\"{}\"", attr.name, value)? - } - AttributeValue::Bool(value) => write!(buf, " {}={}", attr.name, value)?, - AttributeValue::Int(value) => write!(buf, " {}={}", attr.name, value)?, - AttributeValue::Float(value) => { - write!(buf, " {}={}", attr.name, value)? - } - _ => {} - }; + write!(buf, " {}=", attr.name)?; + write_value(buf, &attr.value)?; } } Segment::Node(idx) => match &template.dynamic_nodes[*idx] { @@ -153,17 +150,9 @@ impl Renderer { write!(buf, " style=\"")?; } for attr in &accumulated_dynamic_styles { - match attr.value { - AttributeValue::Text(value) => { - write!(buf, "{}:{};", attr.name, value)? - } - AttributeValue::Bool(value) => { - write!(buf, "{}:{};", attr.name, value)? - } - AttributeValue::Float(f) => write!(buf, "{}:{};", attr.name, f)?, - AttributeValue::Int(i) => write!(buf, "{}:{};", attr.name, i)?, - _ => {} - }; + write!(buf, "{}:", attr.name)?; + write_value_unquoted(buf, &attr.value)?; + write!(buf, ";")?; } if !*inside_style_tag { write!(buf, "\"")?; @@ -248,3 +237,70 @@ fn to_string_works() { assert_eq!(out, "
Hello world 1 -->123<-- Hello world 2
nest 1
nest 2
</diiiiiiiiv>
finalize 0
finalize 1
finalize 2
finalize 3
finalize 4
"); } + +pub(crate) const BOOL_ATTRS: &[&str] = &[ + "allowfullscreen", + "allowpaymentrequest", + "async", + "autofocus", + "autoplay", + "checked", + "controls", + "default", + "defer", + "disabled", + "formnovalidate", + "hidden", + "ismap", + "itemscope", + "loop", + "multiple", + "muted", + "nomodule", + "novalidate", + "open", + "playsinline", + "readonly", + "required", + "reversed", + "selected", + "truespeed", + "webkitdirectory", +]; + +pub(crate) fn str_truthy(value: &str) -> bool { + !value.is_empty() && value != "0" && value.to_lowercase() != "false" +} + +pub(crate) fn truthy(value: &AttributeValue) -> bool { + match value { + AttributeValue::Text(value) => str_truthy(value), + AttributeValue::Bool(value) => *value, + AttributeValue::Int(value) => *value != 0, + AttributeValue::Float(value) => *value != 0.0, + _ => false, + } +} + +pub(crate) fn write_value(buf: &mut impl Write, value: &AttributeValue) -> std::fmt::Result { + match value { + AttributeValue::Text(value) => write!(buf, "\"{}\"", value), + AttributeValue::Bool(value) => write!(buf, "{}", value), + AttributeValue::Int(value) => write!(buf, "{}", value), + AttributeValue::Float(value) => write!(buf, "{}", value), + _ => Ok(()), + } +} + +pub(crate) fn write_value_unquoted( + buf: &mut impl Write, + value: &AttributeValue, +) -> std::fmt::Result { + match value { + AttributeValue::Text(value) => write!(buf, "{}", value), + AttributeValue::Bool(value) => write!(buf, "{}", value), + AttributeValue::Int(value) => write!(buf, "{}", value), + AttributeValue::Float(value) => write!(buf, "{}", value), + _ => Ok(()), + } +} diff --git a/packages/ssr/tests/bool_attr.rs b/packages/ssr/tests/bool_attr.rs new file mode 100644 index 000000000..b7720532d --- /dev/null +++ b/packages/ssr/tests/bool_attr.rs @@ -0,0 +1,38 @@ +use dioxus::prelude::*; + +#[test] +fn static_boolean_attributs() { + fn app(cx: Scope) -> Element { + render! { + div { hidden: "false" } + div { hidden: "true" } + } + } + + let mut dom = VirtualDom::new(app); + _ = dom.rebuild(); + + assert_eq!( + dioxus_ssr::render(&dom), + r#"
"# + ); +} + +#[test] +fn dynamic_boolean_attributs() { + fn app(cx: Scope) -> Element { + let inner_html = "
1234
"; + render! { + div { hidden: false } + div { hidden: true } + } + } + + let mut dom = VirtualDom::new(app); + _ = dom.rebuild(); + + assert_eq!( + dioxus_ssr::render(&dom), + r#"
"# + ); +} From e43bdd815f4db8ed1032ede051baf1653f63baef Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Tue, 8 Aug 2023 11:27:29 -0700 Subject: [PATCH 2/3] fix test failures --- packages/router/src/history/mod.rs | 5 ++++- packages/router/src/hooks/use_route.rs | 2 +- packages/router/src/routable.rs | 4 ++-- packages/ssr/src/renderer.rs | 15 +++++++++++++-- packages/ssr/tests/bool_attr.rs | 1 - 5 files changed, 20 insertions(+), 7 deletions(-) diff --git a/packages/router/src/history/mod.rs b/packages/router/src/history/mod.rs index fda386075..41a745f0c 100644 --- a/packages/router/src/history/mod.rs +++ b/packages/router/src/history/mod.rs @@ -88,15 +88,18 @@ pub trait HistoryProvider { /// # use dioxus::prelude::*; /// # #[inline_props] /// # fn Index(cx: Scope) -> Element { todo!() } + /// # fn Other(cx: Scope) -> Element { todo!() } /// #[derive(Clone, Routable, Debug, PartialEq)] /// enum Route { /// #[route("/")] /// Index {}, + /// #[route("/other")] + /// Other {}, /// } /// let mut history = MemoryHistory::::default(); /// assert_eq!(history.can_go_back(), false); /// - /// history.push(Route::Index {}); + /// history.push(Route::Other {}); /// assert_eq!(history.can_go_back(), true); /// ``` #[must_use] diff --git a/packages/router/src/hooks/use_route.rs b/packages/router/src/hooks/use_route.rs index e6aa906d5..ffa9851f2 100644 --- a/packages/router/src/hooks/use_route.rs +++ b/packages/router/src/hooks/use_route.rs @@ -34,7 +34,7 @@ use crate::utils::use_router_internal::use_router_internal; /// /// #[inline_props] /// fn Index(cx: Scope) -> Element { -/// let path = use_route(&cx).unwrap(); +/// let path: Route = use_route(&cx).unwrap(); /// render! { /// h2 { "Current Path" } /// p { "{path}" } diff --git a/packages/router/src/routable.rs b/packages/router/src/routable.rs index cd3dd354d..039e449e5 100644 --- a/packages/router/src/routable.rs +++ b/packages/router/src/routable.rs @@ -162,9 +162,9 @@ pub trait Routable: std::fmt::Display + std::str::FromStr + Clone + 'static { /// /// #[derive(Routable, Clone, PartialEq, Debug)] /// enum Route { - /// #[route("/")] + /// #[route("/home")] /// Home {}, - /// #[route("/about")] + /// #[route("/home/about")] /// About {}, /// } /// diff --git a/packages/ssr/src/renderer.rs b/packages/ssr/src/renderer.rs index 873d0a036..313839520 100644 --- a/packages/ssr/src/renderer.rs +++ b/packages/ssr/src/renderer.rs @@ -1,6 +1,7 @@ use super::cache::Segment; use crate::cache::StringCache; +use dioxus_core::Attribute; use dioxus_core::{prelude::*, AttributeValue, DynamicNode, RenderReturn}; use std::collections::HashMap; use std::fmt::Write; @@ -90,8 +91,7 @@ impl Renderer { write_value(buf, &attr.value)?; } } else { - write!(buf, " {}=", attr.name)?; - write_value(buf, &attr.value)?; + write_attribute(buf, &attr)?; } } Segment::Node(idx) => match &template.dynamic_nodes[*idx] { @@ -282,6 +282,17 @@ pub(crate) fn truthy(value: &AttributeValue) -> bool { } } +pub(crate) fn write_attribute(buf: &mut impl Write, attr: &Attribute) -> std::fmt::Result { + let name = &attr.name; + match attr.value { + AttributeValue::Text(value) => write!(buf, " {name}=\"{value}\""), + AttributeValue::Bool(value) => write!(buf, " {name}={value}"), + AttributeValue::Int(value) => write!(buf, " {name}={value}"), + AttributeValue::Float(value) => write!(buf, " {name}={value}"), + _ => Ok(()), + } +} + pub(crate) fn write_value(buf: &mut impl Write, value: &AttributeValue) -> std::fmt::Result { match value { AttributeValue::Text(value) => write!(buf, "\"{}\"", value), diff --git a/packages/ssr/tests/bool_attr.rs b/packages/ssr/tests/bool_attr.rs index b7720532d..778e90dcc 100644 --- a/packages/ssr/tests/bool_attr.rs +++ b/packages/ssr/tests/bool_attr.rs @@ -21,7 +21,6 @@ fn static_boolean_attributs() { #[test] fn dynamic_boolean_attributs() { fn app(cx: Scope) -> Element { - let inner_html = "
1234
"; render! { div { hidden: false } div { hidden: true } From 25f02904e9977f20b39cb09a17842798acd37a06 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Tue, 8 Aug 2023 11:39:36 -0700 Subject: [PATCH 3/3] make clippy happy --- packages/desktop/src/protocol.rs | 9 +++++++-- packages/html/src/eval.rs | 2 +- packages/native-core/src/real_dom.rs | 1 - packages/rsx/src/lib.rs | 10 ++-------- packages/ssr/src/cache.rs | 2 +- packages/ssr/src/renderer.rs | 2 +- 6 files changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/desktop/src/protocol.rs b/packages/desktop/src/protocol.rs index 89ce50a48..53652f82f 100644 --- a/packages/desktop/src/protocol.rs +++ b/packages/desktop/src/protocol.rs @@ -158,8 +158,13 @@ fn get_mime_from_path(trimmed: &Path) -> Result<&'static str> { } let res = match infer::get_from_path(trimmed)?.map(|f| f.mime_type()) { - Some(t) if t == "text/plain" => get_mime_by_ext(trimmed), - Some(f) => f, + Some(f) => { + if f == "text/plain" { + get_mime_by_ext(trimmed) + } else { + f + } + } None => get_mime_by_ext(trimmed), }; diff --git a/packages/html/src/eval.rs b/packages/html/src/eval.rs index b39eef805..c90f5125b 100644 --- a/packages/html/src/eval.rs +++ b/packages/html/src/eval.rs @@ -42,7 +42,7 @@ pub fn use_eval(cx: &ScopeState) -> &EvalCreator { Rc::new(move |script: &str| { eval_provider .new_evaluator(script.to_string()) - .map(|evaluator| UseEval::new(evaluator)) + .map(UseEval::new) }) as Rc Result> }) } diff --git a/packages/native-core/src/real_dom.rs b/packages/native-core/src/real_dom.rs index 44c378ddf..b520b2de1 100644 --- a/packages/native-core/src/real_dom.rs +++ b/packages/native-core/src/real_dom.rs @@ -446,7 +446,6 @@ impl RealDom { drop(tree); children.reverse(); if let Some(node) = self.get_mut(id) { - let node = node; f(node); stack.extend(children.iter()); } diff --git a/packages/rsx/src/lib.rs b/packages/rsx/src/lib.rs index 1f01fcd72..783dd3322 100644 --- a/packages/rsx/src/lib.rs +++ b/packages/rsx/src/lib.rs @@ -288,10 +288,7 @@ impl DynamicMapping { let idx = self.last_attribute_idx; self.last_attribute_idx += 1; - self.attribute_to_idx - .entry(attr) - .or_insert_with(Vec::new) - .push(idx); + self.attribute_to_idx.entry(attr).or_default().push(idx); idx } @@ -300,10 +297,7 @@ impl DynamicMapping { let idx = self.last_element_idx; self.last_element_idx += 1; - self.node_to_idx - .entry(node) - .or_insert_with(Vec::new) - .push(idx); + self.node_to_idx.entry(node).or_default().push(idx); idx } diff --git a/packages/ssr/src/cache.rs b/packages/ssr/src/cache.rs index 564be68a2..bcd6ba71f 100644 --- a/packages/ssr/src/cache.rs +++ b/packages/ssr/src/cache.rs @@ -88,7 +88,7 @@ impl StringCache { inner_html = Some(value); } else if let Some("style") = namespace { styles.push((name, value)); - } else if BOOL_ATTRS.contains(&name) { + } else if BOOL_ATTRS.contains(name) { if str_truthy(value) { write!(chain, " {name}=\"{value}\"",)?; } diff --git a/packages/ssr/src/renderer.rs b/packages/ssr/src/renderer.rs index 313839520..20461eb40 100644 --- a/packages/ssr/src/renderer.rs +++ b/packages/ssr/src/renderer.rs @@ -91,7 +91,7 @@ impl Renderer { write_value(buf, &attr.value)?; } } else { - write_attribute(buf, &attr)?; + write_attribute(buf, attr)?; } } Segment::Node(idx) => match &template.dynamic_nodes[*idx] {