Fix rendering bug when component is added back. Closes #3169

assets/js/phoenix_live_view/dom.js

@@ -138,20 +138,27 @@ let DOM = {
     return this.all(el, `${PHX_VIEW_SELECTOR}[${PHX_PARENT_ID}="${parentId}"]`)
   },
 
-  findParentCIDs(node, cids){
-    let initial = new Set(cids)
-    let parentCids =
-      cids.reduce((acc, cid) => {
-        let selector = `[${PHX_COMPONENT}="${cid}"] [${PHX_COMPONENT}]`
-
-        this.filterWithinSameLiveView(this.all(node, selector), node)
+  findExistingParentCIDs(node, cids){
+    // we only want to find parents that exist on the page
+    // if a cid is not on the page, the only way it can be added back to the page
+    // is if a parent adds it back, therefore if a cid does not exist on the page,
+    // we should not try to render it by itself (because it would be rendered twice,
+    // one by the parent, and a second time by itself)
+    let parentCids = new Set()
+    let childrenCids = new Set()
+
+    cids.forEach(cid => {
+      this.filterWithinSameLiveView(this.all(node, `[${PHX_COMPONENT}="${cid}"]`), node).forEach(parent => {
+        parentCids.add(cid)
+        this.all(parent, `[${PHX_COMPONENT}]`)
           .map(el => parseInt(el.getAttribute(PHX_COMPONENT)))
-          .forEach(childCID => acc.delete(childCID))
+          .forEach(childCID => childrenCids.add(childCID))
+      })
+    })
 
-        return acc
-      }, initial)
+    childrenCids.forEach(childCid => parentCids.delete(childCid))
 
-    return parentCids.size === 0 ? new Set(cids) : parentCids
+    return parentCids
   },
 
   filterWithinSameLiveView(nodes, parent){

assets/js/phoenix_live_view/view.js

@@ -544,7 +544,7 @@ export default class View {
     // Otherwise, patch entire LV container.
     if(this.rendered.isComponentOnlyDiff(diff)){
       this.liveSocket.time("component patch complete", () => {
-        let parentCids = DOM.findParentCIDs(this.el, this.rendered.componentCIDs(diff))
+        let parentCids = DOM.findExistingParentCIDs(this.el, this.rendered.componentCIDs(diff))
         parentCids.forEach(parentCID => {
           if(this.componentPatch(this.rendered.getComponent(diff, parentCID), parentCID)){ phxChildrenAdded = true }
         })

assets/test/dom_test.js

@@ -120,7 +120,7 @@ describe("DOM", () => {
     })
   })
 
-  describe("findParentCIDs", () => {
+  describe("findExistingParentCIDs", () => {
     test("returns only parent cids", () => {
       let view = tag("div", {}, `
         <div data-phx-main="true"
@@ -133,19 +133,17 @@ describe("DOM", () => {
       `)
       document.body.appendChild(view)
 
-      expect(DOM.findParentCIDs(view, [1, 2, 3])).toEqual(new Set([1, 2, 3]))
-
       view.appendChild(tag("div", {"data-phx-component": 1}, `
         <div data-phx-component="2"></div>
       `))
-      expect(DOM.findParentCIDs(view, [1, 2, 3])).toEqual(new Set([1, 3]))
+      expect(DOM.findExistingParentCIDs(view, [1, 2])).toEqual(new Set([1]))
 
       view.appendChild(tag("div", {"data-phx-component": 1}, `
         <div data-phx-component="2">
           <div data-phx-component="3"></div>
         </div>
       `))
-      expect(DOM.findParentCIDs(view, [1, 2, 3])).toEqual(new Set([1]))
+      expect(DOM.findExistingParentCIDs(view, [1, 2, 3])).toEqual(new Set([1]))
     })
   })
 

priv/static/phoenix_live_view.js

@@ -729,14 +729,17 @@ var LiveView = (() => {
     findPhxChildren(el, parentId) {
       return this.all(el, `${PHX_VIEW_SELECTOR}[${PHX_PARENT_ID}="${parentId}"]`);
     },
-    findParentCIDs(node, cids) {
-      let initial = new Set(cids);
-      let parentCids = cids.reduce((acc, cid) => {
-        let selector = `[${PHX_COMPONENT}="${cid}"] [${PHX_COMPONENT}]`;
-        this.filterWithinSameLiveView(this.all(node, selector), node).map((el) => parseInt(el.getAttribute(PHX_COMPONENT))).forEach((childCID) => acc.delete(childCID));
-        return acc;
-      }, initial);
-      return parentCids.size === 0 ? new Set(cids) : parentCids;
+    findExistingParentCIDs(node, cids) {
+      let parentCids = new Set();
+      let childrenCids = new Set();
+      cids.forEach((cid) => {
+        this.filterWithinSameLiveView(this.all(node, `[${PHX_COMPONENT}="${cid}"]`), node).forEach((parent) => {
+          parentCids.add(cid);
+          this.all(parent, `[${PHX_COMPONENT}]`).map((el) => parseInt(el.getAttribute(PHX_COMPONENT))).forEach((childCID) => childrenCids.add(childCID));
+        });
+      });
+      childrenCids.forEach((childCid) => parentCids.delete(childCid));
+      return parentCids;
     },
     filterWithinSameLiveView(nodes, parent) {
       if (parent.querySelector(PHX_VIEW_SELECTOR)) {
@@ -3358,7 +3361,7 @@ removing illegal node: "${(childNode.outerHTML || childNode.nodeValue).trim()}"
       let phxChildrenAdded = false;
       if (this.rendered.isComponentOnlyDiff(diff)) {
         this.liveSocket.time("component patch complete", () => {
-          let parentCids = dom_default.findParentCIDs(this.el, this.rendered.componentCIDs(diff));
+          let parentCids = dom_default.findExistingParentCIDs(this.el, this.rendered.componentCIDs(diff));
           parentCids.forEach((parentCID) => {
             if (this.componentPatch(this.rendered.getComponent(diff, parentCID), parentCID)) {
               phxChildrenAdded = true;

test/e2e/playwright.config.js

@@ -10,12 +10,12 @@ const config = {
   use: {
     trace: "retain-on-failure",
     screenshot: "only-on-failure",
-    baseURL: "http://localhost:4000/",
+    baseURL: "http://localhost:4004/",
     ignoreHTTPSErrors: true,
   },
   webServer: {
     command: "npm run e2e:server",
-    url: "http://127.0.0.1:4000/health",
+    url: "http://127.0.0.1:4004/health",
     reuseExistingServer: !process.env.CI,
     stdout: "pipe",
     stderr: "pipe",

test/e2e/test_helper.exs

@@ -1,5 +1,5 @@
 Application.put_env(:phoenix_live_view, Phoenix.LiveViewTest.E2E.Endpoint,
-  http: [ip: {127, 0, 0, 1}, port: 4000],
+  http: [ip: {127, 0, 0, 1}, port: 4004],
   # TODO: switch to bandit when Phoenix 1.7 is used
   # adapter: Bandit.PhoenixAdapter,
   server: true,
@@ -98,6 +98,7 @@ defmodule Phoenix.LiveViewTest.E2E.Router do
     live "/2965", Issue2965Live
     live "/3047/a", Issue3047ALive
     live "/3047/b", Issue3047BLive
+    live "/3169", Issue3169Live
   end
 end
 
@@ -143,4 +144,5 @@ end
     strategy: :one_for_one
   )
 
+IO.puts "Starting e2e server on port #{Phoenix.LiveViewTest.E2E.Endpoint.config(:http)[:port]}"
 Process.sleep(:infinity)

test/e2e/tests/issues/3169.spec.js

@@ -0,0 +1,26 @@
+const { test, expect } = require("@playwright/test");
+const { syncLV } = require("../../utils");
+
+const inputVals = async (page) => {
+  return page.locator(`input[type="text"]`).evaluateAll(list => list.map(i => i.value));
+}
+
+test("updates which add cids back on page are properly magic id change tracked", async ({ page }) => {
+  await page.goto("/issues/3169");
+  await syncLV(page);
+
+  await page.locator("#select-a").click()
+  await syncLV(page);
+  await expect(page.locator("body")).toContainText("FormColumn (c3)");
+  await expect(await inputVals(page)).toEqual(["Record a", "Record a", "Record a"]);
+
+  await page.locator("#select-b").click()
+  await syncLV(page);
+  await expect(page.locator("body")).toContainText("FormColumn (c3)");
+  await expect(await inputVals(page)).toEqual(["Record b", "Record b", "Record b"]);
+
+  await page.locator("#select-z").click()
+  await syncLV(page);
+  await expect(page.locator("body")).toContainText("FormColumn (c3)");
+  await expect(await inputVals(page)).toEqual(["Record z", "Record z", "Record z"]);
+});

test/e2e/tests/navigation.spec.js

@@ -22,9 +22,9 @@ test("can navigate between LiveViews in the same live session over websocket", a
   await syncLV(page);
 
   await expect(networkEvents).toEqual([
-    { method: "GET", url: "http://localhost:4000/navigation/a" },
-    { method: "GET", url: "http://localhost:4000/assets/phoenix/phoenix.min.js" },
-    { method: "GET", url: "http://localhost:4000/assets/phoenix_live_view/phoenix_live_view.js" },
+    { method: "GET", url: "http://localhost:4004/navigation/a" },
+    { method: "GET", url: "http://localhost:4004/assets/phoenix/phoenix.min.js" },
+    { method: "GET", url: "http://localhost:4004/assets/phoenix_live_view/phoenix_live_view.js" },
   ]);
 
   await expect(webSocketEvents).toEqual([
@@ -126,7 +126,7 @@ test("falls back to http navigation when navigating between live sessions", asyn
   await page.getByRole("link", { name: "LiveView (other session)" }).click();
   await syncLV(page);
 
-  await expect(networkEvents).toEqual(expect.arrayContaining([{ method: "GET", url: "http://localhost:4000/stream" }]));
+  await expect(networkEvents).toEqual(expect.arrayContaining([{ method: "GET", url: "http://localhost:4004/stream" }]));
   await expect(webSocketEvents).toEqual(expect.arrayContaining([
     { type: "sent", payload: expect.stringContaining("phx_leave") },
     { type: "sent", payload: expect.stringContaining("phx_join") },