Skip to content

Commit

Permalink
Fix hot reload diffing to empty rsx (#3567)
Browse files Browse the repository at this point in the history
* Fix hot reloading to empty rsx

* Only normalize the template when hot reloading and rendering

* add a test for emptying out an rsx block in hot reloading
  • Loading branch information
ealmloff authored Jan 21, 2025
1 parent 2c6a521 commit f99e50b
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 26 deletions.
7 changes: 5 additions & 2 deletions packages/rsx-hotreload/src/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,10 @@ impl HotReloadResult {
new: &TemplateBody,
name: String,
) -> Option<Self> {
let full_rebuild_state = LastBuildState::new(full_rebuild_state, name);
// Normalize both the full rebuild state and the new state for rendering
let full_rebuild_state = full_rebuild_state.normalized();
let new = new.normalized();
let full_rebuild_state = LastBuildState::new(&full_rebuild_state, name);
let mut s = Self {
full_rebuild_state,
templates: Default::default(),
Expand All @@ -131,7 +134,7 @@ impl HotReloadResult {
literal_component_properties: Default::default(),
};

s.hotreload_body::<Ctx>(new)?;
s.hotreload_body::<Ctx>(&new)?;

Some(s)
}
Expand Down
26 changes: 26 additions & 0 deletions packages/rsx-hotreload/tests/hotreload_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,32 @@ fn invalid_cases() {
assert!(added.is_none());
}

#[test]
fn invalid_empty_rsx() {
let old_template = quote! {
div {
for item in vec![1, 2, 3, 4] {
div { "asasddasdasd" }
div { "123" }
}
for item in vec![4, 5, 6] {
span { "asasddasdasd" }
span { "123" }
}
}
};

// empty out the whole rsx block
let new_template = quote! {};

let location = "file:line:col:0";

let old_template: CallBody = syn::parse2(old_template).unwrap();
let new_template: CallBody = syn::parse2(new_template).unwrap();

assert!(hotreload_callbody::<Mock>(&old_template, &new_template).is_none());
}

#[test]
fn attributes_reload() {
let old = quote! {
Expand Down
54 changes: 30 additions & 24 deletions packages/rsx/src/template_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ impl Parse for TemplateBody {
myself
.diagnostics
.extend(children.diagnostics.into_diagnostics());

Ok(myself)
}
}
Expand All @@ -99,51 +100,39 @@ impl Parse for TemplateBody {
/// This is because the parsing phase filled in all the additional metadata we need
impl ToTokens for TemplateBody {
fn to_tokens(&self, tokens: &mut TokenStream2) {
// If the nodes are completely empty, insert a placeholder node
// Core expects at least one node in the template to make it easier to replace
if self.is_empty() {
// Create an empty template body with a placeholder and diagnostics + the template index from the original
let empty = Self::new(vec![BodyNode::RawExpr(parse_quote! {()})]);
let default = Self {
diagnostics: self.diagnostics.clone(),
template_idx: self.template_idx.clone(),
..empty
};
// And then render the default template body
default.to_tokens(tokens);
return;
}
// First normalize the template body for rendering
let node = self.normalized();

// If we have an implicit key, then we need to write its tokens
let key_tokens = match self.implicit_key() {
let key_tokens = match node.implicit_key() {
Some(tok) => quote! { Some( #tok.to_string() ) },
None => quote! { None },
};

let roots = self.quote_roots();
let roots = node.quote_roots();

// Print paths is easy - just print the paths
let node_paths = self.node_paths.iter().map(|it| quote!(&[#(#it),*]));
let attr_paths = self.attr_paths.iter().map(|(it, _)| quote!(&[#(#it),*]));
let node_paths = node.node_paths.iter().map(|it| quote!(&[#(#it),*]));
let attr_paths = node.attr_paths.iter().map(|(it, _)| quote!(&[#(#it),*]));

// For printing dynamic nodes, we rely on the ToTokens impl
// Elements have a weird ToTokens - they actually are the entrypoint for Template creation
let dynamic_nodes: Vec<_> = self.dynamic_nodes().collect();
let dynamic_nodes: Vec<_> = node.dynamic_nodes().collect();
let dynamic_nodes_len = dynamic_nodes.len();

// We could add a ToTokens for Attribute but since we use that for both components and elements
// They actually need to be different, so we just localize that here
let dyn_attr_printer: Vec<_> = self
let dyn_attr_printer: Vec<_> = node
.dynamic_attributes()
.map(|attr| attr.rendered_as_dynamic_attr())
.collect();
let dynamic_attr_len = dyn_attr_printer.len();

let dynamic_text = self.dynamic_text_segments.iter();
let dynamic_text = node.dynamic_text_segments.iter();

let diagnostics = &self.diagnostics;
let index = self.template_idx.get();
let hot_reload_mapping = self.hot_reload_mapping();
let diagnostics = &node.diagnostics;
let index = node.template_idx.get();
let hot_reload_mapping = node.hot_reload_mapping();

tokens.append_all(quote! {
dioxus_core::Element::Ok({
Expand Down Expand Up @@ -255,6 +244,23 @@ impl TemplateBody {
body
}

/// Normalize the Template body for rendering. If the body is completely empty, insert a placeholder node
pub fn normalized(&self) -> Self {
// If the nodes are completely empty, insert a placeholder node
// Core expects at least one node in the template to make it easier to replace
if self.is_empty() {
// Create an empty template body with a placeholder and diagnostics + the template index from the original
let empty = Self::new(vec![BodyNode::RawExpr(parse_quote! {()})]);
let default = Self {
diagnostics: self.diagnostics.clone(),
template_idx: self.template_idx.clone(),
..empty
};
return default;
}
self.clone()
}

pub fn is_empty(&self) -> bool {
self.roots.is_empty()
}
Expand Down

0 comments on commit f99e50b

Please sign in to comment.