From 834d490beb31c2911cb89976db19a3e2a4240965 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sun, 17 Mar 2024 21:14:26 -0700 Subject: [PATCH 1/3] Fix: #2095, #1990 - Don't merge dynamic attributes together unnecessarily - Walk the workspace until we find a target dir with the dioxusin handle --- packages/core/tests/attributes_pass.rs | 32 ++++++++++ packages/hot-reload/src/lib.rs | 29 +++++++++- packages/playwright-tests/package-lock.json | 24 ++++---- packages/playwright-tests/package.json | 2 +- packages/rsx/src/lib.rs | 22 ++----- packages/rsx/tests/hotreloads.rs | 10 ++++ packages/rsx/tests/parsing.rs | 44 ++++++++++++++ .../rsx/tests/parsing/multiexpr.expanded.rsx | 1 + packages/rsx/tests/parsing/multiexpr.rsx | 11 ++++ packages/rsx/tests/valid/combo.new.rsx | 58 +++++++++++++++++++ packages/rsx/tests/valid/combo.old.rsx | 57 ++++++++++++++++++ 11 files changed, 259 insertions(+), 31 deletions(-) create mode 100644 packages/core/tests/attributes_pass.rs create mode 100644 packages/rsx/tests/parsing.rs create mode 100644 packages/rsx/tests/parsing/multiexpr.expanded.rsx create mode 100644 packages/rsx/tests/parsing/multiexpr.rsx create mode 100644 packages/rsx/tests/valid/combo.new.rsx create mode 100644 packages/rsx/tests/valid/combo.old.rsx diff --git a/packages/core/tests/attributes_pass.rs b/packages/core/tests/attributes_pass.rs new file mode 100644 index 000000000..9c124585f --- /dev/null +++ b/packages/core/tests/attributes_pass.rs @@ -0,0 +1,32 @@ +use dioxus::dioxus_core::{ElementId, Mutation::*}; +use dioxus::prelude::*; + +/// Make sure that rsx! is parsing templates and their attributes properly +#[test] +fn attributes_pass_properly() { + let h = rsx! { + circle { + cx: 50, + cy: 50, + r: 40, + stroke: "green", + fill: "yellow" + } + }; + + let o = h.unwrap(); + + let template = &o.template.get(); + + assert_eq!(template.attr_paths.len(), 3); + + let _circle = template.roots[0]; + let TemplateNode::Element { attrs, tag, namespace, children } = _circle else { + panic!("Expected an element"); + }; + + assert_eq!(tag, "circle"); + assert_eq!(namespace, Some("http://www.w3.org/2000/svg")); + assert_eq!(children.len(), 0); + assert_eq!(attrs.len(), 5); +} diff --git a/packages/hot-reload/src/lib.rs b/packages/hot-reload/src/lib.rs index d20e9fbec..222187fee 100644 --- a/packages/hot-reload/src/lib.rs +++ b/packages/hot-reload/src/lib.rs @@ -31,10 +31,35 @@ pub enum HotReloadMsg { /// Connect to the hot reloading listener. The callback provided will be called every time a template change is detected pub fn connect(mut callback: impl FnMut(HotReloadMsg) + Send + 'static) { std::thread::spawn(move || { - let path = PathBuf::from("./").join("target").join("dioxusin"); + // get the cargo manifest directory, where the target dir lives + let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + + // walk the path until we a find a socket named `dioxusin` inside that folder's target directory + loop { + let maybe = path.join("target").join("dioxusin"); + + if maybe.exists() { + path = maybe; + break; + } + + path = match path.parent() { + Some(parent) => parent.to_path_buf(), + None => { + return eprintln!( + "could not find hot reloading server for crate at {}", + env!("CARGO_MANIFEST_DIR") + ) + } + }; + } // There might be a socket since the we're not running under the hot reloading server - let Ok(socket) = LocalSocketStream::connect(path) else { + let Ok(socket) = LocalSocketStream::connect(path.clone()) else { + println!( + "could not find hot reloading server at {:?}, make sure it's running", + path + ); return; }; diff --git a/packages/playwright-tests/package-lock.json b/packages/playwright-tests/package-lock.json index 666d6bd20..862788821 100644 --- a/packages/playwright-tests/package-lock.json +++ b/packages/playwright-tests/package-lock.json @@ -9,16 +9,16 @@ "version": "1.0.0", "license": "ISC", "devDependencies": { - "@playwright/test": "^1.41.2" + "@playwright/test": "^1.42.1" } }, "node_modules/@playwright/test": { - "version": "1.41.2", - "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.41.2.tgz", - "integrity": "sha512-qQB9h7KbibJzrDpkXkYvsmiDJK14FULCCZgEcoe2AvFAS64oCirWTwzTlAYEbKaRxWs5TFesE1Na6izMv3HfGg==", + "version": "1.42.1", + "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.42.1.tgz", + "integrity": "sha512-Gq9rmS54mjBL/7/MvBaNOBwbfnh7beHvS6oS4srqXFcQHpQCV1+c8JXWE8VLPyRDhgS3H8x8A7hztqI9VnwrAQ==", "dev": true, "dependencies": { - "playwright": "1.41.2" + "playwright": "1.42.1" }, "bin": { "playwright": "cli.js" @@ -42,12 +42,12 @@ } }, "node_modules/playwright": { - "version": "1.41.2", - "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.41.2.tgz", - "integrity": "sha512-v0bOa6H2GJChDL8pAeLa/LZC4feoAMbSQm1/jF/ySsWWoaNItvrMP7GEkvEEFyCTUYKMxjQKaTSg5up7nR6/8A==", + "version": "1.42.1", + "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.42.1.tgz", + "integrity": "sha512-PgwB03s2DZBcNRoW+1w9E+VkLBxweib6KTXM0M3tkiT4jVxKSi6PmVJ591J+0u10LUrgxB7dLRbiJqO5s2QPMg==", "dev": true, "dependencies": { - "playwright-core": "1.41.2" + "playwright-core": "1.42.1" }, "bin": { "playwright": "cli.js" @@ -60,9 +60,9 @@ } }, "node_modules/playwright-core": { - "version": "1.41.2", - "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.41.2.tgz", - "integrity": "sha512-VaTvwCA4Y8kxEe+kfm2+uUUw5Lubf38RxF7FpBxLPmGe5sdNkSg5e3ChEigaGrX7qdqT3pt2m/98LiyvU2x6CA==", + "version": "1.42.1", + "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.42.1.tgz", + "integrity": "sha512-mxz6zclokgrke9p1vtdy/COWBH+eOZgYUVVU34C73M+4j4HLlQJHtfcqiqqxpP0o8HhMkflvfbquLX5dg6wlfA==", "dev": true, "bin": { "playwright-core": "cli.js" diff --git a/packages/playwright-tests/package.json b/packages/playwright-tests/package.json index dd0bea722..abb388e3b 100644 --- a/packages/playwright-tests/package.json +++ b/packages/playwright-tests/package.json @@ -12,6 +12,6 @@ "author": "", "license": "ISC", "devDependencies": { - "@playwright/test": "^1.41.2" + "@playwright/test": "^1.42.1" } } diff --git a/packages/rsx/src/lib.rs b/packages/rsx/src/lib.rs index da3a59858..872c33bac 100644 --- a/packages/rsx/src/lib.rs +++ b/packages/rsx/src/lib.rs @@ -453,7 +453,8 @@ impl<'a> DynamicContext<'a> { } } - fn render_static_node(&mut self, root: &'a BodyNode) -> TokenStream2 { + /// Render a portion of an rsx callbody to a token stream + pub fn render_static_node(&mut self, root: &'a BodyNode) -> TokenStream2 { match root { BodyNode::Element(el) => { let el_name = &el.name; @@ -499,20 +500,10 @@ impl<'a> DynamicContext<'a> { } _ => { - // If this attribute is dynamic, but it already exists in the template, we can reuse the index - if let Some(attribute_index) = self - .attr_paths - .iter() - .position(|path| path == &self.current_path) - { - self.dynamic_attributes[attribute_index].push(attr); - quote! {} - } else { - let ct = self.dynamic_attributes.len(); - self.dynamic_attributes.push(vec![attr]); - self.attr_paths.push(self.current_path.clone()); - quote! { dioxus_core::TemplateAttribute::Dynamic { id: #ct }, } - } + let ct = self.dynamic_attributes.len(); + self.dynamic_attributes.push(vec![attr]); + self.attr_paths.push(self.current_path.clone()); + quote! { dioxus_core::TemplateAttribute::Dynamic { id: #ct }, } } }); @@ -525,7 +516,6 @@ impl<'a> DynamicContext<'a> { out }); - let _opt = el.children.len() == 1; let children = quote! { #(#children),* }; let ns = ns(quote!(NAME_SPACE)); diff --git a/packages/rsx/tests/hotreloads.rs b/packages/rsx/tests/hotreloads.rs index 68dedb370..56d56c58b 100644 --- a/packages/rsx/tests/hotreloads.rs +++ b/packages/rsx/tests/hotreloads.rs @@ -28,6 +28,16 @@ fn hotreloads() { diff_rsx(&new, &old), DiffResult::RsxChanged { .. } )); + + let (old, new) = load_files( + include_str!("./valid/combo.old.rsx"), + include_str!("./valid/combo.new.rsx"), + ); + + assert!(matches!( + diff_rsx(&new, &old), + DiffResult::RsxChanged { .. } + )); } #[test] diff --git a/packages/rsx/tests/parsing.rs b/packages/rsx/tests/parsing.rs new file mode 100644 index 000000000..467a36c22 --- /dev/null +++ b/packages/rsx/tests/parsing.rs @@ -0,0 +1,44 @@ +use dioxus_rsx::{CallBody, DynamicContext}; +use syn::Item; + +#[test] +fn rsx_writeout_snapshot() { + let body = parse_from_str(include_str!("./parsing/multiexpr.rsx")); + + assert_eq!(body.roots.len(), 1); + + let root = &body.roots[0]; + + let el = match root { + dioxus_rsx::BodyNode::Element(el) => el, + _ => panic!("Expected an element"), + }; + + assert_eq!(el.name, "circle"); + + assert_eq!(el.attributes.len(), 5); + + let mut context = DynamicContext::default(); + let o = context.render_static_node(&body.roots[0]); + + // hi!!!!! + // you're probably here because you changed something in how rsx! generates templates and need to update the snapshot + // This is a snapshot test. Make sure the contents are checked before committing a new snapshot. + let stability_tested = o.to_string(); + assert_eq!( + stability_tested.trim(), + include_str!("./parsing/multiexpr.expanded.rsx").trim() + ); +} + +fn parse_from_str(contents: &str) -> CallBody { + // Parse the file + let file = syn::parse_file(contents).unwrap(); + + // The first token should be the macro call + let Item::Macro(call) = file.items.first().unwrap() else { + panic!("Expected a macro call"); + }; + + call.mac.parse_body().unwrap() +} diff --git a/packages/rsx/tests/parsing/multiexpr.expanded.rsx b/packages/rsx/tests/parsing/multiexpr.expanded.rsx new file mode 100644 index 000000000..d503733b3 --- /dev/null +++ b/packages/rsx/tests/parsing/multiexpr.expanded.rsx @@ -0,0 +1 @@ +dioxus_core :: TemplateNode :: Element { tag : dioxus_elements :: circle :: TAG_NAME , namespace : dioxus_elements :: circle :: NAME_SPACE , attrs : & [dioxus_core :: TemplateAttribute :: Dynamic { id : 0usize } , dioxus_core :: TemplateAttribute :: Dynamic { id : 1usize } , dioxus_core :: TemplateAttribute :: Dynamic { id : 2usize } , dioxus_core :: TemplateAttribute :: Static { name : dioxus_elements :: circle :: stroke . 0 , namespace : dioxus_elements :: circle :: stroke . 1 , value : "green" , } , dioxus_core :: TemplateAttribute :: Static { name : dioxus_elements :: circle :: fill . 0 , namespace : dioxus_elements :: circle :: fill . 1 , value : "yellow" , } ,] , children : & [] , } diff --git a/packages/rsx/tests/parsing/multiexpr.rsx b/packages/rsx/tests/parsing/multiexpr.rsx new file mode 100644 index 000000000..94340eac5 --- /dev/null +++ b/packages/rsx/tests/parsing/multiexpr.rsx @@ -0,0 +1,11 @@ + +rsx! { + circle { + cx: 50, + cy: 50, + r: 40, + stroke: "green", + fill: "yellow" + } +} + diff --git a/packages/rsx/tests/valid/combo.new.rsx b/packages/rsx/tests/valid/combo.new.rsx new file mode 100644 index 000000000..c24772b81 --- /dev/null +++ b/packages/rsx/tests/valid/combo.new.rsx @@ -0,0 +1,58 @@ +// This test is used by playwright configured in the root of the repo + +use dioxus::prelude::*; + +fn app() -> Element { + let mut num = use_signal(|| 0); + let mut eval_result = use_signal(String::new); + let a = 123; + + rsx! { + div { + "hello axum! {num}" + button { class: "increment-button", onclick: move |_| num += 1, "Increment" } + } + svg { circle { cx: 50, cy: 50, r: 40, stroke: "green", fill: "yellow" } } + div { class: "raw-attribute-div", "raw-attribute": "raw-attribute-value" } + div { class: "hidden-attribute-div", hidden: true } + div { + class: "dangerous-inner-html-div", + dangerous_inner_html: "

hello dangerous inner html

" + } + input { value: "hello input" } + div { class: "style-div", color: "red", "colored text" } + div { class: "style-div", color: "red", "colored text" } + button { + class: "eval-button", + onclick: move |_| async move { + let mut eval = eval( + r#" + window.document.title = 'Hello from Dioxus Eval!'; + dioxus.send("returned eval value"); + "#, + ); + + let result = eval.recv().await; + if let Ok(serde_json::Value::String(string)) = result { + eval_result.set(string); + } + }, + "Eval!!!!" + "Eval!!!!" + "Eval!!!!!" + "Eval!!!!" + "Eval!!!!" + "Eval!!!!" + } + div { class: "eval-result", "{eval_result}" } + } +} + +fn main() { + // tracing_wasm::set_as_global_default_with_config( + // tracing_wasm::WASMLayerConfigBuilder::default() + // .set_max_level(tracing::Level::TRACE) + // .build(), + // ); + launch(app); +} diff --git a/packages/rsx/tests/valid/combo.old.rsx b/packages/rsx/tests/valid/combo.old.rsx new file mode 100644 index 000000000..d08ca4e87 --- /dev/null +++ b/packages/rsx/tests/valid/combo.old.rsx @@ -0,0 +1,57 @@ +// This test is used by playwright configured in the root of the repo + +use dioxus::prelude::*; + +fn app() -> Element { + let mut num = use_signal(|| 0); + let mut eval_result = use_signal(String::new); + let a = 123; + + rsx! { + div { + "hello axum! {num}" + button { class: "increment-button", onclick: move |_| num += 1, "Increment" } + } + svg { circle { cx: 50, cy: 50, r: 40, stroke: "green", fill: "yellow" } } + div { class: "raw-attribute-div", "raw-attribute": "raw-attribute-value" } + div { class: "hidden-attribute-div", hidden: true } + div { + class: "dangerous-inner-html-div", + dangerous_inner_html: "

hello dangerous inner html

" + } + input { value: "hello input" } + div { class: "style-div", color: "red", "colored text" } + div { class: "style-div", color: "red", "colored text" } + button { + class: "eval-button", + onclick: move |_| async move { + let mut eval = eval( + r#" + window.document.title = 'Hello from Dioxus Eval!'; + dioxus.send("returned eval value"); + "#, + ); + + let result = eval.recv().await; + if let Ok(serde_json::Value::String(string)) = result { + eval_result.set(string); + } + }, + "Eval!!!!" + "Eval!!!!" + "Eval!!!!!" + "Eval!!!!" + "Eval!!!!" + } + div { class: "eval-result", "{eval_result}" } + } +} + +fn main() { + // tracing_wasm::set_as_global_default_with_config( + // tracing_wasm::WASMLayerConfigBuilder::default() + // .set_max_level(tracing::Level::TRACE) + // .build(), + // ); + launch(app); +} From be008471994eaee0303deaf1457abdef98bbb58b Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sun, 17 Mar 2024 21:15:57 -0700 Subject: [PATCH 2/3] Don't squawk when running under cargo --- packages/hot-reload/src/lib.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/hot-reload/src/lib.rs b/packages/hot-reload/src/lib.rs index 222187fee..36bae8d7d 100644 --- a/packages/hot-reload/src/lib.rs +++ b/packages/hot-reload/src/lib.rs @@ -43,14 +43,10 @@ pub fn connect(mut callback: impl FnMut(HotReloadMsg) + Send + 'static) { break; } + // It's likely we're running under just cargo and not dx path = match path.parent() { Some(parent) => parent.to_path_buf(), - None => { - return eprintln!( - "could not find hot reloading server for crate at {}", - env!("CARGO_MANIFEST_DIR") - ) - } + None => return, }; } From 6ab6e56de9718cf5c524353c624a5f21f983dc7d Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sun, 17 Mar 2024 21:39:29 -0700 Subject: [PATCH 3/3] Make clippy happy --- packages/core/tests/attributes_pass.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/tests/attributes_pass.rs b/packages/core/tests/attributes_pass.rs index 9c124585f..32907fd6d 100644 --- a/packages/core/tests/attributes_pass.rs +++ b/packages/core/tests/attributes_pass.rs @@ -1,4 +1,3 @@ -use dioxus::dioxus_core::{ElementId, Mutation::*}; use dioxus::prelude::*; /// Make sure that rsx! is parsing templates and their attributes properly