Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: give a warning when you try to .dispatch() an action immediately (closes #2225) #2286

Merged
merged 2 commits into from
Feb 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading