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