From a1195bc87be203fb39eaec47a3cfba86b79a3001 Mon Sep 17 00:00:00 2001 From: mrdotb Date: Sun, 30 Aug 2026 01:02:31 +0200 Subject: [PATCH] fix: derive dead render state without requiring socket `dead` was computed as: assigns[:socket] == nil or not LiveView.connected?(assigns[:socket]) so a call site that did not pass `socket` looked dead on *every* render, not just the first. With props diffing enabled that silently breaks updates: * `full_props?` stays true, so `data-props` is resent -- but `base_assigns` is still filtered to changed keys, so the snapshot degrades to only the props that changed. * `props_diff: not full_props?` stays false, so `data-props-diff` is never marked as changed and freezes at its mount value. The client is in diff mode (`data-use-diff="true"`), reads only the frozen diff attribute, and the component silently stops reflecting updates. Nothing raises and nothing is logged. A populated `__changed__` means LiveView is re-rendering the component, which only happens in a connected view, so a render can only be dead on the first pass. That is also the only place `dead` is used for anything other than props: the SSR decision is already gated on `init`, so this leaves SSR behaviour byte-for-byte identical. The existing props-diff tests could not catch this: they render the component directly and read the attributes off the HTML, where every attribute is always present. Marking only decides what LiveView puts on the wire. The new test asks the Rendered struct for its dynamic parts with change tracking enabled, so an attribute LiveView would skip comes back as nil -- and it fails without this change. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 4 ++ lib/live_react.ex | 13 ++++- test/live_react_change_tracking_test.exs | 69 ++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 test/live_react_change_tracking_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index b828b71e..82866008 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ See [Conventional Commits](Https://conventionalcommits.org) for commit guideline * Props are now diffed and sent incrementally over `data-props-diff` instead of being fully re-sent on every update (`config :live_react, enable_props_diff: true` by default; opt out globally with `false` or per-component with `diff={false}`). * Added support for `Phoenix.LiveView.stream/3,4` assigns: any `%Phoenix.LiveView.LiveStream{}` value passed as a prop is now automatically diffed and delivered over `data-streams-diff`. +### Bug Fixes: + +* Props diffing no longer requires the call site to pass `socket`. A render was previously treated as dead whenever `socket` was absent, which forced the full-props branch on every update: `data-props` was resent carrying only the *changed* keys, while `data-props-diff` was never marked as changed. A client in diff mode therefore read a diff attribute that never moved and silently stopped updating. `dead` is now only derived on the initial render, where it is both knowable and the only place it is used. + ## [v1.1.0](https://github.com/mrdotb/live_react/compare/v1.0.1...v1.1.0) (2025-06-22) ### Features: diff --git a/lib/live_react.ex b/lib/live_react.ex index 27c33538..c1819f17 100644 --- a/lib/live_react.ex +++ b/lib/live_react.ex @@ -44,7 +44,18 @@ defmodule LiveReact do # Flags derived from the assigns that drive how the component is rendered. defp render_flags(assigns) do init = assigns.__changed__ == nil - dead = assigns[:socket] == nil or not LiveView.connected?(assigns[:socket]) + + # A populated `__changed__` means LiveView is re-rendering this component, + # which only ever happens in a connected view. So a render can only be dead + # on the very first pass -- and that is also the only place `dead` matters, + # since the SSR decision below is already gated on `init`. + # + # Deriving it this way means a call site that does not pass `socket` still + # gets correct props diffing. Previously such a call site looked dead on + # every render, which sent a *partial* snapshot in `data-props` while never + # marking `data-props-diff` as changed -- so the client, being in diff mode, + # read a diff attribute that never moved and silently stopped updating. + dead = init and (assigns[:socket] == nil or not LiveView.connected?(assigns[:socket])) %{ init: init, diff --git a/test/live_react_change_tracking_test.exs b/test/live_react_change_tracking_test.exs new file mode 100644 index 00000000..dd7380e4 --- /dev/null +++ b/test/live_react_change_tracking_test.exs @@ -0,0 +1,69 @@ +defmodule LiveReactChangeTrackingTest do + @moduledoc """ + Covers which attributes LiveView actually *sends* on a re-render. + + The other props-diff tests render the component directly and read the + attributes off the resulting HTML, where every attribute is always present. + That cannot catch a bug in `mark_computed_changed/3`, because marking only + decides what LiveView puts on the wire. Here we ask the `Rendered` struct for + its dynamic parts with change tracking enabled, so an attribute LiveView + would skip comes back as `nil`. + """ + use ExUnit.Case + + import Phoenix.Component + + # The dynamic parts LiveView would actually send, change tracking enabled. + defp sent(assigns) do + %Phoenix.LiveView.Rendered{dynamic: dynamic} = LiveReact.react(assigns) + + dynamic.(true) + |> Enum.reject(&is_nil/1) + |> Enum.map(&IO.iodata_to_binary/1) + end + + # `data-props` carries a full JSON object; `Patch.encode_object/1` swaps `"` for `^`. + defp snapshot?(value), do: String.starts_with?(value, "{^") + defp patch_for_items?(value), do: String.contains?(value, "/items") + + defp updated_assigns(extra) do + extra + |> Map.merge(%{name: "T", items: [%{id: 1, title: "a"}], __changed__: %{}}) + |> assign(:items, [%{id: 1, title: "b"}]) + end + + describe "connected re-render" do + test "sends the diff and does not resend the props snapshot" do + socket = %Phoenix.LiveView.Socket{transport_pid: self()} + + sent = sent(updated_assigns(%{socket: socket})) + + assert Enum.any?(sent, &patch_for_items?/1) + refute Enum.any?(sent, &snapshot?/1) + end + + test "sends the diff even when the call site does not pass socket" do + # Regression: `dead` used to be true whenever `socket` was absent, which + # forced the full-props branch forever. `data-props` was resent carrying + # only the *changed* keys while `data-props-diff` was never marked + # changed, so a client in diff mode never saw another update. + sent = sent(updated_assigns(%{})) + + assert Enum.any?(sent, &patch_for_items?/1) + refute Enum.any?(sent, &snapshot?/1) + end + end + + describe "first render" do + test "sends the full props snapshot, with or without a socket" do + for extra <- [%{}, %{socket: nil}] do + assigns = Map.merge(extra, %{name: "T", items: [%{id: 1}], __changed__: nil}) + + sent = sent(assigns) + + assert Enum.any?(sent, &snapshot?/1) + refute Enum.any?(sent, &patch_for_items?/1) + end + end + end +end