Skip to content

Commit

Permalink
docs: give a warning when you try to .dispatch() an action immediat…
Browse files Browse the repository at this point in the history
…ely (closes #2225) (#2286)
  • Loading branch information
gbj authored Feb 10, 2024
1 parent 8a77691 commit dfddbd6
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 8 deletions.
5 changes: 3 additions & 2 deletions leptos_reactive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ use runtime::*;
pub use runtime::{
as_child_of_current_owner, batch, create_runtime, current_runtime,
on_cleanup, run_as_child, set_current_runtime,
spawn_local_with_current_owner, spawn_local_with_owner,
spawn_local_with_current_owner, spawn_local_with_owner, try_batch,
try_spawn_local_with_current_owner, try_spawn_local_with_owner,
try_with_owner, untrack, untrack_with_diagnostics, with_current_owner,
with_owner, Owner, RuntimeId, ScopedFuture,
Expand All @@ -143,7 +143,8 @@ pub use suspense::{GlobalSuspenseContext, SuspenseContext};
pub use trigger::*;
pub use watch::*;

pub(crate) fn console_warn(s: &str) {
#[doc(hidden)]
pub fn console_warn(s: &str) {
cfg_if::cfg_if! {
if #[cfg(all(target_arch = "wasm32", any(feature = "csr", feature = "hydrate")))] {
web_sys::console::warn_1(&wasm_bindgen::JsValue::from_str(s));
Expand Down
19 changes: 18 additions & 1 deletion leptos_reactive/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1397,12 +1397,30 @@ impl Drop for SetObserverOnDrop {
///
/// # Panics
/// Panics if the runtime has already been disposed.
///
/// To avoid panicking under any circumstances, use [`try_batch`].
#[cfg_attr(
any(debug_assertions, features = "ssr"),
instrument(level = "trace", skip_all,)
)]
#[inline(always)]
pub fn batch<T>(f: impl FnOnce() -> T) -> T {
try_batch(f).expect(
"tried to run a batched update in a runtime that has been disposed",
)
}

/// Attempts to batch any reactive updates, preventing effects from running until the whole
/// function has run. This allows you to prevent rerunning effects if multiple
/// signal updates might cause the same effect to run.
///
/// Unlike [`batch`], this will not panic if the runtime has been disposed.
#[cfg_attr(
any(debug_assertions, features = "ssr"),
instrument(level = "trace", skip_all,)
)]
#[inline(always)]
pub fn try_batch<T>(f: impl FnOnce() -> T) -> Result<T, ReactiveSystemError> {
with_runtime(move |runtime| {
let batching = SetBatchingOnDrop(runtime.batching.get());
runtime.batching.set(true);
Expand All @@ -1415,7 +1433,6 @@ pub fn batch<T>(f: impl FnOnce() -> T) -> T {
runtime.run_effects();
val
})
.expect("tried to run a batched update in a runtime that has been disposed")
}

struct SetBatchingOnDrop(bool);
Expand Down
39 changes: 34 additions & 5 deletions leptos_server/src/action.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
//use crate::{ServerFn, ServerFnError};
#[cfg(debug_assertions)]
use leptos_reactive::console_warn;
use leptos_reactive::{
batch, create_rw_signal, is_suppressing_resource_load, signal_prelude::*,
spawn_local, store_value, use_context, ReadSignal, RwSignal, StoredValue,
create_rw_signal, is_suppressing_resource_load, signal_prelude::*,
spawn_local, store_value, try_batch, use_context, ReadSignal, RwSignal,
StoredValue,
};
use server_fn::{error::ServerFnUrlError, ServerFn, ServerFnError};
use std::{cell::Cell, future::Future, pin::Pin, rc::Rc};
Expand Down Expand Up @@ -93,8 +96,18 @@ where
any(debug_assertions, feature = "ssr"),
tracing::instrument(level = "trace", skip_all,)
)]
#[track_caller]
pub fn dispatch(&self, input: I) {
self.0.with_value(|a| a.dispatch(input))
#[cfg(debug_assertions)]
let loc = std::panic::Location::caller();

self.0.with_value(|a| {
a.dispatch(
input,
#[cfg(debug_assertions)]
loc,
)
})
}

/// Create an [Action].
Expand Down Expand Up @@ -366,7 +379,11 @@ where
any(debug_assertions, feature = "ssr"),
tracing::instrument(level = "trace", skip_all,)
)]
pub fn dispatch(&self, input: I) {
pub fn dispatch(
&self,
input: I,
#[cfg(debug_assertions)] loc: &'static std::panic::Location<'static>,
) {
if !is_suppressing_resource_load() {
let fut = (self.action_fn)(&input);
self.input.set(Some(input));
Expand All @@ -379,7 +396,7 @@ where
pending_dispatches.set(pending_dispatches.get().saturating_sub(1));
spawn_local(async move {
let new_value = fut.await;
batch(move || {
let res = try_batch(move || {
value.set(Some(new_value));
input.set(None);
version.update(|n| *n += 1);
Expand All @@ -389,6 +406,18 @@ where
pending.set(false);
}
});

if res.is_err() {
#[cfg(debug_assertions)]
console_warn(&format!(
"At {loc}, you are dispatching an action in a runtime \
that has already been disposed. This may be because \
you are calling `.dispatch()` in the body of a \
component, during initial server-side rendering. If \
that's the case, you should probably be using \
`create_resource` instead of `create_action`."
));
}
})
}
}
Expand Down

0 comments on commit dfddbd6

Please sign in to comment.