Skip to content

Commit

Permalink
fail early when hook element is not owned by view (#3618)
Browse files Browse the repository at this point in the history
* fail early when hook element is not owned by view

Fixes #3530.

* e2e tests
  • Loading branch information
SteffenDE authored Jan 26, 2025
1 parent a414c2c commit f100c75
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 1 deletion.
4 changes: 3 additions & 1 deletion assets/js/phoenix_live_view/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,9 @@ export default class View {
addHook(el){
let hookElId = ViewHook.elementID(el)

// only ever try to add hooks to elements owned by this view
if(el.getAttribute && !this.ownsElement(el)){ return }

if(hookElId && !this.viewHooks[hookElId]){
// hook created, but not attached (createHook for web component)
let hook = DOM.getCustomElHook(el) || logError(`no hook found for custom element: ${el.id}`)
Expand All @@ -714,7 +717,6 @@ export default class View {
} else {
// new hook found with phx-hook attribute
let hookName = el.getAttribute(`data-phx-${PHX_HOOK}`) || el.getAttribute(this.binding(PHX_HOOK))
if(hookName && !this.ownsElement(el)){ return }
let callbacks = this.liveSocket.getHookCallbacks(hookName)

if(callbacks){
Expand Down
97 changes: 97 additions & 0 deletions test/e2e/support/issues/issue_3530.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
defmodule Phoenix.LiveViewTest.E2E.Issue3530Live do
use Phoenix.LiveView, layout: {__MODULE__, :live}

defmodule NestedLive do
use Phoenix.LiveView

def mount(_params, session, socket) do
{:ok, assign(socket, :item_id, session["item_id"])}
end

def render(assigns) do
~H"""
<div id={"item-outer-#{@item_id}"}>
test hook with nested liveview
<div id={"test-hook-#{@item_id}"} phx-hook="test"></div>
</div>
"""
end
end

def render("live.html", assigns) do
~H"""
<meta name="csrf-token" content={Plug.CSRFProtection.get_csrf_token()} />
<script src="/assets/phoenix/phoenix.min.js">
</script>
<script type="module">
import {LiveSocket} from "/assets/phoenix_live_view/phoenix_live_view.esm.js"
let csrfToken = document.querySelector("meta[name='csrf-token']").getAttribute("content");
let liveSocket = new LiveSocket("/live", window.Phoenix.Socket, {params: {_csrf_token: csrfToken}, hooks: {
test: {
mounted() { console.log(this.__view().id, "mounted hook!") }
}
}})
liveSocket.connect()
window.liveSocket = liveSocket
</script>
{@inner_content}
"""
end

def render(assigns) do
~H"""
<ul id="stream-list" phx-update="stream">
<%= for {dom_id, item} <- @streams.items do %>
{live_render(@socket, NestedLive, id: dom_id, session: %{"item_id" => item.id})}
<% end %>
</ul>
<.link patch="/issues/3530?q=a">patch a</.link>
<.link patch="/issues/3530?q=b">patch b</.link>
<div phx-click="inc">+</div>
"""
end

def mount(_params, _session, socket) do
socket =
socket
|> assign(:count, 3)
|> stream_configure(:items, dom_id: &"item-#{&1.id}")

{:ok, socket}
end

def handle_params(%{"q" => "a"}, _uri, socket) do
socket =
socket
|> stream(:items, [%{id: 1}, %{id: 3}], reset: true)

{:noreply, socket}
end

def handle_params(%{"q" => "b"}, _uri, socket) do
socket =
socket
|> stream(:items, [%{id: 2}, %{id: 3}], reset: true)

{:noreply, socket}
end

def handle_params(_params, _uri, socket) do
socket =
socket
|> stream(:items, [%{id: 1}, %{id: 2}, %{id: 3}], reset: true)

{:noreply, socket}
end

def handle_event("inc", _params, socket) do
socket =
socket
|> update(:count, &(&1 + 1))
|> then(&stream_insert(&1, :items, %{id: &1.assigns.count}))

{:noreply, socket}
end
end
1 change: 1 addition & 0 deletions test/e2e/test_helper.exs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ defmodule Phoenix.LiveViewTest.E2E.Router do
live "/3047/a", Issue3047ALive
live "/3047/b", Issue3047BLive
live "/3169", Issue3169Live
live "/3530", Issue3530Live
live "/3647", Issue3647Live
end
end
Expand Down
46 changes: 46 additions & 0 deletions test/e2e/tests/issues/3530.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
const {test, expect} = require("../../test-fixtures")
const {syncLV} = require("../../utils")

// https://github.com/phoenixframework/phoenix_live_view/issues/3530
test("hook is initialized properly when using a stream of nested LiveViews", async ({page}) => {
let logs = []
page.on("console", (e) => logs.push(e.text()))
const errors = []
page.on("pageerror", (err) => errors.push(err))

await page.goto("/issues/3530")
await syncLV(page)

expect(errors).toEqual([])
expect(logs.filter(e => e.includes("item-1 mounted"))).toHaveLength(1)
expect(logs.filter(e => e.includes("item-2 mounted"))).toHaveLength(1)
expect(logs.filter(e => e.includes("item-3 mounted"))).toHaveLength(1)
logs = []

await page.getByRole("link", {name: "patch a"}).click()
await syncLV(page)

expect(errors).toEqual([])
expect(logs.filter(e => e.includes("item-2 destroyed"))).toHaveLength(1)
expect(logs.filter(e => e.includes("item-1 destroyed"))).toHaveLength(0)
expect(logs.filter(e => e.includes("item-3 destroyed"))).toHaveLength(0)
logs = []

await page.getByRole("link", {name: "patch b"}).click()
await syncLV(page)

expect(errors).toEqual([])
expect(logs.filter(e => e.includes("item-1 destroyed"))).toHaveLength(1)
expect(logs.filter(e => e.includes("item-2 destroyed"))).toHaveLength(0)
expect(logs.filter(e => e.includes("item-3 destroyed"))).toHaveLength(0)
expect(logs.filter(e => e.includes("item-2 mounted"))).toHaveLength(1)
logs = []

await page.locator("div[phx-click=inc]").click()
await syncLV(page)
expect(logs.filter(e => e.includes("item-4 mounted"))).toHaveLength(1)

expect(logs).not.toEqual(expect.arrayContaining([expect.stringMatching("no hook found for custom element")]))
// no uncaught exceptions
expect(errors).toEqual([])
})

0 comments on commit f100c75

Please sign in to comment.