Skip to content

Commit

Permalink
Subscribe/unsubscribe signals only belonging to diff
Browse files Browse the repository at this point in the history
  • Loading branch information
lukechu10 committed Jul 8, 2021
1 parent d0cbc68 commit db0ef30
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 11 deletions.
3 changes: 2 additions & 1 deletion packages/sycamore/src/generic_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ pub trait GenericNode: fmt::Debug + Clone + PartialEq + Eq + Hash + 'static {

/// Sets the `class` attribute on a node.
/// This should have the same outcome as calling `set_attribute("class", value)`.
/// For [`DomNode`], this sets the `className` property directly which is about 2x faster (on Chrome).
/// For [`DomNode`], this sets the `className` property directly which is about 2x faster (on
/// Chrome).
fn set_class_name(&self, value: &str);

/// Sets a property on a node.
Expand Down
47 changes: 37 additions & 10 deletions packages/sycamore/src/rx/effect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ pub(super) struct Running {

impl Running {
/// Clears the dependencies (both links and backlinks).
/// Should be called when re-executing an effect to recreate all dependencies.
fn clear_dependencies(&mut self) {
for dependency in &self.dependencies {
dependency
Expand All @@ -49,6 +48,16 @@ impl Running {
}
self.dependencies.clear();
}

/// Returns the current dependencies and replaces dependencies on the [`Running`] with a new
/// [`HashSet`] with the same capacity as the old [`HashSet`] because the number of dependencies
/// is unlikely to change.
///
/// Does **NOT** unsubscribe the signals pointed to by the dependencies.
fn take_dependencies(&mut self) -> HashSet<Dependency> {
let len = self.dependencies.len();
mem::replace(&mut self.dependencies, HashSet::with_capacity(len))
}
}

/// Owns the effects created in the current reactive scope.
Expand Down Expand Up @@ -192,9 +201,19 @@ pub fn create_effect_initial<R: 'static>(
*ret.borrow_mut() = Some(ret_tmp);
});
running.borrow_mut().as_mut().unwrap().scope = scope;

// Attach new dependencies.
for dep in &running.borrow().as_ref().unwrap().dependencies {
dep.signal().subscribe(Callback(Rc::downgrade(
// Reference the same closure we are in right now.
// When the dependency changes, this closure will be called again.
&running.borrow().as_ref().unwrap().execute,
)));
}
} else {
// Recreate effect dependencies each time effect is called.
running.borrow_mut().as_mut().unwrap().clear_dependencies();
let old_dependencies =
running.borrow_mut().as_mut().unwrap().take_dependencies();

// Destroy old effects before new ones run.

Expand All @@ -208,15 +227,23 @@ pub fn create_effect_initial<R: 'static>(
effect.as_mut().unwrap()();
});
running.borrow_mut().as_mut().unwrap().scope = new_scope;
}

// Attach new dependencies.
for dependency in &running.borrow().as_ref().unwrap().dependencies {
dependency.signal().subscribe(Callback(Rc::downgrade(
// Reference the same closure we are in right now.
// When the dependency changes, this closure will be called again.
&running.borrow().as_ref().unwrap().execute,
)));
let running = running.borrow();
let new_dependencies = &running.as_ref().unwrap().dependencies;
// Unsubscribe from deps in old_dependencies not found in new_dependencies.
for dep in old_dependencies.difference(new_dependencies) {
dep.signal().unsubscribe(&Callback(Rc::downgrade(
&running.as_ref().unwrap().execute,
)));
}
// Subscribe to deps in new_dependencies not found in old_dependencies.
for dep in new_dependencies.difference(&old_dependencies) {
dep.signal().subscribe(Callback(Rc::downgrade(
// Reference the same closure we are in right now.
// When the dependency changes, this closure will be called again.
&running.as_ref().unwrap().execute,
)));
}
}

// Remove reactive context.
Expand Down

0 comments on commit db0ef30

Please sign in to comment.