Skip to content

Commit 7d7b24e

Browse files
SteffenDESteffenDE
authored
Ensure component only patches are not lost due to locked tree (#3942)
* Ensure component only patches are not lost due to locked tree Closes #3941. Possibly related to #3614. When a component-only diff was applied, we did not account for the component being part of a locked tree. Thus, when the tree was unlocked, the component content was unintentionally restored to a previous state. We solve this by checking for a locked tree and applying the patch to that instead. * link to pr
1 parent 5781c47 commit 7d7b24e

File tree

4 files changed

+222
-2
lines changed

4 files changed

+222
-2
lines changed

‎assets/js/phoenix_live_view/dom_patch.js‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,28 @@ export default class DOMPatch {
8585
}
8686

8787
perform(isJoinPatch) {
88-
const { view, liveSocket, html, container, targetContainer } = this;
89-
if (this.isCIDPatch() && !targetContainer) {
88+
const { view, liveSocket, html, container } = this;
89+
let targetContainer = this.targetContainer;
90+
91+
if (this.isCIDPatch() && !this.targetContainer) {
9092
return;
9193
}
9294

95+
if (this.isCIDPatch()) {
96+
// https://github.com/phoenixframework/phoenix_live_view/pull/3942
97+
// we need to ensure that no parent is locked
98+
const closestLock = targetContainer.closest(`[${PHX_REF_LOCK}]`);
99+
if (closestLock) {
100+
const clonedTree = DOM.private(closestLock, PHX_REF_LOCK);
101+
if (clonedTree) {
102+
// if a parent is locked with a cloned tree, we need to patch the cloned tree instead
103+
targetContainer = clonedTree.querySelector(
104+
`[data-phx-component="${this.targetCID}"]`,
105+
);
106+
}
107+
}
108+
}
109+
93110
const focused = liveSocket.getActiveElement();
94111
const { selectionStart, selectionEnd } =
95112
focused && DOM.hasSelectionRange(focused) ? focused : {};

‎test/e2e/support/issues/issue_3941.ex‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
defmodule Phoenix.LiveViewTest.E2E.Issue3941Live do
2+
use Phoenix.LiveView, layout: {__MODULE__, :live}
3+
4+
alias Phoenix.LiveViewTest.E2E.Issue3941Live.Item
5+
6+
@all_items [
7+
"Item_1",
8+
"Item_2"
9+
]
10+
11+
@impl true
12+
def mount(_params, _session, socket) do
13+
socket =
14+
socket
15+
|> assign(:selected_items, @all_items)
16+
|> assign(:filter_options, @all_items)
17+
18+
{:ok, socket}
19+
end
20+
21+
def render("live.html", assigns) do
22+
~H"""
23+
<meta name="csrf-token" content={Plug.CSRFProtection.get_csrf_token()} />
24+
<script src="/assets/phoenix/phoenix.min.js">
25+
</script>
26+
<script type="module">
27+
import {LiveSocket} from "/assets/phoenix_live_view/phoenix_live_view.esm.js"
28+
let csrfToken = document.querySelector("meta[name='csrf-token']").getAttribute("content");
29+
let liveSocket = new LiveSocket("/live", window.Phoenix.Socket, {
30+
params: {_csrf_token: csrfToken},
31+
hooks: {
32+
PagePositionNotifier: {
33+
mounted() {
34+
this.pushEvent("page_position_update", {});
35+
},
36+
}
37+
}
38+
})
39+
liveSocket.connect()
40+
</script>
41+
42+
{@inner_content}
43+
"""
44+
end
45+
46+
def render(assigns) do
47+
~H"""
48+
<.multi_select id="multi-select" items={@filter_options} selected={@selected_items} />
49+
<div :for={item <- @selected_items}>
50+
<.live_component
51+
module={Item}
52+
id={"item-#{item}"}
53+
item={item}
54+
/>
55+
</div>
56+
"""
57+
end
58+
59+
def multi_select(assigns) do
60+
~H"""
61+
<div :for={item <- @items}>
62+
<label for={"item-select-#{item}"}>
63+
<input
64+
type="checkbox"
65+
phx-click="toggle_item"
66+
phx-value-clicked={item}
67+
id={"select-#{item}"}
68+
name="select"
69+
value={item}
70+
checked={item in @selected}
71+
/>
72+
{item}
73+
</label>
74+
</div>
75+
"""
76+
end
77+
78+
@impl true
79+
def handle_event("toggle_item", params = %{"clicked" => clicked_id}, socket) do
80+
selected = socket.assigns.selected_items
81+
82+
selected =
83+
case params["value"] do
84+
nil ->
85+
selected = List.delete(selected, clicked_id)
86+
87+
value ->
88+
selected = selected ++ [value]
89+
end
90+
91+
{:noreply, assign(socket, selected_items: Enum.sort(selected))}
92+
end
93+
94+
def handle_event("page_position_update", _params, socket) do
95+
{:noreply, socket}
96+
end
97+
end
98+
99+
defmodule Phoenix.LiveViewTest.E2E.Issue3941Live.Item do
100+
use Phoenix.LiveComponent
101+
alias Phoenix.LiveViewTest.E2E.Issue3941Live.ItemHeader
102+
103+
@impl true
104+
def update(assigns, socket) do
105+
{:ok,
106+
socket
107+
|> assign(:item, assigns.item)
108+
|> assign_unrendered_component_assigns()}
109+
end
110+
111+
@impl true
112+
def render(assigns) do
113+
~H"""
114+
<div id={"item-#{@item}"} phx-hook="PagePositionNotifier">
115+
<.live_component
116+
id={"item-header-#{@item}"}
117+
module={ItemHeader}
118+
item={@item}
119+
/>
120+
<.unrendered_component :if={false} id="unrendered" any_assign={@any_assign} />
121+
</div>
122+
"""
123+
end
124+
125+
defp unrendered_component(_) do
126+
raise "SHOULD NOT BE CALLED"
127+
end
128+
129+
defp assign_unrendered_component_assigns(socket) do
130+
socket
131+
|> assign(:any_assign, true)
132+
|> assign_async(
133+
:any_assign,
134+
fn ->
135+
{:ok, %{any_assign: true}}
136+
end
137+
)
138+
end
139+
end
140+
141+
defmodule Phoenix.LiveViewTest.E2E.Issue3941Live.ItemHeader do
142+
use Phoenix.LiveComponent
143+
144+
def update(assigns, socket) do
145+
{:ok,
146+
socket
147+
|> assign(:item, assigns.item)
148+
|> assign_async(
149+
:async_assign,
150+
fn ->
151+
{:ok, %{async_assign: :assign}}
152+
end,
153+
reset: true
154+
)}
155+
end
156+
157+
def render(assigns) do
158+
~H"""
159+
<div id={"header-#{@item}"}>
160+
<.async_result assign={@async_assign}>
161+
<:loading>
162+
<div id={@item} class="border border-y-0 bg-red-500 text-white">
163+
{"#{@item} - I AM LOADING"}
164+
</div>
165+
</:loading>
166+
<div id={@item} class="border border-y-0 bg-green-500 text-white">
167+
{"#{@item} - I AM LOADED!"}
168+
</div>
169+
</.async_result>
170+
{inspect(@async_assign)}
171+
</div>
172+
"""
173+
end
174+
end

‎test/e2e/test_helper.exs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ defmodule Phoenix.LiveViewTest.E2E.Router do
238238
live "/3647", Issue3647Live
239239
live "/3681", Issue3681Live
240240
live "/3681/away", Issue3681.AwayLive
241+
live "/3941", Issue3941Live
241242
end
242243

243244
post "/submit", SubmitController, :submit

‎test/e2e/tests/issues/3941.spec.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { test, expect } from "../../test-fixtures";
2+
import { syncLV } from "../../utils";
3+
4+
// https://github.com/phoenixframework/phoenix_live_view/issues/3941
5+
test("component-only patch in locked tree works", async ({ page }) => {
6+
await page.goto("/issues/3941");
7+
await syncLV(page);
8+
9+
// the bug was that, because the parent container was locked,
10+
// the component only patch was applied and later on the (stale) locked
11+
// tree was applied, erasing the patch
12+
await expect(page.locator("#Item_1")).toContainText("I AM LOADED");
13+
await expect(page.locator("#Item_2")).toContainText("I AM LOADED");
14+
15+
await page.locator("#select-Item_1").uncheck();
16+
await page.locator("#select-Item_2").uncheck();
17+
18+
await expect(page.locator("#Item_1")).toHaveCount(0);
19+
await expect(page.locator("#Item_2")).toHaveCount(0);
20+
21+
await page.locator("#select-Item_1").check();
22+
await expect(page.locator("#Item_1")).toContainText("I AM LOADED");
23+
await expect(page.locator("#Item_2")).toHaveCount(0);
24+
25+
await page.locator("#select-Item_2").check();
26+
await expect(page.locator("#Item_1")).toContainText("I AM LOADED");
27+
await expect(page.locator("#Item_2")).toContainText("I AM LOADED");
28+
});

0 commit comments

Comments
 (0)