Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 110 additions & 0 deletions Abies.Tests/DomBehaviorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -508,4 +508,114 @@ private static Node UpdateElement(Node node, string targetId, Func<Element, Elem
}
return node;
}

#region Memo Tests

/// <summary>
/// Test data record for memoization tests.
/// </summary>
private record TestRow(int Id, string Label);

[Fact]
public void Memo_WithSameData_SkipsDiffing()
{
// Create two memo nodes with the same data
var row = new TestRow(1, "Test");
var element = new Element("row-1", "tr", [], new Text("t1", "Test"));

var oldNode = new Memo<TestRow>(row, element);
var newNode = new Memo<TestRow>(row, element);

var patches = Operations.Diff(oldNode, newNode);

// Should produce no patches because data is equal
Assert.Empty(patches);
}
Comment on lines +519 to +533
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memo_WithSameData_SkipsDiffing is not a strong regression test for memo diff-skipping: it diffs two Memo roots and reuses the same Element instance, which could yield no patches even without a correct memo implementation. Consider embedding memo nodes as children of a real root Element (matching production usage) and using distinct but equivalent inner node instances so the test fails if memo unwrapping/skipping is removed or broken.

Copilot uses AI. Check for mistakes.

[Fact]
public void Memo_WithDifferentData_ProducesDiff()
{
// Create two memo nodes with different data
var oldRow = new TestRow(1, "Old");
var newRow = new TestRow(1, "New");

var oldElement = new Element("row-1", "tr", [], new Text("t1", "Old"));
var newElement = new Element("row-1", "tr", [], new Text("t1", "New"));

var oldNode = new Memo<TestRow>(oldRow, oldElement);
var newNode = new Memo<TestRow>(newRow, newElement);

var patches = Operations.Diff(oldNode, newNode);

// Should produce patches because data changed
Assert.NotEmpty(patches);
Assert.Contains(patches, p => p is UpdateText);
}

[Fact]
public void Memo_RendersCorrectHtml()
{
var row = new TestRow(1, "Test");
var element = new Element("row-1", "tr", [], new Text("t1", "Test"));
var memoNode = new Memo<TestRow>(row, element);

var html = Render.Html(memoNode);

// Memo should render the inner element
Assert.Contains("row-1", html);
Assert.Contains("<tr", html);
Assert.Contains("Test", html);
}

[Fact]
public void Memo_GetKey_ReturnsInnerElementKey()
{
var row = new TestRow(1, "Test");
var element = new Element("row-1", "tr",
[new DOMAttribute("k1", "data-key", "custom-key")],
new Text("t1", "Test"));
var memoNode = new Memo<TestRow>(row, element);

// The key should be extracted from the inner element
// We can't test GetKey directly since it's private, but we can test
// that diffing works correctly with keyed memo nodes
var parent = new Element("p1", "tbody", [], memoNode);
var html = Render.Html(parent);

Assert.Contains("custom-key", html);
}

[Fact]
public void Memo_ListReorder_SkipsDiffingUnchangedItems()
{
// Simulate a list reorder where items don't change, just move
var rows = new[]
{
new TestRow(1, "First"),
new TestRow(2, "Second"),
new TestRow(3, "Third")
};

// Create memo-wrapped elements for old tree
var oldChildren = rows.Select((row, i) =>
(Node)new Memo<TestRow>(row, new Element($"row-{row.Id}", "tr", [], new Text($"t{row.Id}", row.Label)))
).ToArray();
var oldTree = new Element("tbody", "tbody", [], oldChildren);

// Reorder: swap first and last
var reorderedRows = new[] { rows[2], rows[1], rows[0] };
var newChildren = reorderedRows.Select((row, i) =>
(Node)new Memo<TestRow>(row, new Element($"row-{row.Id}", "tr", [], new Text($"t{row.Id}", row.Label)))
).ToArray();
var newTree = new Element("tbody", "tbody", [], newChildren);

var patches = Operations.Diff(oldTree, newTree);

// The memo optimization means we should NOT see UpdateText patches
// because the data is equal and diffing was skipped for the inner content.
// We may see structural patches (AddChild, RemoveChild) for reordering.
Assert.DoesNotContain(patches, p => p is UpdateText);
}
Comment on lines +588 to +618
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memo_ListReorder_SkipsDiffingUnchangedItems currently only asserts the absence of UpdateText patches. With the current diff algorithm, a reorder produces only structural patches (or, if memo children are accidentally ignored, no patches at all), so this test can pass even when memo support is broken. Please assert the expected structural behavior too (e.g., patches contain the expected Add/Remove/Move operations, or apply patches and compare final HTML).

Copilot uses AI. Check for mistakes.

#endregion
}
120 changes: 120 additions & 0 deletions Abies/DOM/Operations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,50 @@ public record Text(string Id, string Value) : Node(Id)
/// </summary>
public record Empty() : Node("");

/// <summary>
/// Interface for memoized nodes to enable non-generic data comparison.
/// </summary>
public interface IMemoNode
{
/// <summary>
/// The underlying node that will be rendered.
/// </summary>
Node InnerNode { get; }

/// <summary>
/// Checks if this memo node has the same data as another memo node.
/// Returns false if the other node is not a compatible IMemoNode.
/// </summary>
bool HasSameData(IMemoNode other);
Comment on lines +286 to +297
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Render.Html now unwraps IMemoNode, but other framework traversals still operate only on Element (e.g., handler registration/unregistration and ID preservation). As a result, memo-wrapped subtrees can render handler attributes into HTML but never get their handlers registered, breaking event dispatch. Memo support should include unwrapping in all DOM-walkers that assume Node is an Element tree.

Copilot uses AI. Check for mistakes.
}

/// <summary>
/// Represents a memoized node in the Abies DOM.
/// The node is only re-rendered when the underlying data changes.
/// This is similar to React.memo or Elm's lazy.
/// </summary>
/// <typeparam name="T">The type of the data used to create the node.</typeparam>
/// <param name="Data">The data that was used to create the wrapped node.</param>
/// <param name="InnerNode">The actual DOM node (will be rendered lazily or reused).</param>
/// <remarks>
/// When diffing, if two Memo nodes have equal Data (via Equals),
/// the inner nodes are assumed to be identical and no diff is performed.
/// This dramatically reduces diffing overhead for large lists of stable items.
Comment on lines +302 to +311
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memo<T> XML docs claim the node is "only re-rendered when the underlying data changes" and that the inner node "will be rendered lazily or reused", but the current API always constructs InnerNode eagerly (e.g., Elements.memo(data, render) calls render(data) every render). Please adjust the documentation to match actual behavior (skips diffing only), or change the implementation so it actually avoids re-rendering/allocation when data is equal.

Suggested change
/// The node is only re-rendered when the underlying data changes.
/// This is similar to React.memo or Elm's lazy.
/// </summary>
/// <typeparam name="T">The type of the data used to create the node.</typeparam>
/// <param name="Data">The data that was used to create the wrapped node.</param>
/// <param name="InnerNode">The actual DOM node (will be rendered lazily or reused).</param>
/// <remarks>
/// When diffing, if two Memo nodes have equal Data (via Equals),
/// the inner nodes are assumed to be identical and no diff is performed.
/// This dramatically reduces diffing overhead for large lists of stable items.
/// Memo nodes allow the diff algorithm to skip comparing inner nodes when the underlying data is unchanged.
/// This is conceptually similar to React.memo or Elm's lazy diffing semantics.
/// </summary>
/// <typeparam name="T">The type of the data used to create the node.</typeparam>
/// <param name="Data">The data that was used to create the wrapped node.</param>
/// <param name="InnerNode">The actual DOM node associated with this memoized value.</param>
/// <remarks>
/// When diffing, if two Memo nodes have equal <paramref name="Data"/> (via <see cref="object.Equals(object?, object?)"/>),
/// the inner nodes are assumed to be equivalent and no diff is performed between them.
/// This reduces diffing overhead for large lists of stable items, but does not by itself control when or how
/// <paramref name="InnerNode"/> instances are created.

Copilot uses AI. Check for mistakes.
/// </remarks>
public record Memo<T>(T Data, Node InnerNode) : Node(InnerNode.Id), IMemoNode where T : notnull
{
Node IMemoNode.InnerNode => InnerNode;

bool IMemoNode.HasSameData(IMemoNode other)
{
if (other is Memo<T> otherMemo)
{
return EqualityComparer<T>.Default.Equals(Data, otherMemo.Data);
}
return false;
}
}

/// <summary>
/// Represents a patch operation in the Abies DOM.
/// </summary>
Expand Down Expand Up @@ -602,6 +646,10 @@ private static void RenderNode(Node node, StringBuilder sb)
{
switch (node)
{
case IMemoNode memo:
// Unwrap memo and render the inner node
RenderNode(memo.InnerNode, sb);
break;
case Element element:
sb.Append('<').Append(element.Tag).Append(" id=\"").Append(element.Id).Append('"');
foreach (var attr in element.Attributes)
Expand Down Expand Up @@ -1082,6 +1130,26 @@ public static List<Patch> Diff(Node? oldNode, Node newNode)

private static void DiffInternal(Node oldNode, Node newNode, Element? parent, List<Patch> patches)
{
// =============================================================================
// Memo Node Optimization - Skip diffing when data hasn't changed
// =============================================================================
// If both nodes are Memo<T> with the same T, and their Data is equal,
// we can skip diffing entirely because the rendered content is guaranteed
// to be identical (pure rendering function).
// =============================================================================

// Unwrap Memo nodes, checking for data equality
var (unwrappedOld, unwrappedNew, skipDiff) = UnwrapMemoNodes(oldNode, newNode);
if (skipDiff)
{
// Data is equal - no changes needed, skip diffing completely
return;
}

// Continue with unwrapped nodes
oldNode = unwrappedOld;
newNode = unwrappedNew;

// Text nodes only need an update when the value changes
if (oldNode is Text oldText && newNode is Text newText)
{
Expand Down Expand Up @@ -1686,6 +1754,12 @@ private static void BuildKeySequenceInto(Node[] children, string[] keys)
/// </summary>
private static string? GetKey(Node node)
{
// Unwrap Memo nodes to get the inner node's key
if (node is IMemoNode memo)
{
node = memo.InnerNode;
}

Comment on lines +1757 to +1762
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetKey unwraps a memo node, but DiffChildrenCore still treats Node values as Element/RawHtml/Text when generating Add/Remove patches. If list children are Memo<T> (the expected usage), the reorder/membership paths will generate no structural patches because memo nodes won't match those type checks, leaving the real DOM out of sync. Consider unwrapping memo nodes (recursively) before all child-type switches used for patch creation (remove/add/diff), e.g. via a shared UnwrapMemo(Node) helper.

Copilot uses AI. Check for mistakes.
if (node is not Element element)
{
return null;
Expand All @@ -1711,4 +1785,50 @@ private static void BuildKeySequenceInto(Node[] children, string[] keys)
// Element IDs are always unique, making them ideal for keyed diffing
return element.Id;
}

/// <summary>
/// Unwraps Memo nodes and checks if their data is equal.
/// Returns the unwrapped nodes and a flag indicating if diffing can be skipped.
/// </summary>
/// <returns>
/// A tuple of (unwrappedOld, unwrappedNew, skipDiff) where:
/// - unwrappedOld: The old node with Memo wrapper removed if present
/// - unwrappedNew: The new node with Memo wrapper removed if present
/// - skipDiff: True if both nodes were Memo with equal Data, meaning no diff needed
/// </returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static (Node unwrappedOld, Node unwrappedNew, bool skipDiff) UnwrapMemoNodes(Node oldNode, Node newNode)
{
// Fast path: neither is a Memo node
if (oldNode is not IMemoNode && newNode is not IMemoNode)
{
return (oldNode, newNode, false);
}

// Both are Memo nodes - check if data is equal
if (oldNode is IMemoNode oldMemo && newNode is IMemoNode newMemo)
{
if (oldMemo.HasSameData(newMemo))
{
// Data is equal - skip diffing entirely
return (oldMemo.InnerNode, newMemo.InnerNode, true);
}
Comment on lines +1813 to +1815
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memo diff-skipping currently depends only on HasSameData(). If Data is equal but the InnerNode's type/tag/id differs (or for Text/RawHtml, if the id changes), skipping the diff can leave the real DOM inconsistent because no Replace/Update patches will be emitted. To keep correctness, require at least InnerNode.Id equality and the same node shape/type before returning skipDiff=true (otherwise unwrap and continue diffing).

Suggested change
// Data is equal - skip diffing entirely
return (oldMemo.InnerNode, newMemo.InnerNode, true);
}
// Data is equal - only skip diffing if the inner node identity and shape are also stable.
// This ensures the real DOM remains consistent even when memo data stays the same
// but the underlying node type or Id would otherwise require a Replace/Update patch.
var oldInner = oldMemo.InnerNode;
var newInner = newMemo.InnerNode;
// Id must be equal for safe diff skipping.
if (!string.Equals(oldInner.Id, newInner.Id, StringComparison.Ordinal))
{
return (oldInner, newInner, false);
}
// Node shape/type must be the same to avoid mismatched DOM structures.
if (oldInner.GetType() != newInner.GetType())
{
return (oldInner, newInner, false);
}
// Data, Id, and node shape are all equal - safe to skip diffing entirely.
return (oldInner, newInner, true);
}

Copilot uses AI. Check for mistakes.
// Data changed - need to diff the inner nodes
return (oldMemo.InnerNode, newMemo.InnerNode, false);
}

// Only one is Memo - unwrap it and continue diffing
if (oldNode is IMemoNode oldMemoOnly)
{
return (oldMemoOnly.InnerNode, newNode, false);
}

if (newNode is IMemoNode newMemoOnly)
{
return (oldNode, newMemoOnly.InnerNode, false);
}

// Shouldn't reach here, but return unchanged
return (oldNode, newNode, false);
Comment on lines +1802 to +1832
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UnwrapMemoNodes only unwraps a single memo layer. If a memo node wraps another memo node (easy to do via the memo(data, Node node) overload), DiffInternal will continue with oldNode/newNode still being memo nodes and then fall through without producing correct patches. Please unwrap memo nodes recursively/iteratively until the underlying non-memo Node is reached.

Suggested change
// Fast path: neither is a Memo node
if (oldNode is not IMemoNode && newNode is not IMemoNode)
{
return (oldNode, newNode, false);
}
// Both are Memo nodes - check if data is equal
if (oldNode is IMemoNode oldMemo && newNode is IMemoNode newMemo)
{
if (oldMemo.HasSameData(newMemo))
{
// Data is equal - skip diffing entirely
return (oldMemo.InnerNode, newMemo.InnerNode, true);
}
// Data changed - need to diff the inner nodes
return (oldMemo.InnerNode, newMemo.InnerNode, false);
}
// Only one is Memo - unwrap it and continue diffing
if (oldNode is IMemoNode oldMemoOnly)
{
return (oldMemoOnly.InnerNode, newNode, false);
}
if (newNode is IMemoNode newMemoOnly)
{
return (oldNode, newMemoOnly.InnerNode, false);
}
// Shouldn't reach here, but return unchanged
return (oldNode, newNode, false);
// Iteratively unwrap memo nodes on both sides until we reach the underlying non-memo nodes.
// While unwrapping, preserve the optimization of skipping the diff when both memo nodes
// report equal data via HasSameData.
var currentOld = oldNode;
var currentNew = newNode;
while (true)
{
// Both are Memo nodes - check if data is equal
if (currentOld is IMemoNode oldMemo && currentNew is IMemoNode newMemo)
{
if (oldMemo.HasSameData(newMemo))
{
// Data is equal - skip diffing entirely, but still return the inner nodes
return (oldMemo.InnerNode, newMemo.InnerNode, true);
}
// Data changed - unwrap one level on both sides and continue
currentOld = oldMemo.InnerNode;
currentNew = newMemo.InnerNode;
continue;
}
// Only the old node is a Memo - unwrap and continue
if (currentOld is IMemoNode oldMemoOnly)
{
currentOld = oldMemoOnly.InnerNode;
continue;
}
// Only the new node is a Memo - unwrap and continue
if (currentNew is IMemoNode newMemoOnly)
{
currentNew = newMemoOnly.InnerNode;
continue;
}
// Neither side is a Memo node anymore - we're fully unwrapped
return (currentOld, currentNew, false);
}

Copilot uses AI. Check for mistakes.
}
}
29 changes: 29 additions & 0 deletions Abies/Html/Elements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -517,4 +517,33 @@ public static Node rect(DOM.Attribute[] attributes, [UniqueId(UniqueIdFormat.Htm
public static Node raw(string html, [UniqueId(UniqueIdFormat.HtmlId)] string? id = null)
=> new RawHtml(id!, html);

/// <summary>
/// Creates a memoized node that skips diffing when the data hasn't changed.
/// Use this for list items or components that don't change frequently.
/// </summary>
/// <typeparam name="T">The type of the data (must implement proper Equals).</typeparam>
/// <param name="data">The data that determines when to re-render.</param>
/// <param name="render">A function that renders the data to a Node.</param>
/// <returns>A memoized node that only diffs when data changes.</returns>
/// <example>
/// // Without memo: diffs every row on every render
/// model.Rows.Select(row => TableRow(row))
///
/// // With memo: only diffs rows whose data changed
/// model.Rows.Select(row => memo(row, r => TableRow(r)))
Comment on lines +528 to +533
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The XML doc example is not valid XML documentation markup as-is (plain code/comments inside <example>). Consider wrapping the sample in <code> (and escaping </> if present) so it renders correctly in generated docs/IntelliSense.

Suggested change
/// <example>
/// // Without memo: diffs every row on every render
/// model.Rows.Select(row => TableRow(row))
///
/// // With memo: only diffs rows whose data changed
/// model.Rows.Select(row => memo(row, r => TableRow(r)))
/// <example>
/// <code>
/// // Without memo: diffs every row on every render
/// model.Rows.Select(row => TableRow(row))
///
/// // With memo: only diffs rows whose data changed
/// model.Rows.Select(row => memo(row, r => TableRow(r)))
/// </code>

Copilot uses AI. Check for mistakes.
/// </example>
public static Node memo<T>(T data, Func<T, Node> render) where T : notnull
=> new Memo<T>(data, render(data));

/// <summary>
/// Creates a memoized node that skips diffing when the data hasn't changed.
/// Overload for already-rendered nodes where you want to track data separately.
/// </summary>
/// <typeparam name="T">The type of the data (must implement proper Equals).</typeparam>
/// <param name="data">The data that determines when to re-render.</param>
/// <param name="node">The pre-rendered node to wrap.</param>
/// <returns>A memoized node that only diffs when data changes.</returns>
public static Node memo<T>(T data, Node node) where T : notnull
=> new Memo<T>(data, node);

}
Loading