From bbc7799b7c68ddc8d65e701f50d23467d5f8e87e Mon Sep 17 00:00:00 2001 From: Lukas Potthast Date: Wed, 21 Jun 2023 16:13:18 +0200 Subject: [PATCH] fix: memo with_untracked (#1213) --- CODE_OF_CONDUCT.md | 2 +- CONTRIBUTING.md | 4 ++-- README.md | 1 - leptos_reactive/src/memo.rs | 39 +++++++++++++++++++++++++++-------- leptos_reactive/tests/memo.rs | 26 +++++++++++++++++++++++ 5 files changed, 59 insertions(+), 13 deletions(-) diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md index 1fc4b8556..91d8f3280 100644 --- a/CODE_OF_CONDUCT.md +++ b/CODE_OF_CONDUCT.md @@ -2,7 +2,7 @@ _This Code of Conduct is based on the [Rust Code of Conduct](https://www.rust-lang.org/policies/code-of-conduct) and the [Bevy Code of Conduct](https://raw.githubusercontent.com/bevyengine/bevy/main/CODE_OF_CONDUCT.md), -which are adapted from the [Node.js Policy on Trolling](http://blog.izs.me/post/30036893703/policy-on-trolling) +which are adapted from the [Node.js Policy on Trolling](http://blog.izs.me/post/30036893703/policy-on-trolling) and the [Contributor Covenant](https://www.contributor-covenant.org)._ ## Our Pledge diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0d7672669..3cca1b01c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -42,7 +42,7 @@ Leptos, as a framework, reflects certain technical values: - **Embrace Rust semantics.** Especially in things like UI templating, use Rust semantics or extend them in a predictable way with control-flow components rather than overloading the meaning of Rust terms like `if` or `for` in a - framework-speciic way. + framework-specific way. - **Enhance ergonomics without obfuscating what’s happening.** This is by far the hardest to achieve. It’s often the case that adding additional layers to improve DX (like a custom build tool and starter templates) comes across as @@ -67,7 +67,7 @@ are a few guidelines that will make it a better experience for everyone: - Our CI tests every PR against all the existing examples, sometimes requiring compilation for both server and client side, etc. It’s thorough but slow. If you want to run CI locally to reduce frustration, you can do that by installing - `cargo-make` and using `cargo make check && cargo make test && cargo make + `cargo-make` and using `cargo make check && cargo make test && cargo make check-examples`. ## Architecture diff --git a/README.md b/README.md index 1412457e7..d6f382154 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,6 @@ [![Discord](https://img.shields.io/discord/1031524867910148188?color=%237289DA&label=discord)](https://discord.gg/YdRAhS7eQB) [![Matrix](https://img.shields.io/badge/Matrix-leptos-grey?logo=matrix&labelColor=white&logoColor=black)](https://matrix.to/#/#leptos:matrix.org) - [Website](https://leptos.dev) | [Book](https://leptos-rs.github.io/leptos/) | [Docs.rs](https://docs.rs/leptos/latest/leptos/) | [Playground](https://codesandbox.io/p/sandbox/leptos-rtfggt?file=%2Fsrc%2Fmain.rs%3A1%2C1) | [Discord](https://discord.gg/YdRAhS7eQB) # Leptos diff --git a/leptos_reactive/src/memo.rs b/leptos_reactive/src/memo.rs index c90576298..8b957f68e 100644 --- a/leptos_reactive/src/memo.rs +++ b/leptos_reactive/src/memo.rs @@ -7,6 +7,16 @@ use crate::{ }; use std::{any::Any, cell::RefCell, fmt, marker::PhantomData, rc::Rc}; +// IMPLEMENTATION NOTE: +// Memos are implemented "lazily," i.e., the inner computation is not run +// when the memo is created or when its value is marked as stale, but on demand +// when it is accessed, if the value is stale. This means that the value is stored +// internally as Option, even though it can always be accessed by the user as T. +// This means the inner value can be unwrapped in circumstances in which we know +// `Runtime::update_if_necessary()` has already been called, e.g., in the +// `.try_with_no_subscription()` calls below that are unwrapped with +// `.expect("invariant: must have already been initialized")`. + /// Creates an efficient derived reactive value based on other reactive values. /// /// Unlike a "derived signal," a memo comes with two guarantees: @@ -199,6 +209,17 @@ impl PartialEq for Memo { } } +fn forward_ref_to O>( + f: F, +) -> impl FnOnce(&Option) -> O { + |maybe_value: &Option| { + let ref_t = maybe_value + .as_ref() + .expect("invariant: must have already been initialized"); + f(ref_t) + } +} + impl SignalGetUntracked for Memo { #[cfg_attr( any(debug_assertions, feature = "ssr"), @@ -215,7 +236,11 @@ impl SignalGetUntracked for Memo { )] fn get_untracked(&self) -> T { with_runtime(self.runtime, move |runtime| { - let f = move |maybe_value: &Option| maybe_value.clone().unwrap(); + let f = |maybe_value: &Option| { + maybe_value + .clone() + .expect("invariant: must have already been initialized") + }; match self.id.try_with_no_subscription(runtime, f) { Ok(t) => t, Err(_) => panic_getting_dead_memo( @@ -261,10 +286,8 @@ impl SignalWithUntracked for Memo { ) )] fn with_untracked(&self, f: impl FnOnce(&T) -> O) -> O { - // Unwrapping here is fine for the same reasons as ::get_untracked with_runtime(self.runtime, |runtime| { - match self.id.try_with_no_subscription(runtime, |v: &T| f(v)) { + match self.id.try_with_no_subscription(runtime, forward_ref_to(f)) { Ok(t) => t, Err(_) => panic_getting_dead_memo( #[cfg(any(debug_assertions, feature = "ssr"))] @@ -394,15 +417,13 @@ impl SignalWith for Memo { )] #[track_caller] fn try_with(&self, f: impl FnOnce(&T) -> O) -> Option { - // memo is stored as Option, but will always have T available - // after latest_value() called, so we can unwrap safely - let f = move |maybe_value: &Option| f(maybe_value.as_ref().unwrap()); - let diagnostics = diagnostics!(self); with_runtime(self.runtime, |runtime| { self.id.subscribe(runtime, diagnostics); - self.id.try_with_no_subscription(runtime, f).ok() + self.id + .try_with_no_subscription(runtime, forward_ref_to(f)) + .ok() }) .ok() .flatten() diff --git a/leptos_reactive/tests/memo.rs b/leptos_reactive/tests/memo.rs index ddb35677b..c597e4453 100644 --- a/leptos_reactive/tests/memo.rs +++ b/leptos_reactive/tests/memo.rs @@ -13,6 +13,32 @@ fn basic_memo() { .dispose() } +#[cfg(not(feature = "stable"))] +#[test] +fn signal_with_untracked() { + use leptos_reactive::SignalWithUntracked; + + create_scope(create_runtime(), |cx| { + let m = create_memo(cx, move |_| 5); + let copied_out = m.with_untracked(|value| *value); + assert_eq!(copied_out, 5); + }) + .dispose() +} + +#[cfg(not(feature = "stable"))] +#[test] +fn signal_get_untracked() { + use leptos_reactive::SignalGetUntracked; + + create_scope(create_runtime(), |cx| { + let m = create_memo(cx, move |_| "memo".to_owned()); + let cloned_out = m.get_untracked(); + assert_eq!(cloned_out, "memo".to_owned()); + }) + .dispose() +} + #[cfg(not(feature = "stable"))] #[test] fn memo_with_computed_value() {