diff --git a/crates/vertigo-cli/src/build/wasm_opt.rs b/crates/vertigo-cli/src/build/wasm_opt.rs index 6ae9941d..c4c7122c 100644 --- a/crates/vertigo-cli/src/build/wasm_opt.rs +++ b/crates/vertigo-cli/src/build/wasm_opt.rs @@ -1,12 +1,37 @@ -use std::{fs, process::Command}; +use std::{fs, process::Command, sync::OnceLock}; use super::wasm_path::WasmPath; +/// Features that rustc enables by default for the `wasm32-unknown-unknown` target. +/// +/// Normally `wasm-opt` picks these up from the `target_features` custom section emitted +/// by LLVM, but that section is dropped whenever the binary is stripped +/// (`strip = true` in the cargo profile, which is a very common setting for wasm builds). +/// Without it `wasm-opt` falls back to its own default feature set and refuses to validate +/// the module, e.g.: +/// +/// ```text +/// [wasm-validator error in function 1765] unexpected false: memory.copy operations +/// require bulk memory operations [--enable-bulk-memory-opt] +/// ``` +/// +/// So we always pass them explicitly. +const WASM_FEATURES: &[&str] = &[ + "--enable-bulk-memory", + "--enable-bulk-memory-opt", + "--enable-nontrapping-float-to-int", + "--enable-sign-ext", + "--enable-mutable-globals", + "--enable-reference-types", + "--enable-multivalue", +]; + pub fn run_wasm_opt(from: &WasmPath, to: &WasmPath) -> bool { let from_str = from.as_string(); let to_str = to.as_string(); let mut wasm_opt_command = Command::new("wasm-opt"); + wasm_opt_command.args(supported_features()); wasm_opt_command.args(["-Os", "--strip-debug", "-o", &to_str, &from_str]); log::info!("Running: {wasm_opt_command:?}"); @@ -43,6 +68,34 @@ pub fn run_wasm_opt(from: &WasmPath, to: &WasmPath) -> bool { } } +/// Subset of [`WASM_FEATURES`] understood by the installed `wasm-opt`. +/// +/// Older Binaryen releases reject unknown options, so flags missing from `--help` +/// are filtered out instead of failing the whole optimization step. +fn supported_features() -> &'static Vec<&'static str> { + static FEATURES: OnceLock> = OnceLock::new(); + + FEATURES.get_or_init(|| { + let help = Command::new("wasm-opt").arg("--help").output(); + + let help = match help { + Ok(output) => { + let mut help = String::from_utf8_lossy(&output.stdout).into_owned(); + help.push_str(&String::from_utf8_lossy(&output.stderr)); + help + } + // wasm-opt is missing or unusable - run_wasm_opt will report it + Err(_) => return Vec::new(), + }; + + WASM_FEATURES + .iter() + .copied() + .filter(|feature| help.contains(feature)) + .collect() + }) +} + fn size(path: &str) -> u64 { fs::metadata(path) .map(|md| md.len() / 1_024) diff --git a/crates/vertigo-macro/src/wasm_path.rs b/crates/vertigo-macro/src/wasm_path.rs index c4546e3d..f8544255 100644 --- a/crates/vertigo-macro/src/wasm_path.rs +++ b/crates/vertigo-macro/src/wasm_path.rs @@ -47,8 +47,10 @@ impl WasmPath { use std::fs::File; use std::io::prelude::*; - let mut f = File::create(&self.path)?; - f.write_all(content)?; + let mut f = File::create(&self.path) + .map_err(|err| format!("Can't create {}: {err}", self.as_string()))?; + f.write_all(content) + .map_err(|err| format!("Can't write to {}: {err}", self.as_string()))?; Ok(()) } diff --git a/crates/vertigo/build.rs b/crates/vertigo/build.rs index 5d1b2cd0..846cc038 100644 --- a/crates/vertigo/build.rs +++ b/crates/vertigo/build.rs @@ -1,10 +1,11 @@ use std::env; use std::error::Error; +use std::ffi::OsStr; use std::fs; use std::path::{Path, PathBuf}; fn main() -> Result<(), Box> { - let target_dir = PathBuf::from(env::var("OUT_DIR")?).join("../../.."); + let target_dir = find_target_dir()?; let _ = fs::remove_dir_all(target_dir.join("tailwind")); @@ -42,6 +43,18 @@ fn main() -> Result<(), Box> { Ok(()) } +fn find_target_dir() -> Result> { + let out_dir = PathBuf::from(env::var("OUT_DIR")?); + + let target_dir = out_dir + .ancestors() + .find(|dir| dir.file_name() == Some(OsStr::new("build"))) + .and_then(Path::parent) + .ok_or_else(|| format!("Can't find target dir in OUT_DIR: {}", out_dir.display()))?; + + Ok(target_dir.to_path_buf()) +} + fn bundle_file( in_path: &str, content: &str, diff --git a/crates/vertigo/docs/collection-key-and-list-renderers.md b/crates/vertigo/docs/collection-key-and-list-renderers.md new file mode 100644 index 00000000..0da7e746 --- /dev/null +++ b/crates/vertigo/docs/collection-key-and-list-renderers.md @@ -0,0 +1,163 @@ +# `CollectionKey` and the memoized list renderers + +This document explains a small cluster of related building blocks in Vertigo and +how they fit together: + +- [`CollectionKey`](#collectionkey) — a marker trait describing how to identify + items in a list (the key) and what the item type is. +- [`render_list_memo`](#render_list_memo) and + [`render_resource_list_memo`](#render_resource_list_memo) — high-level helpers + that render reactive lists while **memoizing each item**, so only items that + actually changed are re-rendered. + +Per-item [`Computed`](crate::Computed)s come from +[`keyed_computed_list`](crate::keyed_computed_list). + +```text + Source of truth keyed_computed_list Per-item reactive view Renderer + ┌──────────────────┐ ┌────────────────────┐ ┌───────────────────────────┐ ┌──────────────────────────┐ + │ Value>>│ │ keyed_computed_list│ │ Vec │ │ render_list_memo │ + │ or │ ──────▶ │ │──▶ │ each item = Computed │──▶│ / │ + │ LazyCache>│ graph │ │ │ │ │ render_resource_list_memo│ + └──────────────────┘ └────────────────────┘ └───────────────────────────┘ └──────────────────────────┘ +``` + +--- + +## `CollectionKey` + +```rust,ignore +pub trait CollectionKey { + type Key: Eq + Hash + Clone + std::fmt::Debug + 'static; + type Value: Clone + PartialEq + 'static; + fn get_key(val: &Self::Value) -> Self::Key; +} +``` + +[`CollectionKey`](crate::CollectionKey) is a **marker / descriptor trait**. You implement it on a +zero-sized marker type (not on the item itself), and it declares three things: + +- `Value` — the item type stored in the list. +- `Key` — a stable identity for an item (e.g. a database id). +- `get_key` — how to extract the key from an item. + +Typical implementation: + +```rust,ignore +#[derive(Clone, PartialEq, Eq, Debug)] +pub struct Item { pub id: u32, pub name: String } + +pub struct ItemKey; // marker type + +impl CollectionKey for ItemKey { + type Key = u32; + type Value = Item; + fn get_key(val: &Item) -> u32 { val.id } +} +``` + +The marker type (`ItemKey`) is the generic parameter `T` threaded through +[`LazyListCache`](crate::LazyListCache) and the memoized list renderers. Keying matters for two reasons: + +1. **Memoization** — items keep the *same* per-item `Computed` across + updates as long as their key is stable. Only items whose content actually + changed cause a re-render. +2. **Deduplication** — [`keyed_computed_list`](crate::keyed_computed_list) logs an error and skips + items with a duplicate key within a single list. + +--- + +## `render_list_memo` + +```rust,ignore +pub fn render_list_memo( + value: &Value>>, + render: impl Fn(&Computed) -> DomNode + 'static, +) -> DomNode +``` + +Renders a reactive list from a `Value>>`, memoizing each item. + +Internally it maps the source to `Computed>` and calls +[`render_list`](crate::render::render_list), which runs +[`keyed_computed_list`](crate::keyed_computed_list) so each key keeps a stable +per-item [`Computed`](crate::Computed). + +Because the render closure receives a `&`[`Computed`](crate::Computed)`` (not a bare value), +the rendered subtree for an item re-runs only when that item changes — not when +sibling items or the list order change. + +Use this when **your list already lives in a [`Value`](crate::Value)** that you own. + +--- + +## `render_resource_list_memo` + +```rust,ignore +pub fn render_resource_list_memo( + value: &LazyCache>, + render: impl Fn(&Computed) -> DomNode + 'static, +) -> DomNode +``` + +Identical in shape to [`render_list_memo`](crate::render::render_list_memo), but the source is a +[`LazyCache`](crate::LazyCache)`>` — a lazily-loaded, possibly remote resource that +auto-refreshes on a TTL. Loading / Error states are normalized to an empty list, +then the cache is fed through [`keyed_computed_list`](crate::keyed_computed_list) +exactly like `render_list_memo`. + +Use this when **your list comes from a fetched resource** and you still want +per-item memoization. + +> Related: [`LazyListCache`](crate::LazyListCache) is a higher-level wrapper (also keyed by +> [`CollectionKey`](crate::CollectionKey)) that adds optimistic create/update/delete and per-item +> fetching on top of a list resource. `render_resource_list_memo` is the +> lower-level renderer for a plain `LazyCache>`. See the +> [`LazyListCache` guide](crate::guides::lazy_list_cache). + +--- + +## End-to-end example + +```rust,ignore +use std::rc::Rc; +use vertigo::{dom, CollectionKey, Computed, DomNode, Value}; +use vertigo::render::render_list_memo; // path-dependent; see "Public surface" + +#[derive(Clone, PartialEq, Eq, Debug)] +struct Item { id: u32, name: String } + +struct ItemKey; +impl CollectionKey for ItemKey { + type Key = u32; + type Value = Item; + fn get_key(v: &Item) -> u32 { v.id } +} + +fn view(items: &Value>>) -> DomNode { + render_list_memo::(items, |item: &Computed| { + let item = item.clone(); + item.render_value(|it| dom! {
{it.name}
}) + }) +} +``` + +When `items` is updated: + +- items with unchanged content are **not** re-rendered (their inner `Computed` + did not change), +- only added / removed / reordered / mutated items cause DOM work. + +--- + +## Public surface (what you can use directly) + +| Item | Exported as | +| ----------------------------- | ---------------------------------------- | +| [`CollectionKey`](crate::CollectionKey) | `vertigo::CollectionKey` | +| [`render_list_memo`](crate::render::render_list_memo) | `vertigo::render::render_list_memo` | +| [`render_resource_list_memo`](crate::render::render_resource_list_memo) | `vertigo::render::render_resource_list_memo` | + +[`render_list_memo`](crate::render::render_list_memo) / [`render_resource_list_memo`](crate::render::render_resource_list_memo) are reachable through the public +`render` module (`vertigo::render::…`); they are not re-exported at the crate +root the way [`CollectionKey`](crate::CollectionKey) is. diff --git a/crates/vertigo/docs/lazy-list-cache.md b/crates/vertigo/docs/lazy-list-cache.md index 2a9abbb4..77e6b436 100644 --- a/crates/vertigo/docs/lazy-list-cache.md +++ b/crates/vertigo/docs/lazy-list-cache.md @@ -25,7 +25,7 @@ where you don't need per-item granularity, prefer `T` is a **marker type** implementing [`CollectionKey`](crate::CollectionKey). It declares the item type (`T::Value`) and how to derive a stable key from it (`T::Key`, via `T::get_key`). See the -[companion guide](crate::guides::value_synchronize_and_collections) for the +[companion guide](crate::guides::collection_key_and_list_renderers) for the full rationale. Internally the cache holds, per visible item, a `ListItem` with **two** diff --git a/crates/vertigo/docs/value-synchronize-and-collections.md b/crates/vertigo/docs/value-synchronize-and-collections.md deleted file mode 100644 index caf4f5a2..00000000 --- a/crates/vertigo/docs/value-synchronize-and-collections.md +++ /dev/null @@ -1,266 +0,0 @@ -# `Value::synchronize`, `ValueSynchronize`, `CollectionKey`, and the memoized list renderers - -This document explains a small cluster of related building blocks in Vertigo and -how they fit together: - -- [`Value::synchronize`](#valuesynchronize) — the generic "mirror this reactive - source into a derived structure" mechanism. -- [`ValueSynchronize`](#valuesynchronize-trait) — the trait that a derived - structure implements to become a valid synchronization *target*. -- [`CollectionKey`](#collectionkey) — a marker trait describing how to identify - items in a list (the key) and what the item type is. -- [`render_list_memo`](#render_list_memo) and - [`render_resource_list_memo`](#render_resource_list_memo) — high-level helpers - that render reactive lists while **memoizing each item**, so only items that - actually changed are re-rendered. - -The thread connecting all of them is `Collection`: an internal structure that -turns a flat `Vec` into a list of *stable, per-item* [`Computed`](crate::Computed)s keyed by -[`CollectionKey`](crate::CollectionKey). It is created and kept up to date through `synchronize`, and it -is the data source that the memoized list renderers consume. - -```text - Source of truth synchronize() Per-item reactive view Renderer - ┌──────────────────┐ ┌────────────────────┐ ┌───────────────────────────┐ ┌──────────────────────────┐ - │ Value>>│ │ ValueSynchronize │ │ Collection │ │ render_list_memo │ - │ or │ ──────▶ │ (Collection │──▶ │ Vec> │──▶│ / │ - │ LazyCache>│ event │ is the target) │ │ each item = Computed │ │ render_resource_list_memo│ - └──────────────────┘ └────────────────────┘ └───────────────────────────┘ └──────────────────────────┘ -``` - ---- - -## `Value::synchronize` - -[`Value`](crate::Value) is the basic reactive cell. [`synchronize`](crate::Value::synchronize) lets you create a derived, -self-updating object `R` that mirrors the value and keeps following every change: - -```rust,ignore -pub fn synchronize + Clone + 'static>(&self) - -> (R, DropResource) -``` - -What it does: - -1. Reads the current value of the `Value`. -2. Constructs the target `R` from it via `R::new(init_value)`. -3. Subscribes (`add_event`) so every subsequent `set` on the `Value` is pushed - into the target via `R::set(...)`. -4. Returns the target plus a [`DropResource`](crate::DropResource). **The subscription lives only as - long as that `DropResource` is held** — drop it and the mirroring stops. - -The same pattern exists on the resource-fetching types: - -- [`LazyCache::synchronize`](crate::LazyCache::synchronize) mirrors the cache's current `Resource>` into - a target of type [`ValueSynchronize`](crate::ValueSynchronize)`>`. Loading / Error / Uninitialized - states are normalized to `T::default()` (hence the extra `T: Default + Clone` - bound), so the target always sees a concrete `Rc`. -- Internally this is implemented by `CacheValue::synchronize`, which performs - the same normalize-and-subscribe dance. - -So `synchronize` is *one* mechanism with several entry points ([`Value`](crate::Value), -[`LazyCache`](crate::LazyCache), `CacheValue`), all parameterized over the target type `R`. - ---- - -## `ValueSynchronize` trait - -```rust,ignore -pub trait ValueSynchronize: Sized { - fn new(value: T) -> Self; - fn set(&self, value: T); -} -``` - -This is the contract a type must satisfy to be a valid [`synchronize`](crate::Value::synchronize) *target*: - -- `new(value)` — build the target from the source's initial value. -- `set(value)` — accept each subsequent update. - -Note `set` takes `&self` (not `&mut self`): synchronization targets are expected -to use interior reactivity ([`Value`](crate::Value), `Rc`, etc.) internally, so a shared handle -can absorb updates. That is exactly why targets must also be `Clone` — the -closure registered with `add_event` captures a clone of the target. - -The only target implemented in-tree today is `Collection`, but the trait is -public so you can write your own (e.g. a target that maintains an index, a -running total, or a sorted view). - ---- - -## `CollectionKey` - -```rust,ignore -pub trait CollectionKey { - type Key: Eq + Hash + Clone + std::fmt::Debug + 'static; - type Value: Clone + PartialEq + 'static; - fn get_key(val: &Self::Value) -> Self::Key; -} -``` - -[`CollectionKey`](crate::CollectionKey) is a **marker / descriptor trait**. You implement it on a -zero-sized marker type (not on the item itself), and it declares three things: - -- `Value` — the item type stored in the list. -- `Key` — a stable identity for an item (e.g. a database id). -- `get_key` — how to extract the key from an item. - -Typical implementation: - -```rust,ignore -#[derive(Clone, PartialEq, Eq, Debug)] -pub struct Item { pub id: u32, pub name: String } - -pub struct ItemKey; // marker type - -impl CollectionKey for ItemKey { - type Key = u32; - type Value = Item; - fn get_key(val: &Item) -> u32 { val.id } -} -``` - -The marker type (`ItemKey`) is the generic parameter `T` threaded through -`Collection`, `CollectionModel`, [`LazyListCache`](crate::LazyListCache), and the memoized list -renderers. Keying matters for two reasons: - -1. **Memoization** — items keep the *same* per-item `Value`/`Computed` across - updates as long as their key is stable. Only items whose content actually - changed cause a re-render. -2. **Deduplication** — `Collection` logs an error and skips items with a - duplicate key within a single list. - ---- - -## How `Collection` ties it together - -`Collection` is the internal structure produced by `synchronize`. It is *not* -exported at the crate root, but understanding it explains the whole flow: - -- It holds an `ItemDataCollection` — a `HashMap, Computed)>` - — plus an `order: Value>>` describing the current list - order. -- On `set(new_list)` it: - - extracts the key of each item, - - reuses the existing per-item `Value` if the key is already known (calling - `value.set(item)`, which only triggers downstream work when the item is - actually `!=` the previous one), - - creates a fresh per-item `Value`/`Computed` for new keys, - - updates `order`, - - retains only the keys still present (dropping per-item state for removed - items). -- It implements `ValueSynchronize>>`, which is what makes it a - legal `synchronize` target. - -`Collection::get()` returns `Computed>>`, where each -`CollectionModel` is `{ key: T::Key, model: Computed }`. The -*outer* computed changes when the list's order/membership changes; each *inner* -`model` computed changes only when that specific item changes. This two-level -reactivity is the source of the memoization. - ---- - -## `render_list_memo` - -```rust,ignore -pub fn render_list_memo( - value: &Value>>, - render: impl Fn(&Computed) -> DomNode + 'static, -) -> DomNode -``` - -Renders a reactive list from a `Value>>`, memoizing each item. - -Internally it: - -1. calls `value.synchronize::>()` to obtain the keyed collection - and its `DropResource`, -2. feeds `collection.get()` into the lower-level `render_list`, using each - item's `key` for identity and rendering each item from its **own per-item - `Computed`**, -3. attaches the synchronization `DropResource` (and a handle on the source - `value`) to the resulting node so everything is cleaned up when the node is - dropped. - -Because the render closure receives a `&`[`Computed`](crate::Computed)`` (not a bare value), -the rendered subtree for an item re-runs only when that item changes — not when -sibling items or the list order change. - -Use this when **your list already lives in a [`Value`](crate::Value)** that you own. - ---- - -## `render_resource_list_memo` - -```rust,ignore -pub fn render_resource_list_memo( - value: &LazyCache>, - render: impl Fn(&Computed) -> DomNode + 'static, -) -> DomNode -``` - -Identical in shape to [`render_list_memo`](crate::render::render_list_memo), but the source is a -[`LazyCache`](crate::LazyCache)`>` — a lazily-loaded, possibly remote resource that -auto-refreshes on a TTL. It calls `value.synchronize::>()` on the -`LazyCache` (which normalizes Loading/Error to an empty/`default` list) and then -renders exactly like `render_list_memo`. - -Use this when **your list comes from a fetched resource** and you still want -per-item memoization. - -> Related: [`LazyListCache`](crate::LazyListCache) is a higher-level wrapper (also keyed by -> [`CollectionKey`](crate::CollectionKey)) that adds optimistic create/update/delete and per-item -> fetching on top of a list resource. `render_resource_list_memo` is the -> lower-level renderer for a plain `LazyCache>`. See the -> [`LazyListCache` guide](crate::guides::lazy_list_cache). - ---- - -## End-to-end example - -```rust,ignore -use std::rc::Rc; -use vertigo::{dom, CollectionKey, Computed, DomNode, Value}; -use vertigo::render::render_list_memo; // path-dependent; see "Public surface" - -#[derive(Clone, PartialEq, Eq, Debug)] -struct Item { id: u32, name: String } - -struct ItemKey; -impl CollectionKey for ItemKey { - type Key = u32; - type Value = Item; - fn get_key(v: &Item) -> u32 { v.id } -} - -fn view(items: &Value>>) -> DomNode { - render_list_memo::(items, |item: &Computed| { - let item = item.clone(); - item.render_value(|it| dom! {
{it.name}
}) - }) -} -``` - -When `items` is updated: - -- items with unchanged content are **not** re-rendered (their inner `Computed` - did not change), -- only added / removed / reordered / mutated items cause DOM work. - ---- - -## Public surface (what you can use directly) - -| Item | Exported as | -| ----------------------------- | ---------------------------------------- | -| [`Value::synchronize`](crate::Value::synchronize) | method on [`vertigo::Value`](crate::Value) | -| [`LazyCache::synchronize`](crate::LazyCache::synchronize) | method on [`vertigo::LazyCache`](crate::LazyCache) | -| [`ValueSynchronize`](crate::ValueSynchronize) | `vertigo::ValueSynchronize` | -| [`CollectionKey`](crate::CollectionKey) | `vertigo::CollectionKey` | -| [`render_list_memo`](crate::render::render_list_memo) | `vertigo::render::render_list_memo` | -| [`render_resource_list_memo`](crate::render::render_resource_list_memo) | `vertigo::render::render_resource_list_memo` | -| `Collection`, `CollectionModel` | `vertigo::render::collection::*` (lower-level) | - -[`render_list_memo`](crate::render::render_list_memo) / [`render_resource_list_memo`](crate::render::render_resource_list_memo) are reachable through the public -`render` module (`vertigo::render::…`); they are not re-exported at the crate -root the way [`CollectionKey`](crate::CollectionKey) is. diff --git a/crates/vertigo/docs/websocket-collection.md b/crates/vertigo/docs/websocket-collection.md index 2e79145e..3cb860c9 100644 --- a/crates/vertigo/docs/websocket-collection.md +++ b/crates/vertigo/docs/websocket-collection.md @@ -215,7 +215,7 @@ let rows: Computed> = collection.items_sorted.map(|o| o.unwrap_or_defa For long lists where only a few rows change at a time, pair the inner `Vec` with [`render_list_memo`](crate::render::render_list_memo) so unchanged rows are not re-rendered — see the -[value-synchronize & collections guide](crate::guides::value_synchronize_and_collections). +[keyed collections guide](crate::guides::collection_key_and_list_renderers). --- diff --git a/crates/vertigo/src/computed/dependencies/graph_one_to_many.rs b/crates/vertigo/src/computed/dependencies/graph_one_to_many.rs index c435126d..70b585c2 100644 --- a/crates/vertigo/src/computed/dependencies/graph_one_to_many.rs +++ b/crates/vertigo/src/computed/dependencies/graph_one_to_many.rs @@ -73,7 +73,7 @@ impl GraphOneToMany { pub fn all_connections_len(&self) -> u64 { let mut count: u64 = 0; - for (_, item) in self.data.iter() { + for item in self.data.values() { count += item.len() as u64; } diff --git a/crates/vertigo/src/computed/keyed_computed_list.rs b/crates/vertigo/src/computed/keyed_computed_list.rs new file mode 100644 index 00000000..77f746ac --- /dev/null +++ b/crates/vertigo/src/computed/keyed_computed_list.rs @@ -0,0 +1,189 @@ +use std::{ + collections::{HashMap, HashSet}, + hash::Hash, + rc::Rc, +}; + +use super::{Computed, ToComputed, struct_mut::ValueMut}; + +/// One entry in a [`keyed_computed_list`]: a stable key plus a per-item value. +/// +/// For [`keyed_computed_list`] itself, `V` is [`Computed`]. `PartialEq` then +/// compares the key and the identity of that `Computed` (not the item value), so +/// observers of the outer list ignore in-place item updates. +pub struct KeyedListItem { + pub key: K, + pub value: V, +} + +impl Clone for KeyedListItem { + fn clone(&self) -> Self { + KeyedListItem { + key: self.key.clone(), + value: self.value.clone(), + } + } +} + +impl PartialEq for KeyedListItem { + fn eq(&self, other: &Self) -> bool { + self.key == other.key && self.value == other.value + } +} + +/// Maps a reactive list into a reactive list of per-item [`Computed`]s, reusing the same +/// `Computed` instance for each key across updates (Solid ``-style). +/// +/// The **outer** computed changes when membership or order changes. Each **inner** +/// `Computed` changes only when that item's value changes (`T: PartialEq`). Duplicate +/// keys are logged and skipped (the first occurrence is kept). +/// +/// If a per-item `Computed` is read after its key has left the source list, the last +/// seen value is returned. Drop observers when a row unmounts. +/// +/// This is the `Computed`-to-`Computed` transform used by +/// [`render_list`](crate::render::render_list) / +/// [`render_list_memo`](crate::render::render_list_memo). +/// +/// ```rust +/// use vertigo::{keyed_computed_list, transaction, Value}; +/// +/// #[derive(Clone, PartialEq, Debug)] +/// struct Person { +/// id: u32, +/// name: String, +/// } +/// +/// let people = Value::new(vec![Person { +/// id: 1, +/// name: "Ann".into(), +/// }]); +/// +/// let rows = keyed_computed_list(people.to_computed(), |person| person.id); +/// +/// transaction(|ctx| { +/// let list = rows.get(ctx); +/// assert_eq!(list.len(), 1); +/// assert_eq!(list[0].key, 1); +/// assert_eq!(list[0].value.get(ctx).name, "Ann"); +/// }); +/// ``` +pub fn keyed_computed_list( + items: impl ToComputed>, + get_key: impl Fn(&T) -> K + 'static, +) -> Computed>>> +where + T: Clone + PartialEq + 'static, + K: Clone + Eq + Hash + std::fmt::Debug + 'static, +{ + let items = items.to_computed(); + let get_key = Rc::new(get_key); + + // Behind an `Rc` for the same reason as `hash` below: both readers would otherwise + // deep-copy the whole list on every update. + let unique_keyed_items = Computed::from({ + move |ctx| { + let mut result = Vec::new(); + let mut seen = HashSet::new(); + + for item in items.get(ctx) { + let key = get_key(&item); + + if seen.contains(&key) { + log::error!( + "keyed_computed_list: duplicate key {:?}; keeping the first occurrence", + key + ); + continue; + } + + seen.insert(key.clone()); + result.push((key, item)); + } + + Rc::new(result) + } + }); + + // Rows are looked up by key from here. A row only notifies when its own value + // changes, because `Computed` compares with `PartialEq` before notifying. + // + // Behind an `Rc` because every row reads this map, and `Computed::get` hands back a + // clone of the cached value - cloning the map itself would make one update cost + // `rows * rows` item clones. + let hash = Computed::from({ + let unique_keyed_items = unique_keyed_items.clone(); + move |ctx| { + Rc::new( + unique_keyed_items + .get(ctx) + .iter() + .map(|(key, item)| (key.clone(), item.clone())) + .collect::>(), + ) + } + }); + + let cache_computed = Rc::new(ValueMut::new(HashMap::>::new())); + let cache_list_items = Rc::new(ValueMut::new( + HashMap::>>::new(), + )); + + Computed::from({ + move |ctx| { + let unique_items = unique_keyed_items.get(ctx); + let mut result_list = Vec::with_capacity(unique_items.len()); + + for (key, item) in unique_items.iter() { + let next_computed = cache_computed.change(|cache| { + if let Some(prev) = cache.get(key) { + prev.clone() + } else { + let hash = hash.clone(); + let last = Rc::new(ValueMut::new(item.clone())); + let lookup_key = key.clone(); + Computed::from(move |ctx| match hash.get(ctx).get(&lookup_key) { + Some(val) => { + let val = val.clone(); + last.set(val.clone()); + val + } + None => { + log::error!( + "keyed_computed_list: item Computed for key {:?} was read after that key left the source list; returning last value", + lookup_key + ); + last.get() + } + }) + } + }); + + let list_item = cache_list_items.change(|cache| match cache.get(key) { + Some(prev) if prev.value == next_computed => prev.clone(), + _ => KeyedListItem { + key: key.clone(), + value: next_computed.clone(), + }, + }); + + result_list.push(list_item); + } + + cache_computed.set( + result_list + .iter() + .map(|item| (item.key.clone(), item.value.clone())) + .collect(), + ); + cache_list_items.set( + result_list + .iter() + .map(|item| (item.key.clone(), item.clone())) + .collect(), + ); + + result_list + } + }) +} diff --git a/crates/vertigo/src/computed/mod.rs b/crates/vertigo/src/computed/mod.rs index 10b96754..f60b209a 100644 --- a/crates/vertigo/src/computed/mod.rs +++ b/crates/vertigo/src/computed/mod.rs @@ -2,6 +2,7 @@ mod auto_map; mod computed_box; pub mod context; mod dependencies; +mod keyed_computed_list; pub use dependencies::{Dependencies, get_dependencies}; mod drop_resource; mod graph_id; @@ -20,9 +21,10 @@ pub use computed_box::Computed; pub use drop_resource::DropResource; pub use graph_id::GraphId; pub use graph_value::GraphValue; +pub use keyed_computed_list::{KeyedListItem, keyed_computed_list}; pub use reactive::Reactive; pub use to_computed::ToComputed; -pub use value::{Value, ValueSynchronize}; +pub use value::Value; /// Allows to create `Computed` out of `Value`, `Value`, ... /// diff --git a/crates/vertigo/src/computed/struct_mut/hash_map_mut.rs b/crates/vertigo/src/computed/struct_mut/hash_map_mut.rs index 944a3215..0cd9fc16 100644 --- a/crates/vertigo/src/computed/struct_mut/hash_map_mut.rs +++ b/crates/vertigo/src/computed/struct_mut/hash_map_mut.rs @@ -74,7 +74,7 @@ impl HashMapMut { pub fn filter_and_map(&self, map: fn(&V) -> Option) -> Vec { let state = self.data.get(); let mut list = Vec::new(); - for (_, value) in (*state).iter() { + for value in (*state).values() { if let Some(mapped) = map(value) { list.push(mapped); } @@ -104,7 +104,7 @@ impl HashMapMut { let mut out = Vec::new(); - for (_, callback) in state.iter() { + for callback in state.values() { out.push((*callback).clone()); } diff --git a/crates/vertigo/src/computed/tests/keyed_computed_list.rs b/crates/vertigo/src/computed/tests/keyed_computed_list.rs new file mode 100644 index 00000000..61de41ce --- /dev/null +++ b/crates/vertigo/src/computed/tests/keyed_computed_list.rs @@ -0,0 +1,701 @@ +use std::{ + cell::{Cell, RefCell}, + collections::HashMap, + rc::Rc, +}; + +use crate::{ + Computed, DropResource, KeyedListItem, Value, computed::struct_mut::ValueMut, + keyed_computed_list, transaction, +}; + +#[derive(Clone, PartialEq, Debug)] +struct Person { + id: &'static str, + name: &'static str, + age: i32, +} + +fn bob() -> Person { + Person { + id: "1", + name: "Bob", + age: 43, + } +} + +fn frank(age: i32) -> Person { + Person { + id: "2", + name: "Frank", + age, + } +} + +/// Like a JS `Signal.set(newArray)`: every assignment notifies, even when the +/// payload is structurally equal. `Value>` would swallow that case via `PartialEq`. +#[derive(Clone)] +struct SignalList(Vec); + +impl PartialEq for SignalList { + fn eq(&self, _other: &Self) -> bool { + false + } +} + +#[derive(Clone, Debug, PartialEq)] +struct CleanDumpItem { + id: &'static str, + name: &'static str, + age: i32, + revision: u32, +} + +#[derive(Clone, Debug, PartialEq)] +struct CleanDump { + list_revision: u32, + items: Vec, +} + +struct DumpItem { + id: &'static str, + name: RefCell<&'static str>, + age: RefCell, + revision: RefCell, + _unsubscribe: RefCell>, +} + +struct Dump { + list_revision: u32, + items: Vec>, +} + +/// Mirrors the TypeScript `autorun` dump: subscribe to the outer list, and for each +/// new key start a nested subscribe on that row's `Computed`. +fn watch_dump( + list: Computed>>>, +) -> (Rc>, DropResource) { + let dump = Rc::new(RefCell::new(Dump { + list_revision: 0, + items: Vec::new(), + })); + + let unsub = list.subscribe({ + let dump = dump.clone(); + move |rows| { + let mut dump = dump.borrow_mut(); + + let mut prev: HashMap<&'static str, Rc> = HashMap::new(); + for item in dump.items.drain(..) { + prev.insert(item.id, item); + } + + let mut new_items = Vec::new(); + for record in rows { + let id = record.key; + if let Some(prev_item) = prev.remove(&id) { + new_items.push(prev_item); + continue; + } + + let new_item = Rc::new(DumpItem { + id, + name: RefCell::new(""), + age: RefCell::new(0), + revision: RefCell::new(0), + _unsubscribe: RefCell::new(None), + }); + + let item_unsub = record.value.subscribe({ + let new_item = new_item.clone(); + move |person| { + *new_item.name.borrow_mut() = person.name; + *new_item.age.borrow_mut() = person.age; + *new_item.revision.borrow_mut() += 1; + } + }); + *new_item._unsubscribe.borrow_mut() = Some(item_unsub); + new_items.push(new_item); + } + + dump.items = new_items; + dump.list_revision += 1; + } + }); + + (dump, unsub) +} + +fn get_dump(dump: &RefCell) -> CleanDump { + let dump = dump.borrow(); + CleanDump { + list_revision: dump.list_revision, + items: dump + .items + .iter() + .map(|item| CleanDumpItem { + id: item.id, + name: *item.name.borrow(), + age: *item.age.borrow(), + revision: *item.revision.borrow(), + }) + .collect(), + } +} + +fn people_computed(source: &Value) -> Computed> { + let source = source.clone(); + Computed::from(move |ctx| source.get(ctx).0) +} + +#[test] +fn exposes_computed_values_for_the_initial_list() { + let source = Value::new(SignalList(Vec::new())); + let list = keyed_computed_list(people_computed(&source), |item| item.id); + + let (dump, _watch) = watch_dump(list); + + assert_eq!( + get_dump(&dump), + CleanDump { + list_revision: 1, + items: vec![], + } + ); + + source.set(SignalList(vec![bob()])); + assert_eq!( + get_dump(&dump), + CleanDump { + list_revision: 2, + items: vec![CleanDumpItem { + id: "1", + name: "Bob", + age: 43, + revision: 1, + }], + } + ); + + // New array, same content — source notifies, keyed list must not. + source.set(SignalList(vec![bob()])); + assert_eq!( + get_dump(&dump), + CleanDump { + list_revision: 2, + items: vec![CleanDumpItem { + id: "1", + name: "Bob", + age: 43, + revision: 1, + }], + } + ); + + source.set(SignalList(vec![bob(), frank(23)])); + assert_eq!( + get_dump(&dump), + CleanDump { + list_revision: 3, + items: vec![ + CleanDumpItem { + id: "1", + name: "Bob", + age: 43, + revision: 1, + }, + CleanDumpItem { + id: "2", + name: "Frank", + age: 23, + revision: 1, + }, + ], + } + ); + + source.set(SignalList(vec![bob(), frank(24)])); + assert_eq!( + get_dump(&dump), + CleanDump { + list_revision: 3, + items: vec![ + CleanDumpItem { + id: "1", + name: "Bob", + age: 43, + revision: 1, + }, + CleanDumpItem { + id: "2", + name: "Frank", + age: 24, + revision: 2, + }, + ], + } + ); + + source.set(SignalList(vec![frank(24)])); + assert_eq!( + get_dump(&dump), + CleanDump { + list_revision: 4, + items: vec![CleanDumpItem { + id: "2", + name: "Frank", + age: 24, + revision: 2, + }], + } + ); + + source.set(SignalList(vec![frank(30)])); + assert_eq!( + get_dump(&dump), + CleanDump { + list_revision: 4, + items: vec![CleanDumpItem { + id: "2", + name: "Frank", + age: 30, + revision: 3, + }], + } + ); + + source.set(SignalList(vec![frank(30)])); + assert_eq!( + get_dump(&dump), + CleanDump { + list_revision: 4, + items: vec![CleanDumpItem { + id: "2", + name: "Frank", + age: 30, + revision: 3, + }], + } + ); +} + +/// A row notifies only when its own value changes. Rewriting the list with equal +/// content, reordering it, or adding a key must leave the existing rows quiet. +#[test] +fn unchanged_rows_do_not_notify() { + fn zoe() -> Person { + Person { + id: "3", + name: "Zoe", + age: 30, + } + } + + let source = Value::new(SignalList(vec![bob(), frank(23)])); + let list = keyed_computed_list(people_computed(&source), |item| item.id); + + let (bob_row, frank_row) = transaction(|ctx| { + let rows = list.get(ctx); + (rows[0].value.clone(), rows[1].value.clone()) + }); + + let bob_calls = Rc::new(Cell::new(0)); + let frank_calls = Rc::new(Cell::new(0)); + + let _bob_sub = bob_row.subscribe({ + let bob_calls = bob_calls.clone(); + move |_| bob_calls.set(bob_calls.get() + 1) + }); + let _frank_sub = frank_row.subscribe({ + let frank_calls = frank_calls.clone(); + move |_| frank_calls.set(frank_calls.get() + 1) + }); + + // `subscribe` delivers the current value straight away. + assert_eq!((bob_calls.get(), frank_calls.get()), (1, 1), "initial read"); + + // A brand new list carrying structurally equal rows. + source.set(SignalList(vec![bob(), frank(23)])); + assert_eq!( + (bob_calls.get(), frank_calls.get()), + (1, 1), + "equal content" + ); + + // Same membership, different order. + source.set(SignalList(vec![frank(23), bob()])); + assert_eq!((bob_calls.get(), frank_calls.get()), (1, 1), "reorder"); + + // A key appears; the rows that were already there are untouched. + source.set(SignalList(vec![frank(23), bob(), zoe()])); + assert_eq!((bob_calls.get(), frank_calls.get()), (1, 1), "new key"); + + // Only the row that really changed notifies. + source.set(SignalList(vec![frank(24), bob(), zoe()])); + assert_eq!( + (bob_calls.get(), frank_calls.get()), + (1, 2), + "only Frank changed" + ); +} + +/// Building a keyed list allocates graph nodes. Doing that from a computed that re-runs +/// while the graph is being refreshed must work - a render closure reached during an update +/// is exactly that situation. +#[test] +fn can_be_built_during_a_refresh() { + let trigger = Value::new(1); + + let ages = Computed::from({ + let trigger = trigger.clone(); + move |ctx| { + let age = trigger.get(ctx); + let source = Value::new(vec![Person { + id: "1", + name: "Ann", + age, + }]); + + let list = keyed_computed_list(source.to_computed(), |item| item.id); + + list.get(ctx) + .iter() + .map(|row| row.value.get(ctx).age) + .collect::>() + } + }); + + let seen = Rc::new(RefCell::new(Vec::new())); + let _subscription = ages.subscribe({ + let seen = seen.clone(); + move |ages| seen.borrow_mut().push(ages) + }); + + assert_eq!(*seen.borrow(), vec![vec![1]]); + + // Re-runs the closure - and so builds a second keyed list - mid-refresh. + trigger.set(2); + + assert_eq!(*seen.borrow(), vec![vec![1], vec![2]]); +} + +/// The duplicate-key path logs and skips; make sure it does so safely mid-refresh too. +#[test] +fn duplicate_keys_during_a_refresh() { + let trigger = Value::new(1); + + let names = Computed::from({ + let trigger = trigger.clone(); + move |ctx| { + let age = trigger.get(ctx); + let source = Value::new(vec![ + Person { + id: "1", + name: "first", + age, + }, + Person { + id: "1", + name: "duplicate", + age, + }, + ]); + + let list = keyed_computed_list(source.to_computed(), |item| item.id); + + list.get(ctx) + .iter() + .map(|row| row.value.get(ctx).name) + .collect::>() + } + }); + + let seen = Rc::new(RefCell::new(Vec::new())); + let _subscription = names.subscribe({ + let seen = seen.clone(); + move |names| seen.borrow_mut().push(names) + }); + + trigger.set(2); + + assert_eq!(*seen.borrow(), vec![vec!["first"]]); +} + +/// Counts how often the item type is cloned, to pin down the cost of an update. +#[derive(Debug)] +struct Counted { + id: u32, + value: u32, + clones: Rc>, +} + +impl Clone for Counted { + fn clone(&self) -> Self { + self.clones.set(self.clones.get() + 1); + + Counted { + id: self.id, + value: self.value, + clones: self.clones.clone(), + } + } +} + +impl PartialEq for Counted { + fn eq(&self, other: &Self) -> bool { + self.id == other.id && self.value == other.value + } +} + +/// Build a list of `rows` rows, observe every row the way a rendered list does, then +/// change a single row and report how many item clones that update cost. +fn clones_for_one_row_update(rows: u32) -> usize { + let clones = Rc::new(Cell::new(0)); + + let build = |first_value: u32| { + (0..rows) + .map(|id| Counted { + id, + value: if id == 0 { first_value } else { id }, + clones: clones.clone(), + }) + .collect::>() + }; + + let source = Value::new(build(0)); + let list = keyed_computed_list(source.to_computed(), |item| item.id); + + // Observe the same way a rendered list does: the list itself, plus every row. + let _row_subscriptions = transaction(|ctx| list.get(ctx)) + .into_iter() + .map(|item| item.value.subscribe(|_| {})) + .collect::>(); + let _list_subscription = list.subscribe(|_| {}); + + clones.set(0); + source.set(build(1)); + clones.get() +} + +/// Changing one row must cost work proportional to the list, not to its square. +/// +/// Every row's `Computed` reads the shared key->value map, and `Computed::get` hands +/// back a clone of the cached value - so if that map is not behind an `Rc`, each of +/// the n rows copies all n items on every update. +#[test] +fn one_row_update_scales_linearly() { + let small = clones_for_one_row_update(20); + let large = clones_for_one_row_update(80); + + assert!( + large < small * 6, + "updating one row looks quadratic: 20 rows cost {small} clones, \ + 80 rows cost {large} (linear would be about 4x, quadratic about 16x)" + ); +} + +#[test] +fn keeps_the_first_item_when_duplicate_keys_appear() { + let source = Value::new(vec![ + Person { + id: "1", + name: "first", + age: 1, + }, + Person { + id: "1", + name: "second", + age: 2, + }, + Person { + id: "2", + name: "other", + age: 3, + }, + ]); + + let list = keyed_computed_list(source.to_computed(), |item| item.id); + + transaction(|ctx| { + let rows = list.get(ctx); + let values: Vec = rows.iter().map(|item| item.value.get(ctx)).collect(); + let keys: Vec<&'static str> = rows.iter().map(|item| item.key).collect(); + + assert_eq!( + values, + vec![ + Person { + id: "1", + name: "first", + age: 1, + }, + Person { + id: "2", + name: "other", + age: 3, + }, + ] + ); + assert_eq!(keys, vec!["1", "2"]); + }); +} + +/// Local copy of the TypeScript `mapKeyedListState` (not public API yet). +fn map_keyed_list_state( + list: Computed>>>, + create_state: impl Fn(Computed) -> S + 'static, +) -> Computed>> +where + T: Clone + 'static, + S: Clone + 'static, + K: Clone + Eq + std::hash::Hash + 'static, +{ + let cache = Rc::new(ValueMut::new(HashMap::>::new())); + + Computed::from(move |ctx| { + let mut result = Vec::new(); + + for item in list.get(ctx) { + let next = cache.change(|cache| { + if let Some(prev) = cache.get(&item.key) { + prev.clone() + } else { + KeyedListItem { + key: item.key.clone(), + value: create_state(item.value.clone()), + } + } + }); + result.push(next); + } + + cache.set( + result + .iter() + .map(|item| (item.key.clone(), item.clone())) + .collect(), + ); + result + }) +} + +#[test] +fn keyed_list_builds_a_keyed_computed_list() { + let source = Value::new(vec![Person { + id: "1", + name: "Ann", + age: 20, + }]); + + let list = keyed_computed_list(source.to_computed(), |item| item.id); + + transaction(|ctx| { + let rows: Vec<(&'static str, Person)> = list + .get(ctx) + .into_iter() + .map(|item| (item.key, item.value.get(ctx))) + .collect(); + + assert_eq!( + rows, + vec![( + "1", + Person { + id: "1", + name: "Ann", + age: 20, + } + )] + ); + }); +} + +#[derive(Clone)] +struct RowState { + label: Computed<&'static str>, +} + +impl PartialEq for RowState { + fn eq(&self, other: &Self) -> bool { + self.label == other.label + } +} + +#[test] +fn keyed_list_map_runs_create_state_once_per_key() { + let source = Value::new(vec![Person { + id: "1", + name: "Ann", + age: 20, + }]); + let create_count = Rc::new(std::cell::Cell::new(0)); + + let list = keyed_computed_list(source.to_computed(), |item| item.id); + let rows = map_keyed_list_state(list, { + let create_count = create_count.clone(); + move |person| { + create_count.set(create_count.get() + 1); + RowState { + label: Computed::from({ + let person = person.clone(); + move |ctx| person.get(ctx).name + }), + } + } + }); + + let first_label_id = transaction(|ctx| { + let rows = rows.get(ctx); + assert_eq!(create_count.get(), 1); + assert_eq!(rows[0].value.label.get(ctx), "Ann"); + rows[0].value.label.id() + }); + + source.set(vec![Person { + id: "1", + name: "Ann", + age: 21, + }]); + + transaction(|ctx| { + let current = rows.get(ctx); + assert_eq!(create_count.get(), 1); + assert_eq!(current[0].value.label.id(), first_label_id); + assert_eq!(current[0].value.label.get(ctx), "Ann"); + }); + + source.set(vec![ + Person { + id: "1", + name: "Ann", + age: 21, + }, + Person { + id: "2", + name: "Bob", + age: 30, + }, + ]); + + transaction(|ctx| { + let current = rows.get(ctx); + assert_eq!(current.len(), 2); + assert_eq!(create_count.get(), 2); + assert_eq!(current[0].value.label.id(), first_label_id); + }); +} + +#[test] +fn returns_last_value_after_key_leaves_the_list() { + let source = Value::new(vec![bob()]); + let list = keyed_computed_list(source.to_computed(), |item| item.id); + + let stale_item = transaction(|ctx| list.get(ctx)[0].value.clone()); + + source.set(Vec::new()); + + transaction(|ctx| { + assert_eq!(list.get(ctx).len(), 0); + assert_eq!(stale_item.get(ctx), bob()); + }); +} diff --git a/crates/vertigo/src/computed/tests/mod.rs b/crates/vertigo/src/computed/tests/mod.rs index 174b140d..2c1bab3c 100644 --- a/crates/vertigo/src/computed/tests/mod.rs +++ b/crates/vertigo/src/computed/tests/mod.rs @@ -1,4 +1,6 @@ pub mod app_state; pub mod box_value_version; pub mod computed; +pub mod keyed_computed_list; pub mod nested_reactivity; +pub mod value_copies; diff --git a/crates/vertigo/src/computed/tests/value_copies.rs b/crates/vertigo/src/computed/tests/value_copies.rs new file mode 100644 index 00000000..448ccfba --- /dev/null +++ b/crates/vertigo/src/computed/tests/value_copies.rs @@ -0,0 +1,74 @@ +//! What a `Value` write costs when nothing is listening to it. + +use std::{cell::Cell, rc::Rc}; + +use crate::Value; + +/// Counts how often an item is cloned. +#[derive(Debug)] +struct Counted { + value: u32, + clones: Rc>, +} + +impl Clone for Counted { + fn clone(&self) -> Self { + self.clones.set(self.clones.get() + 1); + + Counted { + value: self.value, + clones: self.clones.clone(), + } + } +} + +impl PartialEq for Counted { + fn eq(&self, other: &Self) -> bool { + self.value == other.value + } +} + +fn list(len: u32, offset: u32, clones: &Rc>) -> Vec { + (0..len) + .map(|value| Counted { + value: value + offset, + clones: clones.clone(), + }) + .collect() +} + +#[test] +fn new_takes_ownership_of_the_payload() { + let clones = Rc::new(Cell::new(0)); + + let _value = Value::new(list(10, 0, &clones)); + + assert_eq!(clones.get(), 0, "`Value::new` must not copy what it stores"); +} + +#[test] +fn set_without_listeners_does_not_copy_the_payload() { + let clones = Rc::new(Cell::new(0)); + let value = Value::new(list(10, 0, &clones)); + + clones.set(0); + value.set(list(10, 100, &clones)); + + assert_eq!(clones.get(), 0, "a write nobody observes must not copy"); +} + +#[test] +fn set_still_delivers_to_listeners() { + let clones = Rc::new(Cell::new(0)); + let value = Value::new(list(3, 0, &clones)); + + let seen = Rc::new(Cell::new(0)); + let _event = value.add_event({ + let seen = seen.clone(); + move |list: Vec| seen.set(list.len()) + }); + + value.set(list(3, 100, &clones)); + + assert_eq!(seen.get(), 3); +} diff --git a/crates/vertigo/src/computed/value.rs b/crates/vertigo/src/computed/value.rs index 99dba6aa..895415d6 100644 --- a/crates/vertigo/src/computed/value.rs +++ b/crates/vertigo/src/computed/value.rs @@ -1,7 +1,5 @@ use std::rc::Rc; -use vertigo_macro::bind; - use crate::{Context, DomNode, ToComputed, computed::value_inner::ValueInner}; use super::{Computed, DropResource, GraphId, dependencies::get_dependencies}; @@ -181,57 +179,4 @@ impl Value { pub fn add_event(&self, callback: impl Fn(T) + 'static) -> DropResource { self.inner.add_event(callback) } - - /// Mirror this `Value` into a derived, self-updating structure `R`. - /// - /// Builds the target with [`R::new`](ValueSynchronize::new) from the current - /// value, then subscribes so every later [`set`](Value::set) is forwarded to - /// [`R::set`](ValueSynchronize::set). The returned [`DropResource`] owns the - /// subscription — synchronization stops as soon as it is dropped. - /// - /// The most common target is the keyed list collection used by - /// [`render_list_memo`](crate::render::render_list_memo), but any type that - /// implements [`ValueSynchronize`] can be used (e.g. an index or a sorted - /// view). Implement [`ValueSynchronize`] to provide your own target. - pub fn synchronize + Clone + 'static>(&self) -> (R, DropResource) { - let init_value = self.inner.get(); - let synchronize_target = R::new(init_value); - - let drop_synchronize = self.add_event(bind!(synchronize_target, |current| { - synchronize_target.set(current); - })); - - (synchronize_target, drop_synchronize) - } -} - -/// Contract for a type that can be kept in sync with a reactive source via -/// [`Value::synchronize`] (and [`LazyCache::synchronize`](crate::LazyCache::synchronize)). -/// -/// A synchronization *target* is constructed from the source's initial value and -/// then receives every subsequent update. Because `set` takes `&self`, targets -/// are expected to rely on interior reactivity (e.g. an inner [`Value`]) and to -/// be cheaply `Clone`able — the update closure captures a clone of the target. -/// -/// ```rust -/// use vertigo::{Value, ValueSynchronize}; -/// -/// // A target that simply mirrors the latest value into its own `Value`. -/// #[derive(Clone)] -/// struct Mirror(Value); -/// -/// impl ValueSynchronize for Mirror { -/// fn new(value: i32) -> Self { Mirror(Value::new(value)) } -/// fn set(&self, value: i32) { self.0.set(value); } -/// } -/// -/// let source = Value::new(1); -/// let (mirror, _drop) = source.synchronize::(); -/// source.set(2); // `mirror` now follows `source`; dropping `_drop` stops it. -/// ``` -pub trait ValueSynchronize: Sized { - /// Build the target from the source's current value. - fn new(value: T) -> Self; - /// Apply a subsequent update from the source. - fn set(&self, value: T); } diff --git a/crates/vertigo/src/computed/value_inner.rs b/crates/vertigo/src/computed/value_inner.rs index e8867fdd..50599b58 100644 --- a/crates/vertigo/src/computed/value_inner.rs +++ b/crates/vertigo/src/computed/value_inner.rs @@ -11,13 +11,19 @@ impl ValueInner { pub fn new(value: T) -> ValueInner { ValueInner { id: GraphId::new_value(), - value: ValueMut::new(value.clone()), + value: ValueMut::new(value), events: EventEmitter::default(), } } #[must_use] pub fn set(&self, value: T) -> bool { + // The clone only exists to hand the new value to the listeners, so skip it when + // there are none - otherwise every write deep-copies whatever is stored. + if self.events.is_empty() { + return self.value.set_if_changed(value); + } + let change = self.value.set_if_changed(value.clone()); if change { diff --git a/crates/vertigo/src/dom/dom_comment.rs b/crates/vertigo/src/dom/dom_comment.rs index 1841eff8..8d27a813 100644 --- a/crates/vertigo/src/dom/dom_comment.rs +++ b/crates/vertigo/src/dom/dom_comment.rs @@ -1,3 +1,5 @@ +use std::rc::Rc; + use crate::{ DomNode, computed::{ @@ -9,6 +11,34 @@ use crate::{ use super::dom_id::DomId; +/// The nodes a marker keeps directly in front of itself, in document order. +/// +/// A marker created with [`DomComment::new_marker`] renders its content as siblings +/// placed just before the marker comment. Reporting those ids here lets the marker +/// carry them along when it is moved inside its parent, instead of tearing the +/// content down and building it again — so DOM state (focus, selection, scroll +/// position, running animations) survives a move. +/// +/// Whoever creates the content is responsible for keeping this up to date; a marker +/// that reports nothing is rebuilt on every move. +#[derive(Clone)] +pub struct MarkerContent { + ids: Rc>>, +} + +impl MarkerContent { + fn new() -> MarkerContent { + MarkerContent { + ids: Rc::new(ValueMut::new(Vec::new())), + } + } + + /// Replace the list of nodes standing in front of the marker. + pub fn set(&self, ids: Vec) { + self.ids.set(ids); + } +} + /// A Real DOM representative - comment kind pub struct DomComment { pub id_dom: DomId, @@ -28,17 +58,39 @@ impl DomComment { } } - pub fn new_marker Option + 'static>( + /// Create a comment that marks a place in the DOM, and mount content in front of it. + /// + /// `mount` runs when the marker enters a parent, and again if it is ever moved to a + /// *different* parent. Moving the marker inside the same parent does **not** re-run it: + /// the marker instead re-inserts the nodes reported through [`MarkerContent`] ahead of + /// itself, so the existing subtree travels with the marker rather than being rebuilt. + pub fn new_marker Option + 'static>( comment_value: &'static str, mount: F, ) -> DomComment { let id_comment = DomId::default(); + let content = MarkerContent::new(); let when_mount = { let current_client: ValueMut> = ValueMut::new(None); + let mounted_in: ValueMut> = ValueMut::new(None); + let content = content.clone(); move |parent_id| { - let client = mount(parent_id, id_comment); + let owned = content.ids.get(); + + if !owned.is_empty() && mounted_in.get() == Some(parent_id) { + // Already mounted here, so this is a move within the parent. Bring the + // content along - each nested marker does the same for its own content. + for child_id in owned { + get_driver_dom().insert_before(parent_id, child_id, Some(id_comment)); + } + + return; + } + + let client = mount(parent_id, id_comment, &content); + mounted_in.set(Some(parent_id)); current_client.change(|current| { *current = client; @@ -71,7 +123,7 @@ impl DomComment { pub fn dom_fragment(mut list: Vec) -> DomComment { list.reverse(); - Self::new_marker("list dom node", move |parent_id, comment_id| { + Self::new_marker("list dom node", move |parent_id, comment_id, content| { let mut prev_node = comment_id; for node in list.iter() { @@ -80,6 +132,9 @@ impl DomComment { prev_node = node_id; } + // `list` was reversed up front, so document order is back-to-front here. + content.set(list.iter().rev().map(|node| node.id_dom()).collect()); + None }) } diff --git a/crates/vertigo/src/driver_module/dom.rs b/crates/vertigo/src/driver_module/dom.rs index b495c7e7..37108fa2 100644 --- a/crates/vertigo/src/driver_module/dom.rs +++ b/crates/vertigo/src/driver_module/dom.rs @@ -15,7 +15,8 @@ use super::StaticString; struct Commands { commands: VecMut, - // For testing/debuging purposes + /// Opt-in tap on the command stream, used by [`crate::dev::inspect`]. Nothing + /// subscribes to it unless a debugging session asks for it. new_command: EventEmitter, } @@ -27,7 +28,6 @@ impl Commands { } } - #[allow(dead_code)] fn inspect_command(&self, func: impl Fn(DriverDomCommand) + 'static) -> DropResource { self.new_command.add(func) } @@ -86,7 +86,8 @@ impl DriverDom { } } - #[allow(dead_code)] + /// Watch every DOM command as it is produced. For debugging and tests only - each + /// subscriber gets its own clone of every command. pub fn inspect_command(&self, func: impl Fn(DriverDomCommand) + 'static) -> DropResource { self.commands.inspect_command(func) } diff --git a/crates/vertigo/src/driver_module/event_emitter.rs b/crates/vertigo/src/driver_module/event_emitter.rs index e677875f..23738d66 100644 --- a/crates/vertigo/src/driver_module/event_emitter.rs +++ b/crates/vertigo/src/driver_module/event_emitter.rs @@ -35,7 +35,18 @@ impl EventEmitter { }) } + /// True when nothing is listening, so the caller can skip preparing a value to emit. + pub fn is_empty(&self) -> bool { + self.list.is_empty() + } + pub fn trigger(&self, value: &T) { + // Emitters on hot paths (every DOM command, every `Value` write) usually have no + // listeners at all, so do not snapshot the callback list for them. + if self.list.is_empty() { + return; + } + let callback_list = self .list .map(|state| state.values().cloned().collect::>()); @@ -45,3 +56,68 @@ impl EventEmitter { } } } + +#[cfg(test)] +mod tests { + use std::{cell::Cell, rc::Rc}; + + use super::EventEmitter; + + /// Counts how often the payload is cloned on its way to the listeners. + #[derive(Debug)] + struct Counted { + clones: Rc>, + } + + impl Counted { + fn new(clones: &Rc>) -> Counted { + Counted { + clones: clones.clone(), + } + } + } + + impl Clone for Counted { + fn clone(&self) -> Self { + self.clones.set(self.clones.get() + 1); + + Counted { + clones: self.clones.clone(), + } + } + } + + #[test] + fn trigger_without_listeners_does_not_touch_the_payload() { + let clones = Rc::new(Cell::new(0)); + let emitter = EventEmitter::::default(); + + assert!(emitter.is_empty()); + + emitter.trigger(&Counted::new(&clones)); + + assert_eq!(clones.get(), 0); + } + + #[test] + fn trigger_clones_the_payload_once_per_listener() { + let clones = Rc::new(Cell::new(0)); + let calls = Rc::new(Cell::new(0)); + let emitter = EventEmitter::::default(); + + let _first = emitter.add({ + let calls = calls.clone(); + move |_| calls.set(calls.get() + 1) + }); + let _second = emitter.add({ + let calls = calls.clone(); + move |_| calls.set(calls.get() + 1) + }); + + assert!(!emitter.is_empty()); + + emitter.trigger(&Counted::new(&clones)); + + assert_eq!((calls.get(), clones.get()), (2, 2)); + } +} diff --git a/crates/vertigo/src/fetch/cache_value.rs b/crates/vertigo/src/fetch/cache_value.rs index 48642598..e031c149 100644 --- a/crates/vertigo/src/fetch/cache_value.rs +++ b/crates/vertigo/src/fetch/cache_value.rs @@ -1,6 +1,6 @@ use crate::{ Computed, DropResource, - computed::{Value, ValueSynchronize, context::Context}, + computed::{Value, context::Context}, driver_module::api::api_timers, fetch::api_response::ApiResponse, }; @@ -63,35 +63,4 @@ impl CacheValue { pub fn set(&self, value: ApiResponse) { self.value_write.set(value); } - - pub fn synchronize> + Clone + 'static>( - &self, - ) -> (R, DropResource) - where - T: Default + Clone, - { - use crate::{Resource, transaction}; - use std::rc::Rc; - use vertigo_macro::bind; - - fn normalize(val: ApiResponse) -> Rc { - match val { - ApiResponse::Uninitialized => Rc::new(T::default()), - ApiResponse::Data { value, expiry: _ } => match value { - Resource::Ready(data) => data, - Resource::Loading => Rc::new(T::default()), - Resource::Error(_) => Rc::new(T::default()), - }, - } - } - - let init_val = transaction(|ctx| normalize(self.value_write.get(ctx))); - let target = R::new(init_val); - - let drop = self.value_write.add_event(bind!(target, |val| { - target.set(normalize(val)); - })); - - (target, drop) - } } diff --git a/crates/vertigo/src/fetch/lazy_cache.rs b/crates/vertigo/src/fetch/lazy_cache.rs index 88d424ea..c0146ec5 100644 --- a/crates/vertigo/src/fetch/lazy_cache.rs +++ b/crates/vertigo/src/fetch/lazy_cache.rs @@ -2,8 +2,8 @@ use std::fmt::Debug; use std::rc::Rc; use crate::{ - Computed, DomNode, DropResource, JsJsonDeserialize, RequestResponse, Resource, ToComputed, - computed::{ValueSynchronize, context::Context, struct_mut::ValueMut}, + Computed, DomNode, JsJsonDeserialize, RequestResponse, Resource, ToComputed, + computed::{context::Context, struct_mut::ValueMut}, driver_module::api::{api_fetch, api_fetch_cache}, fetch::{api_response::ApiResponse, cache_value::CacheValue}, get_driver, transaction, @@ -233,25 +233,6 @@ impl LazyCache { self_clone.queued.set(false); }); } - - /// Mirror this cache into a derived, self-updating structure `R`. - /// - /// Like [`Value::synchronize`](crate::Value::synchronize), but the source is - /// a fetched resource: the cache's `Resource>` is normalized to a - /// concrete `Rc` (Loading / Error / Uninitialized become `T::default()`, - /// hence the `T: Default` bound) before being pushed into the target. - /// - /// The target keeps following the cache across refreshes until the returned - /// [`DropResource`] is dropped. This is the mechanism behind - /// [`render_resource_list_memo`](crate::render::render_resource_list_memo). - pub fn synchronize> + Clone + 'static>( - &self, - ) -> (R, DropResource) - where - T: Default + Clone, - { - self.value.synchronize() - } } impl ToComputed>> for LazyCache { diff --git a/crates/vertigo/src/lib.rs b/crates/vertigo/src/lib.rs index 45524af2..97ef4a11 100644 --- a/crates/vertigo/src/lib.rs +++ b/crates/vertigo/src/lib.rs @@ -12,6 +12,7 @@ //! * Data storing //! * [Value] - Read-write reactive value //! * [Computed] - Read-only (computed) reactive value +//! * [keyed_computed_list] - Stable per-key [`Computed`]s from a reactive list //! * [LazyCache] - Lazy cache for fetched resources //! * [LazyListCache] - Lazy cache for fetched lists (optimized for CRUD operations) //! * [WsCollection] - Reactive collection driven by a server subscription over a WebSocket @@ -23,8 +24,7 @@ //! //! # Guides //! -//! * [guides::value_synchronize_and_collections] - `Value::synchronize`, `ValueSynchronize`, -//! `CollectionKey` and the memoized list renderers +//! * [guides::collection_key_and_list_renderers] - `CollectionKey` and the memoized list renderers //! * [guides::lazy_list_cache] - `LazyListCache`: optimistic, per-item reactive list cache //! * [guides::websocket_collection] - `WsCollection`: server-pushed reactive collections over a WebSocket @@ -56,8 +56,8 @@ mod websocket_collection; /// directory so it gets its own rustdoc page instead of being inlined into the /// crate root. pub mod guides { - #[doc = include_str!("../docs/value-synchronize-and-collections.md")] - pub mod value_synchronize_and_collections {} + #[doc = include_str!("../docs/collection-key-and-list-renderers.md")] + pub mod collection_key_and_list_renderers {} #[doc = include_str!("../docs/lazy-list-cache.md")] pub mod lazy_list_cache {} @@ -69,8 +69,8 @@ pub mod guides { // Exports from vertigo pub use computed::{ - AutoMap, Computed, Dependencies, DropResource, Reactive, ToComputed, Value, ValueSynchronize, - context::Context, + AutoMap, Computed, Dependencies, DropResource, KeyedListItem, Reactive, ToComputed, Value, + context::Context, keyed_computed_list, }; pub use css::{ css_structs::{Css, CssGroup}, @@ -78,7 +78,7 @@ pub use css::{ }; pub use dom::{ attr_value::{AttrValue, CssAttrValue}, - dom_comment::DomComment, + dom_comment::{DomComment, MarkerContent}, dom_element::DomElement, dom_element_ref::DomElementRef, dom_id::DomId, diff --git a/crates/vertigo/src/render/collection.rs b/crates/vertigo/src/render/collection.rs index 1d6d672c..bd201066 100644 --- a/crates/vertigo/src/render/collection.rs +++ b/crates/vertigo/src/render/collection.rs @@ -1,15 +1,12 @@ -use crate::computed::ValueSynchronize; -use crate::{Computed, Value, dev::HashMapMut, transaction}; -use std::{collections::HashSet, hash::Hash, marker::PhantomData, rc::Rc}; - -use log; +use std::hash::Hash; /// Describes how items in a reactive list are identified. /// /// Implement this on a zero-sized **marker type** (not on the item itself). The /// marker is the generic parameter threaded through the keyed-collection /// machinery: [`render_list_memo`](crate::render::render_list_memo), -/// [`render_resource_list_memo`](crate::render::render_resource_list_memo) and +/// [`render_resource_list_memo`](crate::render::render_resource_list_memo) (via +/// [`keyed_computed_list`](crate::keyed_computed_list)) and /// [`LazyListCache`](crate::LazyListCache). /// /// The associated [`Key`](CollectionKey::Key) gives each item a stable identity. @@ -40,254 +37,3 @@ pub trait CollectionKey { /// Extract the key that identifies `val`. fn get_key(val: &Self::Value) -> Self::Key; } - -#[derive(Clone)] -struct ItemData { - value: Value, - computed: Computed, -} - -struct ItemDataCollection { - items: Rc>>, - _marker: PhantomData, -} - -impl Clone for ItemDataCollection { - fn clone(&self) -> Self { - ItemDataCollection { - items: self.items.clone(), - _marker: PhantomData, - } - } -} - -impl ItemDataCollection { - pub fn new() -> ItemDataCollection { - ItemDataCollection { - items: Rc::new(HashMapMut::new()), - _marker: PhantomData, - } - } - - fn get_item(&self, key: &T::Key, item: &T::Value) -> CollectionModel { - if let Some(model) = self.items.get(key) { - model.value.set(item.clone()); - return CollectionModel { - key: key.clone(), - model: model.computed, - }; - } - - let model_value = Value::new(item.clone()); - let model_computed = model_value.to_computed(); - - let model = ItemData { - value: model_value, - computed: model_computed, - }; - - self.items.insert(key.clone(), model.clone()); - - CollectionModel { - key: key.clone(), - model: model.computed, - } - } - - fn translate(&self, list: Rc>) -> Vec> { - let mut new_order: Vec> = Vec::with_capacity(list.len()); - let mut seen_keys = HashSet::new(); - - for item in list.as_ref() { - let key = T::get_key(item); - - if seen_keys.contains(&key) { - log::error!("Duplicate key found in Collection: {:?}", key); - continue; - } - - seen_keys.insert(key.clone()); - - let model = self.get_item(&key, item); - new_order.push(model); - } - - new_order - } - - fn retain(&self, new_keys: HashSet) { - self.items.retain(|k, _| new_keys.contains(k)); - } -} - -pub struct CollectionModel { - pub key: T::Key, - pub model: Computed, -} - -impl Clone for CollectionModel { - fn clone(&self) -> Self { - CollectionModel { - key: self.key.clone(), - model: self.model.clone(), - } - } -} - -impl PartialEq for CollectionModel { - fn eq(&self, other: &Self) -> bool { - self.key == other.key && self.model == other.model - } -} - -pub struct Collection { - items: ItemDataCollection, - order: Value>>, -} - -impl Clone for Collection { - fn clone(&self) -> Self { - Collection { - items: self.items.clone(), - order: self.order.clone(), - } - } -} - -impl Collection { - pub fn new(list: Rc>) -> Collection { - let items = ItemDataCollection::new(); - let order = items.translate(list); - - Collection { - items, - order: Value::new(order), - } - } - - pub fn set(&self, list: Rc>) { - transaction(|_ctx| { - let new_order = self.items.translate(list); - - let new_keys = new_order - .iter() - .map(|item| item.key.clone()) - .collect::>(); - - self.order.set(new_order); - - self.items.retain(new_keys); - }) - } - - pub fn get(&self) -> Computed>> { - self.order.to_computed() - } -} - -impl ValueSynchronize>> for Collection { - fn new(value: Rc>) -> Self { - Collection::new(value) - } - - fn set(&self, value: Rc>) { - self.set(value); - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_collection_basic() { - #[derive(Clone, PartialEq, Debug)] - struct Item { - id: i32, - name: String, - } - - struct ItemId; - impl CollectionKey for ItemId { - type Key = i32; - type Value = Item; - fn get_key(val: &Item) -> i32 { - val.id - } - } - - let col = Collection::::new(Rc::new(Vec::new())); - - let items = vec![ - Item { - id: 1, - name: "One".into(), - }, - Item { - id: 2, - name: "Two".into(), - }, - ]; - - col.set(Rc::new(items.clone())); - - transaction(|ctx| { - let res = col.get().get(ctx); - assert_eq!(res.len(), 2); - assert_eq!(res[0].model.get(ctx).name, "One"); - assert_eq!(res[1].model.get(ctx).name, "Two"); - }); - } - - #[test] - fn test_collection_reactivity() { - #[derive(Clone, PartialEq, Debug)] - struct Item { - id: i32, - val: i32, - } - - struct ItemId; - impl CollectionKey for ItemId { - type Key = i32; - type Value = Item; - fn get_key(val: &Item) -> i32 { - val.id - } - } - - let col = Collection::::new(Rc::new(Vec::new())); - - col.set(Rc::new(vec![Item { id: 1, val: 10 }])); - - let list_computed = col.get(); - let item_computed = transaction(|ctx| { - // list[0] is (Key, Computed) - list_computed.get(ctx)[0].model.clone() - }); - - transaction(|ctx| { - assert_eq!(item_computed.get(ctx).val, 10); - }); - - // Update item 1 - col.set(Rc::new(vec![Item { id: 1, val: 20 }])); - - transaction(|ctx| { - assert_eq!(item_computed.get(ctx).val, 20); - }); - - // Add item - col.set(Rc::new(vec![ - Item { id: 1, val: 20 }, - Item { id: 2, val: 30 }, - ])); - - transaction(|ctx| { - let new_list = list_computed.get(ctx); - assert_eq!(new_list.len(), 2); - // Ensure old reference still works and has correct value - assert_eq!(item_computed.get(ctx).val, 20); - }); - } -} diff --git a/crates/vertigo/src/render/render_list.rs b/crates/vertigo/src/render/render_list.rs index 2bd456f6..4b6bb4e4 100644 --- a/crates/vertigo/src/render/render_list.rs +++ b/crates/vertigo/src/render/render_list.rs @@ -5,11 +5,18 @@ use std::{ }; use crate::{ - Computed, DomComment, DomNode, ToComputed, computed::struct_mut::ValueMut, dom::dom_id::DomId, - driver_module::get_driver_dom, + Computed, DomComment, DomNode, KeyedListItem, ToComputed, computed::struct_mut::ValueMut, + dom::dom_id::DomId, driver_module::get_driver_dom, keyed_computed_list, }; -/// Render iterable value (reactively transforms `Iterator` into Node with list of rendered elements ) +/// Render an iterable as a keyed list of DOM nodes. +/// +/// Each key keeps a stable [`Computed`](crate::Computed) (via [`keyed_computed_list`]). +/// `render` is called when a key **appears**; the node is dropped when the key +/// **leaves**. Item content updates go through that `Computed` — embed it in +/// `dom!`, or wrap with [`Computed::render_value`](crate::Computed::render_value). +/// +/// Duplicate keys are skipped (first occurrence is kept). /// /// ```rust /// use vertigo::{dom, Value, render::render_list}; @@ -23,7 +30,7 @@ use crate::{ /// let elements = render_list( /// &my_list.to_computed(), /// |el| el.0, -/// |el| dom! {
{el.1}
} +/// |el| el.render_value(|el| dom! {
{el.1}
}) /// ); /// /// dom! { @@ -32,50 +39,42 @@ use crate::{ /// /// }; /// ``` -/// -/// pub fn render_list< - T: PartialEq + Clone + 'static, - K: Eq + Hash, - L: IntoIterator + Clone + PartialEq + 'static, + T: Clone + PartialEq + 'static, + K: Clone + Eq + Hash + std::fmt::Debug + 'static, >( - computed: impl ToComputed, + computed: impl ToComputed>, get_key: impl Fn(&T) -> K + 'static, - render: impl Fn(&T) -> DomNode + 'static, + render: impl Fn(&Computed) -> DomNode + 'static, ) -> DomNode { - let get_key = Rc::new(get_key); + let rows = keyed_computed_list(computed, get_key); let render = Rc::new(render); - let computed: Computed = computed.to_computed(); - - DomComment::new_marker("list element", move |parent_id, comment_id| { - let current_list: Rc>> = + DomComment::new_marker("list element", move |parent_id, comment_id, content| { + let current_list: Rc>> = Rc::new(ValueMut::new(VecDeque::new())); - Some(computed.clone().subscribe({ - let get_key = get_key.clone(); + Some(rows.clone().subscribe({ let render = render.clone(); + let content = content.clone(); move |new_list| { - let new_list = VecDeque::from_iter(new_list); - current_list.change({ - let get_key = get_key.clone(); - let render = render.clone(); - - move |current| { - let current_list = std::mem::take(current); - - let new_order = reorder_nodes( - parent_id, - comment_id, - current_list, - new_list, - get_key.clone(), - render, - ); - - *current = new_order; - } + current_list.change(|current| { + let prev = std::mem::take(current); + *current = reorder_nodes( + parent_id, + comment_id, + prev, + VecDeque::from(new_list), + render.as_ref(), + ); + + content.set( + current + .iter() + .flat_map(|(_, row)| [row.anchor_id(), row.node.id_dom()]) + .collect(), + ); }) } })) @@ -83,26 +82,58 @@ pub fn render_list< .into() } -fn reorder_nodes( +/// One rendered row of the list. +/// +/// A row is not necessarily a single sibling under the list parent: +/// [`render_value`](crate::Computed::render_value) keeps its content *in front of* +/// its own marker and re-creates it every time that marker is mounted. `anchor` is +/// an empty comment kept directly in front of the row, so that whatever shape the +/// row has, it always has one stable node marking where it begins — that is what +/// insert and move operations anchor on. +struct Row { + anchor: DomComment, + node: DomNode, +} + +impl Row { + fn new(node: DomNode) -> Row { + Row { + anchor: DomComment::new("row"), + node, + } + } + + fn anchor_id(&self) -> DomId { + self.anchor.id_dom() + } + + /// Insert the row - or move it, when it is already mounted - in front of `before`. + fn insert_before(&self, parent_id: DomId, before: DomId) { + let driver = get_driver_dom(); + + driver.insert_before(parent_id, self.anchor.id_dom(), Some(before)); + // Mounting the node renders its content in front of itself, which lands + // between the anchor and the node. + driver.insert_before(parent_id, self.node.id_dom(), Some(before)); + } +} + +fn reorder_nodes( parent_id: DomId, comment_id: DomId, - mut real_child: VecDeque<(T, DomNode)>, - mut new_child: VecDeque, - get_key: Rc K + 'static>, - render: Rc DomNode + 'static>, -) -> VecDeque<(T, DomNode)> { + mut real_child: VecDeque<(K, Row)>, + mut new_child: VecDeque>>, + render: &dyn Fn(&Computed) -> DomNode, +) -> VecDeque<(K, Row)> { let pairs_top = get_pairs_top(&mut real_child, &mut new_child); let mut pairs_bottom = get_pairs_bottom(&mut real_child, &mut new_child); - let last_before: DomId = find_first_dom(&pairs_bottom).unwrap_or(comment_id); - let mut pairs_middle = get_pairs_middle( - parent_id, - last_before, - real_child, - new_child, - get_key, - render, - ); + let last_before = pairs_bottom + .front() + .map(|(_, row)| row.anchor_id()) + .unwrap_or(comment_id); + + let mut pairs_middle = get_pairs_middle(parent_id, last_before, real_child, new_child, render); let mut pairs = pairs_top; pairs.append(&mut pairs_middle); @@ -110,41 +141,25 @@ fn reorder_nodes( pairs } -fn find_first_dom(list: &VecDeque<(T, DomNode)>) -> Option { - if let Some((_, first)) = list.front() { - return Some(first.id_dom()); - } - - None -} - -// Try to match starting from top -fn get_pairs_top( - current: &mut VecDeque<(T, DomNode)>, - new_child: &mut VecDeque, -) -> VecDeque<(T, DomNode)> { +fn get_pairs_top( + current: &mut VecDeque<(K, Row)>, + new_child: &mut VecDeque>>, +) -> VecDeque<(K, Row)> { let mut pairs_top = VecDeque::new(); loop { - let node = current.pop_front(); - let child = new_child.pop_front(); - - match (node, child) { - (Some((id1, node)), Some(id2)) => { - if id1 == id2 { - pairs_top.push_back((id1, node)); + match (current.pop_front(), new_child.pop_front()) { + (Some((key, node)), Some(item)) => { + if key == item.key { + pairs_top.push_back((key, node)); continue; } - current.push_front((id1, node)); - new_child.push_front(id2); - } - (Some(pair), None) => { - current.push_front(pair); - } - (None, Some(child)) => { - new_child.push_front(child); + current.push_front((key, node)); + new_child.push_front(item); } + (Some(pair), None) => current.push_front(pair), + (None, Some(item)) => new_child.push_front(item), (None, None) => {} } @@ -152,33 +167,25 @@ fn get_pairs_top( } } -// Try to match starting from bottom -fn get_pairs_bottom( - current: &mut VecDeque<(T, DomNode)>, - new_child: &mut VecDeque, -) -> VecDeque<(T, DomNode)> { +fn get_pairs_bottom( + current: &mut VecDeque<(K, Row)>, + new_child: &mut VecDeque>>, +) -> VecDeque<(K, Row)> { let mut pairs_bottom = VecDeque::new(); loop { - let node = current.pop_back(); - let child = new_child.pop_back(); - - match (node, child) { - (Some((id1, node)), Some(id2)) => { - if id1 == id2 { - pairs_bottom.push_front((id1, node)); + match (current.pop_back(), new_child.pop_back()) { + (Some((key, node)), Some(item)) => { + if key == item.key { + pairs_bottom.push_front((key, node)); continue; } - current.push_back((id1, node)); - new_child.push_back(id2); - } - (Some(node), None) => { - current.push_back(node); - } - (None, Some(child)) => { - new_child.push_back(child); + current.push_back((key, node)); + new_child.push_back(item); } + (Some(pair), None) => current.push_back(pair), + (None, Some(item)) => new_child.push_back(item), (None, None) => {} } @@ -186,161 +193,639 @@ fn get_pairs_bottom( } } -fn get_pairs_middle( +fn get_pairs_middle( parent_id: DomId, last_before: DomId, - real_child: VecDeque<(T, DomNode)>, - new_child: VecDeque, - get_key: Rc K + 'static>, - render: Rc DomNode + 'static>, -) -> VecDeque<(T, DomNode)> { - let mut pairs_middle: VecDeque<(T, DomNode)> = VecDeque::new(); + real_child: VecDeque<(K, Row)>, + new_child: VecDeque>>, + render: &dyn Fn(&Computed) -> DomNode, +) -> VecDeque<(K, Row)> { + let mut cache: HashMap = real_child.into_iter().collect(); + let mut pairs_middle = VecDeque::new(); + + for item in new_child { + let row = match cache.remove(&item.key) { + Some(row) => row, + None => Row::new(render(&item.value)), + }; - let mut real_node: CacheNode = CacheNode::new(get_key, render); + row.insert_before(parent_id, last_before); + pairs_middle.push_back((item.key, row)); + } - for (id, node) in real_child.into_iter() { - real_node.insert(&id, node); + pairs_middle +} + +#[cfg(test)] +mod tests { + use std::{cell::Cell, rc::Rc}; + + use super::{Row, render_list, reorder_nodes}; + use crate::{self as vertigo, dom}; + use crate::{ + Computed, DomId, DomNode, KeyedListItem, Value, + dev::inspect::{DomDebugFragment, log_start}, + }; + + fn row(key: u32, label: &str) -> (u32, String) { + (key, label.to_string()) } - let mut last_before = last_before; + fn item(key: u32, label: &str) -> KeyedListItem> { + KeyedListItem { + key, + value: Computed::from({ + let label = label.to_string(); + move |_| label.clone() + }), + } + } - for item in new_child.into_iter().rev() { - let node = real_node.get_or_create(&item); - let node_id = node.id_dom(); - pairs_middle.push_front((item, node)); + fn mount_render_value_list(items: &Value>) -> DomNode { + let list = render_list( + items, + |item| item.0, + |item| item.render_value(|item| dom! {
  • {item.1.as_str()}
  • }), + ); + dom! {
      {list}
    } + } - get_driver_dom().insert_before(parent_id, node_id, Some(last_before)); - last_before = node_id; + fn pseudo_html(items: &Value>) -> String { + log_start(); + let _root = mount_render_value_list(items); + DomDebugFragment::from_log().to_pseudo_html() } - pairs_middle -} + fn pseudo_html_after(items: &Value>, update: impl FnOnce()) -> String { + log_start(); + let _root = mount_render_value_list(items); + update(); + DomDebugFragment::from_log().to_pseudo_html() + } -struct CacheNode { - get_key: Rc K + 'static>, - create_new: Rc DomNode + 'static>, - data: HashMap>, -} + fn row_html(label: &str) -> String { + format!("
  • {label}
  • ") + } -impl CacheNode { - pub fn new( - get_key: Rc K + 'static>, - create_new: Rc DomNode + 'static>, - ) -> CacheNode { - CacheNode { - get_key, - create_new, - data: HashMap::new(), - } + fn list_html(rows: &[&str]) -> String { + format!( + "
      {rows}
    ", + rows = rows.iter().map(|label| row_html(label)).collect::() + ) } - pub fn insert(&mut self, item: &T, element: DomNode) { - let key = (self.get_key)(item); - let queue = self.data.entry(key).or_default(); - queue.push_back((item.clone(), element)); + /// A row whose root is a plain element (no `render_value` wrapper around it). + fn mount_element_list(items: &Value>) -> DomNode { + let list = render_list( + items, + |item| item.0, + |item| { + let label = item.map(|item| item.1); + dom! {
  • {label}
  • } + }, + ); + dom! {
      {list}
    } } - pub fn get_or_create(&mut self, item: &T) -> DomNode { - let key = (self.get_key)(item); - let element = self.data.entry(key).or_default().pop_front(); + fn element_pseudo_html_after( + items: &Value>, + update: impl FnOnce(), + ) -> String { + log_start(); + let _root = mount_element_list(items); + update(); + DomDebugFragment::from_log().to_pseudo_html() + } - let CacheNode { create_new, .. } = self; + fn element_list_html(rows: &[&str]) -> String { + format!( + "
      {rows}
    ", + rows = rows + .iter() + .map(|label| format!("
  • {label}
  • ")) + .collect::() + ) + } - match element { - Some((old_item, node)) if old_item == *item => node, - Some((_old_item, _node)) => create_new(item), - None => create_new(item), - } + #[test] + fn element_rows_prepend() { + let items = Value::new(vec![row(2, "two"), row(3, "three")]); + + let html = element_pseudo_html_after(&items, || { + items.set(vec![row(1, "one"), row(2, "two"), row(3, "three")]); + }); + + assert_eq!(html, element_list_html(&["one", "two", "three"])); } -} -#[cfg(test)] -mod tests { - use std::rc::Rc; + #[test] + fn element_rows_insert_in_middle() { + let items = Value::new(vec![row(1, "one"), row(3, "three")]); - use super::reorder_nodes; - use crate::{DomId, DomNode, computed::struct_mut::ValueMut}; + let html = element_pseudo_html_after(&items, || { + items.set(vec![row(1, "one"), row(2, "two"), row(3, "three")]); + }); - #[derive(Clone, PartialEq, Debug)] - struct Item { - id: u32, - label: String, + assert_eq!(html, element_list_html(&["one", "two", "three"])); } + /// The trailing row is untouched, so the moved rows have to anchor on it + /// instead of on the list marker. #[test] - fn rerenders_node_when_item_changes_but_key_stays_the_same() { - let old_item = Item { - id: 1, - label: "old".to_string(), - }; - let new_item = Item { - id: 1, - label: "new".to_string(), - }; + fn element_rows_reorder_in_middle() { + let items = Value::new(vec![ + row(1, "one"), + row(2, "two"), + row(3, "three"), + row(4, "four"), + ]); + + let html = element_pseudo_html_after(&items, || { + items.set(vec![ + row(1, "one"), + row(3, "three"), + row(2, "two"), + row(4, "four"), + ]); + }); + + assert_eq!(html, element_list_html(&["one", "three", "two", "four"])); + } + + /// A row only ever anchors on nodes it owns, so unrelated siblings under the + /// same parent keep their place. + #[test] + fn sibling_before_the_list_keeps_its_place() { + let items = Value::new(vec![row(2, "two"), row(3, "three"), row(4, "four")]); + + log_start(); + let list = render_list( + &items, + |item| item.0, + |item| item.render_value(|item| dom! {
  • {item.1.as_str()}
  • }), + ); + let _root = dom! {
    • "header"
    • {list}
    }; + items.set(vec![ + row(1, "one"), + row(3, "three"), + row(2, "two"), + row(4, "four"), + ]); + + let rows = ["one", "three", "two", "four"] + .iter() + .map(|label| row_html(label)) + .collect::(); + + assert_eq!( + DomDebugFragment::from_log().to_pseudo_html(), + format!("
    • header
    • {rows}
    ") + ); + } + + /// Two lists interleaved under one parent: each moves only its own rows. + #[test] + fn two_lists_in_the_same_parent_do_not_interfere() { + let left = Value::new(vec![row(1, "l1"), row(2, "l2")]); + let right = Value::new(vec![row(1, "r1"), row(2, "r2"), row(3, "r3")]); + + log_start(); + let left_list = render_list( + &left, + |item| item.0, + |item| item.render_value(|item| dom! {
  • {item.1.as_str()}
  • }), + ); + let right_list = render_list( + &right, + |item| item.0, + |item| item.render_value(|item| dom! {
  • {item.1.as_str()}
  • }), + ); + let _root = dom! {
      {left_list}{right_list}
    }; + + right.set(vec![row(2, "r2"), row(1, "r1"), row(3, "r3")]); + + let left_rows = ["l1", "l2"] + .iter() + .map(|label| row_html(label)) + .collect::(); + let right_rows = ["r2", "r1", "r3"] + .iter() + .map(|label| row_html(label)) + .collect::(); + + assert_eq!( + DomDebugFragment::from_log().to_pseudo_html(), + format!("
      {left_rows}{right_rows}
    ") + ); + } + + /// A row that is itself a list spans several siblings of the outer list's + /// parent, none of which is at a fixed offset from the row's own node. + #[test] + fn row_that_is_itself_a_list_reorders() { + let items = Value::new(vec![row(1, "one"), row(2, "two"), row(3, "three")]); + + log_start(); + let list = render_list( + &items, + |item| item.0, + |item| { + let labels = item.map(|item| vec![item.1]); + render_list( + labels, + |label| label.clone(), + |label| label.render_value(|label| dom! {
  • {label}
  • }), + ) + }, + ); + let _root = dom! {
      {list}
    }; + items.set(vec![row(2, "two"), row(1, "one"), row(3, "three")]); + + let rows = ["two", "one", "three"] + .iter() + .map(|label| { + format!("
  • {label}
  • ") + }) + .collect::(); + + assert_eq!( + DomDebugFragment::from_log().to_pseudo_html(), + format!("
      {rows}
    ") + ); + } + + /// A key appearing during an update runs `render` while the graph is mid-refresh, so + /// the nested list this row renders is built at that point. + #[test] + fn row_that_is_itself_a_list_can_be_added_during_an_update() { + let items = Value::new(vec![row(1, "one")]); + + log_start(); + let list = render_list( + &items, + |item| item.0, + |item| { + let labels = item.map(|item| vec![item.1]); + render_list( + labels, + |label| label.clone(), + |label| label.render_value(|label| dom! {
  • {label}
  • }), + ) + }, + ); + let _root = dom! {
      {list}
    }; + items.set(vec![row(1, "one"), row(2, "two")]); + + let rows = ["one", "two"] + .iter() + .map(|label| { + format!("
  • {label}
  • ") + }) + .collect::(); + + assert_eq!( + DomDebugFragment::from_log().to_pseudo_html(), + format!("
      {rows}
    ") + ); + } + + #[test] + fn key_removed_and_added_again() { + let items = Value::new(vec![row(1, "one"), row(2, "two"), row(3, "three")]); + + let html = pseudo_html_after(&items, || { + items.set(vec![row(1, "one"), row(3, "three")]); + items.set(vec![row(1, "one"), row(2, "two"), row(3, "three")]); + }); - let render_calls = Rc::new(ValueMut::new(0usize)); - let render_calls_for_closure = render_calls.clone(); + assert_eq!(html, list_html(&["one", "two", "three"])); + } - let render: Rc DomNode> = Rc::new(move |item| { - render_calls_for_closure.change(|count| *count += 1); - DomNode::from(item.label.clone()) + #[test] + fn renders_three_items_in_source_order() { + let items = Value::new(vec![row(1, "one"), row(2, "two"), row(3, "three")]); + + assert_eq!(pseudo_html(&items), list_html(&["one", "two", "three"])); + } + + #[test] + fn updates_item_content_without_rerendering() { + let items = Value::new(vec![row(1, "one")]); + let render_calls = Rc::new(Cell::new(0)); + + log_start(); + let list = render_list(&items, |item| item.0, { + let render_calls = render_calls.clone(); + move |item| { + render_calls.set(render_calls.get() + 1); + item.render_value(|item| dom! {
  • {item.1.as_str()}
  • }) + } }); + let _root = dom! {
      {list}
    }; + assert_eq!(render_calls.get(), 1); - let old_node = render(&old_item); - render_calls.set(0); + items.set(vec![row(1, "two")]); - let result = reorder_nodes( - DomId::from_u64(100), - DomId::from_u64(101), - std::collections::VecDeque::from([(old_item.clone(), old_node)]), - std::collections::VecDeque::from([new_item]), - Rc::new(|item: &Item| item.id), - render, + let html = DomDebugFragment::from_log().to_pseudo_html(); + assert_eq!(render_calls.get(), 1); + assert_eq!(html, list_html(&["two"])); + } + + #[test] + fn appends_item_after_initial_render() { + let items = Value::new(vec![row(1, "one"), row(2, "two")]); + + let html = pseudo_html_after(&items, || { + items.set(vec![row(1, "one"), row(2, "two"), row(3, "three")]); + }); + + assert_eq!(html, list_html(&["one", "two", "three"])); + } + + #[test] + fn removes_item_from_middle() { + let items = Value::new(vec![row(1, "one"), row(2, "two"), row(3, "three")]); + + let html = pseudo_html_after(&items, || { + items.set(vec![row(1, "one"), row(3, "three")]); + }); + + assert_eq!(html, list_html(&["one", "three"])); + } + + #[test] + fn removes_all_items() { + let items = Value::new(vec![row(1, "one"), row(2, "two")]); + + let html = pseudo_html_after(&items, || { + items.set(Vec::new()); + }); + + assert_eq!(html, "
    "); + } + + #[test] + fn reorders_items_in_middle() { + let items = Value::new(vec![row(1, "one"), row(2, "two"), row(3, "three")]); + + let html = pseudo_html_after(&items, || { + items.set(vec![row(1, "one"), row(3, "three"), row(2, "two")]); + }); + + assert_eq!(html, list_html(&["one", "three", "two"])); + } + + #[test] + fn prepends_item() { + let items = Value::new(vec![row(2, "two"), row(3, "three")]); + + let html = pseudo_html_after(&items, || { + items.set(vec![row(1, "one"), row(2, "two"), row(3, "three")]); + }); + + assert_eq!(html, list_html(&["one", "two", "three"])); + } + + /// Moving a row must carry its DOM along, not rebuild it — a rebuilt row loses + /// focus, selection, scroll position and running animations. + #[test] + fn moving_a_row_keeps_its_content() { + let items = Value::new(vec![ + row(1, "one"), + row(2, "two"), + row(3, "three"), + row(4, "four"), + ]); + let content_renders = Rc::new(Cell::new(0)); + + log_start(); + let list = render_list(&items, |item| item.0, { + let content_renders = content_renders.clone(); + move |item| { + let content_renders = content_renders.clone(); + item.render_value(move |item| { + content_renders.set(content_renders.get() + 1); + dom! {
  • {item.1.as_str()}
  • } + }) + } + }); + let _root = dom! {
      {list}
    }; + assert_eq!(content_renders.get(), 4); + + items.set(vec![ + row(1, "one"), + row(3, "three"), + row(2, "two"), + row(4, "four"), + ]); + + assert_eq!( + content_renders.get(), + 4, + "moving a row must not re-render its content" + ); + assert_eq!( + DomDebugFragment::from_log().to_pseudo_html(), + list_html(&["one", "three", "two", "four"]) ); + } + + /// A moved row keeps its subscription, so a later value change still lands in the + /// row's new position rather than where it used to be. + #[test] + fn updating_a_moved_row_replaces_content_in_place() { + let items = Value::new(vec![ + row(1, "one"), + row(2, "two"), + row(3, "three"), + row(4, "four"), + ]); + + let html = pseudo_html_after(&items, || { + items.set(vec![ + row(1, "one"), + row(3, "three"), + row(2, "two"), + row(4, "four"), + ]); + items.set(vec![ + row(1, "one"), + row(3, "three"), + row(2, "TWO"), + row(4, "four"), + ]); + }); + + assert_eq!(html, list_html(&["one", "three", "TWO", "four"])); + } + + /// The same guarantee one level down: moving a row whose node is a nested marker + /// must not rebuild the nested subtree either. + #[test] + fn moving_a_row_keeps_nested_content() { + let items = Value::new(vec![ + row(1, "one"), + row(2, "two"), + row(3, "three"), + row(4, "four"), + ]); + let content_renders = Rc::new(Cell::new(0)); + + log_start(); + let list = render_list(&items, |item| item.0, { + let content_renders = content_renders.clone(); + move |item| { + let label = item.map(|item| item.1); + let content_renders = content_renders.clone(); + item.render_value(move |_| { + let content_renders = content_renders.clone(); + label.render_value(move |label| { + content_renders.set(content_renders.get() + 1); + dom! {
  • {label}
  • } + }) + }) + } + }); + let _root = dom! {
      {list}
    }; + assert_eq!(content_renders.get(), 4); + + items.set(vec![ + row(1, "one"), + row(3, "three"), + row(2, "two"), + row(4, "four"), + ]); - assert_eq!(result.len(), 1); - assert_eq!(result[0].0.label, "new"); assert_eq!( - render_calls.get(), - 1, - "Expected rerender when item changes and key stays the same" + content_renders.get(), + 4, + "moving a row must not re-render its nested content" ); } + /// A row that occupies more than two siblings: the inner `render_value` adds + /// its own marker, so the row is `[anchor][content][inner marker][outer marker]`. #[test] - fn reuses_cached_node_when_item_value_is_unchanged() { - let item = Item { - id: 1, - label: "same".to_string(), - }; + fn nested_render_value_rows_reorder() { + let items = Value::new(vec![ + row(1, "one"), + row(2, "two"), + row(3, "three"), + row(4, "four"), + ]); + + log_start(); + let list = render_list( + &items, + |item| item.0, + |item| { + let label = item.map(|item| item.1); + item.render_value(move |_| label.render_value(|label| dom! {
  • {label}
  • })) + }, + ); + let _root = dom! {
      {list}
    }; + items.set(vec![ + row(1, "one"), + row(3, "three"), + row(2, "two"), + row(4, "four"), + ]); + + let rows = ["one", "three", "two", "four"] + .iter() + .map(|label| format!("
  • {label}
  • ")) + .collect::(); - let render_calls = Rc::new(ValueMut::new(0usize)); - let render_calls_for_closure = render_calls.clone(); + assert_eq!( + DomDebugFragment::from_log().to_pseudo_html(), + format!("
      {rows}
    ") + ); + } + + #[test] + fn renders_without_render_value_markers() { + let items = Value::new(vec![row(1, "one"), row(2, "two"), row(3, "three")]); + + log_start(); + let list = render_list( + &items, + |item| item.0, + |item| crate::transaction(|ctx| dom! {
  • {item.get(ctx).1.as_str()}
  • }), + ); + let _root = dom! {
      {list}
    }; - let render: Rc DomNode> = Rc::new(move |item| { - render_calls_for_closure.change(|count| *count += 1); - DomNode::from(item.label.clone()) + assert_eq!( + DomDebugFragment::from_log().to_pseudo_html(), + "
    • one
    • two
    • three
    " + ); + } + + #[test] + fn renders_empty_list_then_populates() { + let items = Value::new(Vec::<(u32, String)>::new()); + + let html = pseudo_html_after(&items, || { + items.set(vec![row(1, "one")]); }); - let old_node = render(&item); - let old_node_id = old_node.id_dom(); - render_calls.set(0); + assert_eq!(html, list_html(&["one"])); + } + + #[test] + fn skips_duplicate_keys() { + let items = Value::new(vec![row(1, "first"), row(1, "duplicate"), row(2, "two")]); + + assert_eq!(pseudo_html(&items), list_html(&["first", "two"])); + } + + #[test] + fn reuses_node_when_key_stays() { + let existing = Row::new(DomNode::from("same")); + let node_id = existing.node.id_dom(); + let render_calls = Rc::new(std::cell::Cell::new(0usize)); let result = reorder_nodes( DomId::from_u64(200), DomId::from_u64(201), - std::collections::VecDeque::from([(item.clone(), old_node)]), - std::collections::VecDeque::from([item]), - Rc::new(|item: &Item| item.id), - render, + std::collections::VecDeque::from([(1, existing)]), + std::collections::VecDeque::from([item(1, "same")]), + &{ + let render_calls = render_calls.clone(); + move |value: &Computed| { + render_calls.set(render_calls.get() + 1); + crate::transaction(|ctx| DomNode::from(value.get(ctx))) + } + }, ); assert_eq!(result.len(), 1); - assert_eq!(render_calls.get(), 0, "Expected cached DomNode reuse"); - assert_eq!( - result[0].1.id_dom(), - old_node_id, - "Expected to get the same DomNode from cache" + assert_eq!(render_calls.get(), 0); + assert_eq!(result[0].1.node.id_dom(), node_id); + } + + #[test] + fn renders_only_the_new_key() { + let existing = Row::new(DomNode::from("old")); + let existing_id = existing.node.id_dom(); + let render_calls = Rc::new(std::cell::Cell::new(0usize)); + + let result = reorder_nodes( + DomId::from_u64(100), + DomId::from_u64(101), + std::collections::VecDeque::from([(1, existing)]), + std::collections::VecDeque::from([item(1, "old"), item(2, "new")]), + &{ + let render_calls = render_calls.clone(); + move |value: &Computed| { + render_calls.set(render_calls.get() + 1); + crate::transaction(|ctx| DomNode::from(value.get(ctx))) + } + }, ); + + assert_eq!(result.len(), 2); + assert_eq!(result[0].1.node.id_dom(), existing_id); + assert_eq!(result[1].0, 2); + assert_eq!(render_calls.get(), 1); } } diff --git a/crates/vertigo/src/render/render_list_memo.rs b/crates/vertigo/src/render/render_list_memo.rs index a1268646..8efc3ccb 100644 --- a/crates/vertigo/src/render/render_list_memo.rs +++ b/crates/vertigo/src/render/render_list_memo.rs @@ -1,36 +1,21 @@ use std::rc::Rc; -use vertigo_macro::bind; use crate::{ - Computed, DomNode, DropResource, LazyCache, Value, - render::{collection::Collection, render_list}, + Computed, DomNode, LazyCache, Resource, Value, + render::{collection::CollectionKey, render_list}, }; /// Renders a reactive list from a `Value>>`, memoizing each item. /// -/// So that only items whose values actually changed are re-rendered. The list -/// automatically stays in sync with the source value and cleans up when dropped. -pub fn render_list_memo( +/// Thin wrapper around [`render_list`]: maps `Rc>` to `Vec` and uses +/// [`CollectionKey`](crate::CollectionKey) for identity. `render` is called once +/// per key with that item's stable [`Computed`](crate::Computed). +pub fn render_list_memo( value: &Value>>, render: impl Fn(&Computed) -> DomNode + 'static, ) -> DomNode { - let (collection, drop_synchronize) = value.synchronize::>(); - - let computed = collection.get(); - - let result = render_list( - computed, - |item| item.key.clone(), - move |item| render(&item.model), - ); - - result.append_drop_resource(drop_synchronize); - - result.append_drop_resource(DropResource::new(bind!(value, || { - drop(value); - }))); - - result + let items = value.to_computed().map(Rc::unwrap_or_clone); + render_list(items, T::get_key, render) } /// Renders a reactive list from a `LazyCache>`, memoizing each item. @@ -38,25 +23,15 @@ pub fn render_list_memo( /// So that only items whose values actually changed are re-rendered. Unlike /// `render_list_memo`, the source is a lazily-loaded cache (e.g. fetched from a /// remote resource), and the list updates whenever the cache is refreshed. -pub fn render_resource_list_memo( +/// +/// Loading / Error states are treated as an empty list. +pub fn render_resource_list_memo( value: &LazyCache>, render: impl Fn(&Computed) -> DomNode + 'static, ) -> DomNode { - let (collection, drop_event) = value.synchronize::>(); - - let computed = collection.get(); - - let result = render_list( - computed, - |item| item.key.clone(), - move |item| render(&item.model), - ); - - result.append_drop_resource(drop_event); - - result.append_drop_resource(DropResource::new(bind!(value, || { - drop(value); - }))); - - result + let items = value.to_computed().map(|resource| match resource { + Resource::Ready(list) => Rc::unwrap_or_clone(list), + Resource::Loading | Resource::Error(_) => Vec::new(), + }); + render_list(items, T::get_key, render) } diff --git a/crates/vertigo/src/render/render_value.rs b/crates/vertigo/src/render/render_value.rs index 3b1b0f69..1201c9a7 100644 --- a/crates/vertigo/src/render/render_value.rs +++ b/crates/vertigo/src/render/render_value.rs @@ -21,11 +21,12 @@ pub fn render_value_option( ) -> DomNode { let render = Rc::new(render); - DomComment::new_marker("v", move |parent_id, comment_id| { + DomComment::new_marker("v", move |parent_id, comment_id, content| { let current_node: ValueMut> = ValueMut::new(None); Some(computed.clone().subscribe({ let render = render.clone(); + let content = content.clone(); move |value| { let new_element = render(value).inspect(|new_element| { @@ -36,6 +37,8 @@ pub fn render_value_option( ); }); + content.set(new_element.iter().map(|node| node.id_dom()).collect()); + current_node.change(|current| { *current = new_element; }); diff --git a/crates/vertigo/src/tests/mod.rs b/crates/vertigo/src/tests/mod.rs index 47473955..db6821a8 100644 --- a/crates/vertigo/src/tests/mod.rs +++ b/crates/vertigo/src/tests/mod.rs @@ -7,4 +7,3 @@ mod css; mod dom; mod js_macro; mod jsjson_bytes; -mod repro_panic; diff --git a/crates/vertigo/src/tests/repro_panic.rs b/crates/vertigo/src/tests/repro_panic.rs deleted file mode 100644 index 5cec160d..00000000 --- a/crates/vertigo/src/tests/repro_panic.rs +++ /dev/null @@ -1,66 +0,0 @@ -use std::rc::Rc; -use vertigo::render::collection::{Collection, CollectionKey}; -use vertigo::{Value, transaction}; - -#[derive(Clone, PartialEq, Debug, Default)] -struct Item { - id: i32, - value: i32, -} - -struct ItemKey; -impl CollectionKey for ItemKey { - type Key = i32; - type Value = Item; - fn get_key(val: &Item) -> i32 { - val.id - } -} - -#[test] -fn test_collection_new_in_refresh() { - let val = Value::new(1); - - // Create a computed that creates a Collection inside its map - let comp = val.to_computed().map(|v| { - println!("Computing map for {}", v); - let list = Rc::new(vec![Item { id: v, value: 10 }]); - - // This should pass with the fix - let _col: Collection = Collection::new(list); - v - }); - - let _sub = comp.subscribe(|_| {}); - - println!("Triggering update..."); - transaction(|_| { - val.set(2); - }); - println!("Update triggered."); -} - -#[test] -fn test_duplicate_keys_in_refresh() { - let val = Value::new(1); - - let comp = val.to_computed().map(|v| { - println!("Computing map for {}", v); - // List with duplicate keys! - let list = Rc::new(vec![ - Item { id: 100, value: 1 }, - Item { id: 100, value: 2 }, // Duplicate key '100' - ]); - - let _col: Collection = Collection::new(list); - v - }); - - let _sub = comp.subscribe(|_| {}); - - println!("Triggering update..."); - transaction(|_| { - val.set(2); - }); - println!("Update triggered."); -} diff --git a/demo/app/src/app/chat/component.rs b/demo/app/src/app/chat/component.rs index 2987ac70..2906fe90 100644 --- a/demo/app/src/app/chat/component.rs +++ b/demo/app/src/app/chat/component.rs @@ -15,7 +15,7 @@ pub fn Chat(ws_chat: String) { |message| { dom! {
    - { message.clone() } + { message }
    } }, diff --git a/demo/app/src/app/dropfiles/mod.rs b/demo/app/src/app/dropfiles/mod.rs index 89bb9a6d..28274f3c 100644 --- a/demo/app/src/app/dropfiles/mod.rs +++ b/demo/app/src/app/dropfiles/mod.rs @@ -19,12 +19,14 @@ impl DropFiles { &state.list, |item| item.name.clone(), |file| { - let message = format_line(file); - dom! { -
    - { message } -
    - } + file.render_value(|file| { + let message = format_line(&file); + dom! { +
    + { message } +
    + } + }) }, ); diff --git a/demo/app/src/app/list/mod.rs b/demo/app/src/app/list/mod.rs index 0f3001d8..d8bd6e07 100644 --- a/demo/app/src/app/list/mod.rs +++ b/demo/app/src/app/list/mod.rs @@ -104,11 +104,13 @@ pub fn ListDemo() { padding: 5px; border: 1px solid #eee; "}; - dom! { -
    - {item} -
    - } + item.render_value(move |item| { + dom! { +
    + {item} +
    + } + }) }, ); diff --git a/demo/app/src/app/todo/select.rs b/demo/app/src/app/todo/select.rs index 07447502..db6882dc 100644 --- a/demo/app/src/app/todo/select.rs +++ b/demo/app/src/app/todo/select.rs @@ -40,10 +40,15 @@ where options, |item| item.to_string(), move |item| { - let text_item = item.to_string(); - let selected = is_selected(&value, item); - - dom! { } + item.render_value({ + let value = value.clone(); + move |item| { + let text_item = item.to_string(); + let selected = is_selected(&value, &item); + + dom! { } + } + }) }, ); diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 1f8e98b4..122b0f5c 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,6 +1,59 @@ +## 0.13.0 - unreleased + +Keyed list rendering was rebuilt around per-key `Computed`s. `render_list` and the memoized +list renderers changed shape, and the `Value::synchronize` machinery they used to rely on is +gone. See [`guides::collection_key_and_list_renderers`](https://docs.rs/vertigo/latest/vertigo/guides/collection_key_and_list_renderers/index.html). + +### Added + +* `keyed_computed_list` - maps a reactive list into a list of per-item `Computed`s, reusing the + same `Computed` for a given key across updates (Solid ``-style), together with `KeyedListItem` +* `MarkerContent` - lets a marker comment report the nodes it keeps in front of itself, so the + subtree travels with the marker instead of being rebuilt when it moves + +### Changed + +* **Breaking**: `render_list` now takes a `Vec` source (previously any + `IntoIterator + Clone + PartialEq`), its render closure receives `&Computed` instead of `&T`, + and the key type must implement `Debug`. The closure runs once per key *appearance*; item + updates flow through the per-key `Computed`, so embed it in `dom!` or wrap it with + `Computed::render_value` +* **Breaking**: the render closures of `render_list_memo` and `render_resource_list_memo` receive + `&Computed`. `render_resource_list_memo` renders `Loading` and `Error` as an empty list +* **Breaking**: the mount closure of `DomComment::new_marker` takes a third argument, + `&MarkerContent`. A marker moved within the same parent no longer re-runs its mount +* Every `render_list` row is preceded by an anchor comment node, which marks where the row begins + regardless of the shape the row renders to +* Guide `guides::value_synchronize_and_collections` replaced by + `guides::collection_key_and_list_renderers` + +### Removed + +* **Breaking**: `ValueSynchronize`, `Value::synchronize`, `LazyCache::synchronize` and + `CacheValue::synchronize`. `render_list_memo` no longer mirrors its source into a side + structure, so there is nothing left to synchronize; use `keyed_computed_list` to derive + per-item `Computed`s +* **Breaking**: `Collection` and `CollectionModel`, superseded by `keyed_computed_list`. + `CollectionKey` stays and still describes how list items are identified + +### Fixed + +* `render_list` corrupted sibling order when reordering or inserting rows whose root is a plain + element rather than a `render_value` marker +* Moving a row no longer destroys and rebuilds its DOM; the existing nodes are repositioned, so + their state (input values, listeners, children) survives a reorder +* Updating one row of a keyed list cost work proportional to the *square* of the list length, + because every row copied the whole shared key-to-value map on each update +* `Value::new` and `Value::set` no longer deep-copy the payload when nothing is listening for + `Value::add_event` +* `vertigo build` no longer fails wasm optimization with *"memory.copy operations require bulk + memory operations"* - the WASM features enabled by default for `wasm32-unknown-unknown` are now + passed to `wasm-opt` explicitly, because `strip = true` in the cargo profile removes the + `target_features` section that `wasm-opt` would otherwise read them from + ## 0.12.0 - 2026-07-01 ### Added diff --git a/package.json b/package.json index e22a01a5..67fea15d 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,7 @@ "rollup": "^4.60.1", "rollup-plugin-sourcemaps": "^0.6.3", "terser": "^5.46.1", - "tslib": "^2.8.1" + "tslib": "^2.8.1", + "typescript": "~6.0.3" } } diff --git a/tests/basic/src/lib.rs b/tests/basic/src/lib.rs index 392f8c87..19387619 100644 --- a/tests/basic/src/lib.rs +++ b/tests/basic/src/lib.rs @@ -48,22 +48,26 @@ pub fn app(state: AppState) -> DomNode { Mode::Div => render_list( &rows, |row| row.0.clone(), - |(key, label)| { - dom! { - - } + |row| { + row.render_value(|(key, label)| { + dom! { + + } + }) }, ), Mode::Div4 => render_list( &rows, |row| row.0.clone(), - |(key, label)| { - dom! { -
    -
    "Row"
    "Label"
    - -
    - } + |row| { + row.render_value(|(key, label)| { + dom! { +
    +
    "Row"
    "Label"
    + +
    + } + }) }, ), });