dnrops
/
leptos
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

change: run effects after a tick (#1680)

This commit is contained in:
Greg Johnston 2023-09-11 21:01:35 -04:00 committed by GitHub
parent 651356a9ec
commit a317874f93
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 294 additions and 284 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
docs/book/src/ssr/24_hydration_bugs.md
View File

@ -87,43 +87,6 @@ The WASM version of your app, running in the browser, expects to find three item
It’s pretty rare that you do this intentionally, but it could happen from somehow running different logic on the server and in the browser. If you’re seeing warnings like this and you don’t think it’s your fault, it’s much more likely that it’s a bug with `<Suspense/>` or something. Feel free to go ahead and open an [issue](https://github.com/leptos-rs/leptos/issues) or [discussion](https://github.com/leptos-rs/leptos/discussions) on GitHub for help.
### Mutating the DOM during rendering
This is a slightly more common way to create a client/server mismatch: updating a signal _during rendering_ in a way that mutates the view.
```rust
#[component]
pub fn App() -> impl IntoView {
let (loaded, set_loaded) = create_signal(false);
// create_effect only runs on the client
create_effect(move |_| {
// do something like reading from localStorage
set_loaded(true);
});
move || {
if loaded() {
view! { <p>"Hello, world!"</p> }.into_any()
} else {
view! { <div class="loading">"Loading..."</div> }.into_any()
}
}
}
```
This one gives us the scary panic
```
panicked at 'assertion failed: `(left == right)`
left: `"DIV"`,
right: `"P"`: SSR and CSR elements have the same hydration key but different node kinds.
```
And a handy link to this page!
The problem here is that `create_effect` runs **immediately** and **synchronously**, but only in the browser. As a result, on the server, `loaded` is false, and a `<div>` is rendered. But on the browser, by the time the view is being rendered, `loaded` has already been set to `true`, and the browser is expecting to find a `<p>`.
#### Solution
You can simply tell the effect to wait a tick before updating the signal, by using something like `request_animation_frame`, which will set a short timeout and then update the signal before the next frame.

8
examples/todomvc/src/lib.rs
View File

@ -210,13 +210,7 @@ pub fn TodoMVC() -> impl IntoView {
// focus the main input on load
create_effect(move |_| {
if let Some(input) = input_ref.get() {
// We use request_animation_frame here because the NodeRef
// is filled when the element is created, but before it's mounted
// to the DOM. Calling .focus() before it's mounted does nothing.
// So inside, we wait a tick for the browser to mount it, then .focus()
request_animation_frame(move || {
let _ = input.focus();
});
let _ = input.focus();
}
});

322
leptos_dom/src/components/dyn_child.rs
View File

@ -7,7 +7,7 @@ use std::{cell::RefCell, fmt, ops::Deref, rc::Rc};
cfg_if! {
if #[cfg(all(target_arch = "wasm32", feature = "web"))] {
use crate::{mount_child, prepare_to_move, unmount_child, MountKind, Mountable, Text};
use leptos_reactive::create_effect;
use leptos_reactive::create_render_effect;
use wasm_bindgen::JsCast;
}
}
@ -181,194 +181,204 @@ where
let span = tracing::Span::current();
#[cfg(all(target_arch = "wasm32", feature = "web"))]
create_effect(move |prev_run: Option<Option<web_sys::Node>>| {
#[cfg(debug_assertions)]
let _guard = span.enter();
create_render_effect(
move |prev_run: Option<Option<web_sys::Node>>| {
#[cfg(debug_assertions)]
let _guard = span.enter();
let new_child = child_fn().into_view();
let new_child = child_fn().into_view();
let mut child_borrow = child.borrow_mut();
let mut child_borrow = child.borrow_mut();
// Is this at least the second time we are loading a child?
if let Some(prev_t) = prev_run {
let child = child_borrow.take().unwrap();
// Is this at least the second time we are loading a child?
if let Some(prev_t) = prev_run {
let child = child_borrow.take().unwrap();
// We need to know if our child wasn't moved elsewhere.
// If it was, `DynChild` no longer "owns" that child, and
// is therefore no longer sound to unmount it from the DOM
// or to reuse it in the case of a text node
// We need to know if our child wasn't moved elsewhere.
// If it was, `DynChild` no longer "owns" that child, and
// is therefore no longer sound to unmount it from the DOM
// or to reuse it in the case of a text node
// TODO check does this still detect moves correctly?
let was_child_moved = prev_t.is_none()
&& child
.get_closing_node()
.next_non_view_marker_sibling()
.as_ref()
!= Some(&closing);
// TODO check does this still detect moves correctly?
let was_child_moved = prev_t.is_none()
&& child
.get_closing_node()
.next_non_view_marker_sibling()
.as_ref()
!= Some(&closing);
// If the previous child was a text node, we would like to
// make use of it again if our current child is also a text
// node
let ret = if let Some(prev_t) = prev_t {
// Here, our child is also a text node
// If the previous child was a text node, we would like to
// make use of it again if our current child is also a text
// node
let ret = if let Some(prev_t) = prev_t {
// Here, our child is also a text node
// nb: the match/ownership gymnastics here
// are so that, if we can reuse the text node,
// we can take ownership of new_t so we don't clone
// the contents, which in O(n) on the length of the text
if matches!(new_child, View::Text(_)) {
if !was_child_moved && child != new_child {
let mut new_t = match new_child {
View::Text(t) => t,
_ => unreachable!(),
};
prev_t
.unchecked_ref::<web_sys::Text>()
.set_data(&new_t.content);
// nb: the match/ownership gymnastics here
// are so that, if we can reuse the text node,
// we can take ownership of new_t so we don't clone
// the contents, which in O(n) on the length of the text
if matches!(new_child, View::Text(_)) {
if !was_child_moved && child != new_child {
let mut new_t = match new_child {
View::Text(t) => t,
_ => unreachable!(),
};
prev_t
.unchecked_ref::<web_sys::Text>()
.set_data(&new_t.content);
// replace new_t's text node with the prev node
// see discussion: https://github.com/leptos-rs/leptos/pull/1472
new_t.node = prev_t.clone();
// replace new_t's text node with the prev node
// see discussion: https://github.com/leptos-rs/leptos/pull/1472
new_t.node = prev_t.clone();
let new_child = View::Text(new_t);
**child_borrow = Some(new_child);
let new_child = View::Text(new_t);
**child_borrow = Some(new_child);
Some(prev_t)
} else {
let new_t = new_child.as_text().unwrap();
Some(prev_t)
} else {
let new_t = new_child.as_text().unwrap();
mount_child(
MountKind::Before(&closing),
&new_child,
);
**child_borrow = Some(new_child.clone());
Some(new_t.node.clone())
}
}
// Child is not a text node, so we can remove the previous
// text node
else {
if !was_child_moved && child != new_child {
// Remove the text
closing
.previous_non_view_marker_sibling()
.unwrap()
.unchecked_into::<web_sys::Element>()
.remove();
}
// Mount the new child, and we're done
mount_child(
MountKind::Before(&closing),
&new_child,
);
**child_borrow = Some(new_child.clone());
**child_borrow = Some(new_child);
Some(new_t.node.clone())
None
}
}
// Child is not a text node, so we can remove the previous
// text node
// Otherwise, the new child can still be a text node,
// but we know the previous child was not, so no special
// treatment here
else {
if !was_child_moved && child != new_child {
// Remove the text
closing
.previous_non_view_marker_sibling()
.unwrap()
.unchecked_into::<web_sys::Element>()
.remove();
// Technically, I think this check shouldn't be necessary, but
// I can imagine some edge case that the child changes while
// hydration is ongoing
if !HydrationCtx::is_hydrating() {
let same_child = child == new_child;
if !was_child_moved && !same_child {
// Remove the child
let start = child.get_opening_node();
let end = &closing;
match child {
View::CoreComponent(
crate::CoreComponent::DynChild(
child,
),
) => {
let start =
child.get_opening_node();
let end = child.closing.node;
prepare_to_move(
&child.document_fragment,
&start,
&end,
);
}
View::Component(child) => {
let start =
child.get_opening_node();
let end = child.closing.node;
prepare_to_move(
&child.document_fragment,
&start,
&end,
);
}
_ => unmount_child(&start, end),
}
}
// Mount the new child
// If it's the same child, don't re-mount
if !same_child {
mount_child(
MountKind::Before(&closing),
&new_child,
);
}
}
// Mount the new child, and we're done
// We want to reuse text nodes, so hold onto it if
// our child is one
let t =
new_child.get_text().map(|t| t.node.clone());
**child_borrow = Some(new_child);
t
};
ret
}
// Otherwise, we know for sure this is our first time
else {
// If it's a text node, we want to use the old text node
// as the text node for the DynChild, rather than the new
// text node being created during hydration
let new_child = if HydrationCtx::is_hydrating()
&& new_child.get_text().is_some()
{
let t = closing
.previous_non_view_marker_sibling()
.unwrap()
.unchecked_into::<web_sys::Text>();
let new_child = match new_child {
View::Text(text) => text,
_ => unreachable!(),
};
t.set_data(&new_child.content);
View::Text(Text {
node: t.unchecked_into(),
content: new_child.content,
})
} else {
new_child
};
// If we are not hydrating, we simply mount the child
if !HydrationCtx::is_hydrating() {
mount_child(
MountKind::Before(&closing),
&new_child,
);
**child_borrow = Some(new_child);
None
}
}
// Otherwise, the new child can still be a text node,
// but we know the previous child was not, so no special
// treatment here
else {
// Technically, I think this check shouldn't be necessary, but
// I can imagine some edge case that the child changes while
// hydration is ongoing
if !HydrationCtx::is_hydrating() {
let same_child = child == new_child;
if !was_child_moved && !same_child {
// Remove the child
let start = child.get_opening_node();
let end = &closing;
match child {
View::CoreComponent(
crate::CoreComponent::DynChild(child),
) => {
let start = child.get_opening_node();
let end = child.closing.node;
prepare_to_move(
&child.document_fragment,
&start,
&end,
);
}
View::Component(child) => {
let start = child.get_opening_node();
let end = child.closing.node;
prepare_to_move(
&child.document_fragment,
&start,
&end,
);
}
_ => unmount_child(&start, end),
}
}
// Mount the new child
// If it's the same child, don't re-mount
if !same_child {
mount_child(
MountKind::Before(&closing),
&new_child,
);
}
}
// We want to reuse text nodes, so hold onto it if
// our child is one
// We want to update text nodes, rather than replace them, so
// make sure to hold onto the text node
let t = new_child.get_text().map(|t| t.node.clone());
**child_borrow = Some(new_child);
t
};
ret
}
// Otherwise, we know for sure this is our first time
else {
// If it's a text node, we want to use the old text node
// as the text node for the DynChild, rather than the new
// text node being created during hydration
let new_child = if HydrationCtx::is_hydrating()
&& new_child.get_text().is_some()
{
let t = closing
.previous_non_view_marker_sibling()
.unwrap()
.unchecked_into::<web_sys::Text>();
let new_child = match new_child {
View::Text(text) => text,
_ => unreachable!(),
};
t.set_data(&new_child.content);
View::Text(Text {
node: t.unchecked_into(),
content: new_child.content,
})
} else {
new_child
};
// If we are not hydrating, we simply mount the child
if !HydrationCtx::is_hydrating() {
mount_child(MountKind::Before(&closing), &new_child);
}
// We want to update text nodes, rather than replace them, so
// make sure to hold onto the text node
let t = new_child.get_text().map(|t| t.node.clone());
**child_borrow = Some(new_child);
t
}
});
},
);
#[cfg(not(all(target_arch = "wasm32", feature = "web")))]
{

156
leptos_dom/src/components/each.rs
View File

@ -12,7 +12,7 @@ mod web {
mount_child, prepare_to_move, MountKind, Mountable, RANGE,
};
pub use drain_filter_polyfill::VecExt as VecDrainFilterExt;
pub use leptos_reactive::create_effect;
pub use leptos_reactive::create_render_effect;
pub use std::cell::OnceCell;
pub use wasm_bindgen::JsCast;
}
@ -406,90 +406,100 @@ where
let each_fn = as_child_of_current_owner(each_fn);
#[cfg(all(target_arch = "wasm32", feature = "web"))]
create_effect(move |prev_hash_run: Option<HashRun<FxIndexSet<K>>>| {
let mut children_borrow = children.borrow_mut();
create_render_effect(
move |prev_hash_run: Option<HashRun<FxIndexSet<K>>>| {
let mut children_borrow = children.borrow_mut();
#[cfg(all(
not(debug_assertions),
target_arch = "wasm32",
feature = "web"
))]
let opening = if let Some(Some(child)) = children_borrow.get(0) {
// correctly remove opening <!--<EachItem/>-->
let child_opening = child.get_opening_node();
#[cfg(debug_assertions)]
#[cfg(all(
not(debug_assertions),
target_arch = "wasm32",
feature = "web"
))]
let opening = if let Some(Some(child)) = children_borrow.get(0)
{
use crate::components::dyn_child::NonViewMarkerSibling;
child_opening
.previous_non_view_marker_sibling()
.unwrap_or(child_opening)
// correctly remove opening <!--<EachItem/>-->
let child_opening = child.get_opening_node();
#[cfg(debug_assertions)]
{
use crate::components::dyn_child::NonViewMarkerSibling;
child_opening
.previous_non_view_marker_sibling()
.unwrap_or(child_opening)
}
#[cfg(not(debug_assertions))]
{
child_opening
}
} else {
closing.clone()
};
let items_iter = items_fn().into_iter();
let (capacity, _) = items_iter.size_hint();
let mut hashed_items = FxIndexSet::with_capacity_and_hasher(
capacity,
Default::default(),
);
if let Some(HashRun(prev_hash_run)) = prev_hash_run {
if !prev_hash_run.is_empty() {
let mut items = Vec::with_capacity(capacity);
for item in items_iter {
hashed_items.insert(key_fn(&item));
items.push(Some(item));
}
let cmds = diff(&prev_hash_run, &hashed_items);
apply_diff(
#[cfg(all(
target_arch = "wasm32",
feature = "web"
))]
&opening,
#[cfg(all(
target_arch = "wasm32",
feature = "web"
))]
&closing,
cmds,
&mut children_borrow,
items,
&each_fn,
);
return HashRun(hashed_items);
}
}
#[cfg(not(debug_assertions))]
{
child_opening
}
} else {
closing.clone()
};
let items_iter = items_fn().into_iter();
// if previous run is empty
*children_borrow = Vec::with_capacity(capacity);
#[cfg(all(target_arch = "wasm32", feature = "web"))]
let fragment = crate::document().create_document_fragment();
let (capacity, _) = items_iter.size_hint();
let mut hashed_items = FxIndexSet::with_capacity_and_hasher(
capacity,
Default::default(),
);
for item in items_iter {
hashed_items.insert(key_fn(&item));
let (child, disposer) = each_fn(item);
let each_item = EachItem::new(disposer, child.into_view());
if let Some(HashRun(prev_hash_run)) = prev_hash_run {
if !prev_hash_run.is_empty() {
let mut items = Vec::with_capacity(capacity);
for item in items_iter {
hashed_items.insert(key_fn(&item));
items.push(Some(item));
#[cfg(all(target_arch = "wasm32", feature = "web"))]
{
_ = fragment
.append_child(&each_item.get_mountable_node());
}
let cmds = diff(&prev_hash_run, &hashed_items);
apply_diff(
#[cfg(all(target_arch = "wasm32", feature = "web"))]
&opening,
#[cfg(all(target_arch = "wasm32", feature = "web"))]
&closing,
cmds,
&mut children_borrow,
items,
&each_fn,
);
return HashRun(hashed_items);
children_borrow.push(Some(each_item));
}
}
// if previous run is empty
*children_borrow = Vec::with_capacity(capacity);
#[cfg(all(target_arch = "wasm32", feature = "web"))]
let fragment = crate::document().create_document_fragment();
for item in items_iter {
hashed_items.insert(key_fn(&item));
let (child, disposer) = each_fn(item);
let each_item = EachItem::new(disposer, child.into_view());
#[cfg(all(target_arch = "wasm32", feature = "web"))]
{
_ = fragment.append_child(&each_item.get_mountable_node());
}
closing
.unchecked_ref::<web_sys::Element>()
.before_with_node_1(&fragment)
.expect("before to not err");
children_borrow.push(Some(each_item));
}
#[cfg(all(target_arch = "wasm32", feature = "web"))]
closing
.unchecked_ref::<web_sys::Element>()
.before_with_node_1(&fragment)
.expect("before to not err");
HashRun(hashed_items)
});
HashRun(hashed_items)
},
);
#[cfg(not(all(target_arch = "wasm32", feature = "web")))]
{

2
leptos_dom/src/html.rs
View File

@ -774,7 +774,7 @@ impl<El: ElementDescriptor + 'static> HtmlElement<El> {
let class_list = self.element.as_ref().class_list();
leptos_reactive::create_effect(
leptos_reactive::create_render_effect(
move |prev_classes: Option<
SmallVec<[Oco<'static, str>; 4]>,
>| {

4
leptos_dom/src/node_ref.rs
View File

@ -1,6 +1,6 @@
use crate::{html::ElementDescriptor, HtmlElement};
use leptos_reactive::{
create_effect, create_rw_signal, signal_prelude::*, RwSignal,
create_render_effect, create_rw_signal, signal_prelude::*, RwSignal,
};
use std::cell::Cell;
@ -134,7 +134,7 @@ impl<T: ElementDescriptor + 'static> NodeRef<T> {
{
let f = Cell::new(Some(f));
create_effect(move |_| {
create_render_effect(move |_| {
if let Some(node_ref) = self.get() {
f.take().unwrap()(node_ref);
}

49
leptos_reactive/src/effect.rs
View File

@ -3,15 +3,22 @@ use cfg_if::cfg_if;
use std::{any::Any, cell::RefCell, marker::PhantomData, rc::Rc};
/// Effects run a certain chunk of code whenever the signals they depend on change.
/// `create_effect` immediately runs the given function once, tracks its dependence
/// `create_effect` queues the given function to run once, tracks its dependence
/// on any signal values read within it, and reruns the function whenever the value
/// of a dependency changes.
///
/// Effects are intended to run *side-effects* of the system, not to synchronize state
/// *within* the system. In other words: don't write to signals within effects.
/// *within* the system. In other words: don't write to signals within effects, unless
/// you’re coordinating with some other non-reactive side effect.
/// (If you need to define a signal that depends on the value of other signals, use a
/// derived signal or [`create_memo`](crate::create_memo)).
///
/// This first run is queued for the next microtask, i.e., it runs after all other
/// synchronous code has completed. In practical terms, this means that if you use
/// `create_effect` in the body of the component, it will run *after* the view has been
/// created and (presumably) mounted. (If you need an effect that runs immediately, use
/// [`create_render_effect`].)
///
/// The effect function is called with an argument containing whatever value it returned
/// the last time it ran. On the initial run, this is `None`.
///
@ -63,12 +70,20 @@ where
{
cfg_if! {
if #[cfg(not(feature = "ssr"))] {
use crate::{Owner, queue_microtask, with_owner};
let runtime = Runtime::current();
let owner = Owner::current();
let id = runtime.create_effect(f);
//crate::macros::debug_warn!("creating effect {e:?}");
_ = with_runtime( |runtime| {
runtime.update_if_necessary(id);
queue_microtask(move || {
with_owner(owner.unwrap(), move || {
_ = with_runtime( |runtime| {
runtime.update_if_necessary(id);
});
});
});
Effect { id, ty: PhantomData }
} else {
// clear warnings
@ -246,7 +261,10 @@ where
}
}
#[doc(hidden)]
/// Creates an effect exactly like [`create_effect`], but runs immediately rather
/// than being queued until the end of the current microtask. This is mostly used
/// inside the renderer but is available for use cases in which scheduling the effect
/// for the next tick is not optimal.
#[cfg_attr(
any(debug_assertions, feature="ssr"),
instrument(
@ -258,11 +276,26 @@ where
)
)]
#[inline(always)]
pub fn create_render_effect<T>(f: impl Fn(Option<T>) -> T + 'static)
pub fn create_render_effect<T>(
f: impl Fn(Option<T>) -> T + 'static,
) -> Effect<T>
where
T: 'static,
{
create_effect(f);
cfg_if! {
if #[cfg(not(feature = "ssr"))] {
let runtime = Runtime::current();
let id = runtime.create_effect(f);
_ = with_runtime( |runtime| {
runtime.update_if_necessary(id);
});
Effect { id, ty: PhantomData }
} else {
// clear warnings
_ = f;
Effect { id: Default::default(), ty: PhantomData }
}
}
}
/// A handle to an effect, can be used to explicitly dispose of the effect.