Closed theduke closed 3 years ago
Investigation shows that CachedNode::set_attribute
is called correctly on each update.
The code that seems relevant is this: https://github.com/anp/moxie/blob/main/dom/src/cached_node.rs#L39
A scopeguard drop handler does node.remove_attribute(&name)
.
So it seems like the scopeguard is not functioning correctly.
Futher finding of something that seems related: the id of the dom node that is used changes with each render. (determined with logging JsValue::into_abi()
).
So something seems to be wrong with the dom node caching.
The builders (like div()
etc) use moxie::cache
to cache the node, so in theory they should remain the same nodes on each run as long as the dom structure does not change, correct?
Looking into this on my end, I think the problem is that the scopeguard is firing after we've already set the updated attribute. The fix that works locally looks like this:
--- a/dom/src/cached_node.rs
+++ b/dom/src/cached_node.rs
@@ -31,15 +31,27 @@ impl CachedNode {
// TODO accept PartialEq+ToString implementors
#[topo::nested(slot = "&(self.id, name)")]
pub(crate) fn set_attribute(&self, name: &'static str, value: &str) {
+ let mut should_set = false;
cache_with(
value,
- |v| {
- self.node.set_attribute(name, v);
+ |_| {
+ // when this isn't the first time the attribute is being set for this element,
+ // this closure executes while the previous attribute's guard is still live.
+ // if we actually set the attribute here, it will be removed when this closure exits
+ // which we definitely don't want. easiest fix is to set the attribute after our
+ // hypothetical cleanup has completed
+ should_set = true;
let name = name.to_owned();
- scopeguard::guard(self.node.clone(), move |node| node.remove_attribute(&name))
+ scopeguard::guard(self.node.clone(), move |node| {
+ node.remove_attribute(&name);
+ })
},
|_| {},
);
+
+ if should_set {
+ self.node.set_attribute(name, value);
+ }
}
Need to write a regression test but will have a PR up shortly.
Attributes based on dynamic values from state are currently not functional.
A self-contained example can be found here: https://github.com/theduke/moxie-reproduction The repo has compiled code committed, just run
http-server
or similar in the root .The exact behaviour is: