Force remove stream elements on join patch (#3730)

* force remove stream elements on join patch

* add implicit test

* check owner before removing elements

Author: SteffenDE

assets/js/phoenix_live_view/dom_patch.js

@@ -298,11 +298,19 @@ export default class DOMPatch {
 
       // clear stream items from the dead render if they are not inserted again
       if(isJoinPatch){
-        DOM.all(this.container, `[${phxUpdate}=${PHX_STREAM}]`, el => {
-          Array.from(el.children).forEach(child => {
-            this.removeStreamChildElement(child)
+        DOM.all(this.container, `[${phxUpdate}=${PHX_STREAM}]`)
+          // it is important to filter the element before removing them, as
+          // it may happen that streams are nested and the owner check fails if
+          // a parent is removed before a child
+          .filter(el => this.view.ownsElement(el))
+          .forEach(el => {
+            Array.from(el.children).forEach(child => {
+              // we already performed the owner check, each child is guaranteed to be owned
+              // by the view. To prevent the nested owner check from failing in case of nested
+              // streams where the parent is removed before the child, we force the removal
+              this.removeStreamChildElement(child, true)
+            })
           })
-        })
       }
 
       morph.call(this, targetContainer, html)
@@ -356,11 +364,11 @@ export default class DOMPatch {
     }
   }
 
-  removeStreamChildElement(child){
+  removeStreamChildElement(child, force=false){
     // make sure to only remove elements owned by the current view
     // see https://github.com/phoenixframework/phoenix_live_view/issues/3047
     // and https://github.com/phoenixframework/phoenix_live_view/issues/3681
-    if(!this.view.ownsElement(child)){ return }
+    if(!force && !this.view.ownsElement(child)){ return }
 
     // we need to store the node if it is actually re-added in the same patch
     // we do NOT want to execute phx-remove, we do NOT want to call onNodeDiscarded

lib/phoenix_live_view/test/client_proxy.ex

@@ -91,6 +91,9 @@ defmodule Phoenix.LiveViewTest.ClientProxy do
     # because the live_module assign was set.
     root_html = DOM.parse(response_html, fn msg -> send(self(), {:test_error, msg}) end)
 
+    # clear stream elements from static render
+    root_html = DOM.remove_stream_children(root_html)
+
     {id, session_token, static_token, redirect_url} =
       case Map.fetch(opts, :live_redirect) do
         {:ok, {id, session_token, static_token}} ->

lib/phoenix_live_view/test/dom.ex

@@ -255,6 +255,19 @@ defmodule Phoenix.LiveViewTest.DOM do
     |> elem(1)
   end
 
+  @doc """
+  Removes stream children from the given HTML tree.
+  """
+  def remove_stream_children(html_tree) do
+    walk(html_tree, fn {tag, attrs, children} = node ->
+      if attribute(node, "phx-update") == "stream" do
+        {tag, attrs, []}
+      else
+        {tag, attrs, children}
+      end
+    end)
+  end
+
   # Diff merging
 
   def merge_diff(rendered, diff) do

test/support/live_views/streams.ex

@@ -712,18 +712,26 @@ defmodule Phoenix.LiveViewTest.Support.StreamNestedComponentResetLive do
 
     # first mount
     def update(assigns, socket) do
+      items =
+        if connected?(socket) do
+          [
+            %{id: assigns.id <> "-a", name: "N-A"},
+            %{id: assigns.id <> "-b", name: "N-B"},
+            %{id: assigns.id <> "-c", name: "N-C"},
+            %{id: assigns.id <> "-d", name: "N-D"}
+          ]
+        else
+          [
+            %{id: assigns.id <> "-e", name: "N-E"},
+            %{id: assigns.id <> "-f", name: "N-F"},
+            %{id: assigns.id <> "-g", name: "N-G"},
+            %{id: assigns.id <> "-h", name: "N-H"}
+          ]
+        end
+
       socket
       |> assign(assigns)
-      |> stream(
-        :nested,
-        [
-          %{id: assigns.id <> "-a", name: "N-A"},
-          %{id: assigns.id <> "-b", name: "N-B"},
-          %{id: assigns.id <> "-c", name: "N-C"},
-          %{id: assigns.id <> "-d", name: "N-D"}
-        ],
-        reset: true
-      )
+      |> stream(:nested, items, reset: true)
       |> then(&{:ok, &1})
     end