Skip to content

Commit

Permalink
proprly clear viewHooks when destroying a hook
Browse files Browse the repository at this point in the history
Relates to: #3496
Fixes: #3623

In #3496, I cleared the hookId on the element in destroyed,
but the view's destroyHook function actually relied on the hookId to
clean up the view's viewHooks object. This would cause hooks' destroy
function to be called multiple times, as the view was not able to remove
old hooks.
  • Loading branch information
SteffenDE committed Jan 10, 2025
1 parent cc92bdd commit 95986aa
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
5 changes: 4 additions & 1 deletion assets/js/phoenix_live_view/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -725,9 +725,12 @@ export default class View {
}

destroyHook(hook){
// __destroyed clears the elementID from the hook, therefore
// we need to get it before calling __destroyed
const hookId = ViewHook.elementID(hook.el)
hook.__destroyed()
hook.__cleanup__()
delete this.viewHooks[ViewHook.elementID(hook.el)]
delete this.viewHooks[hookId]
}

applyPendingUpdates(){
Expand Down
2 changes: 2 additions & 0 deletions assets/test/view_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,7 @@ describe("View Hooks", function(){
liveview_version
})
expect(view.el.firstChild.innerHTML).toBe("TEST MOUNT")
expect(Object.keys(view.viewHooks)).toHaveLength(1)

view.update({
s: ["<h2 id=\"up\" phx-hook=\"Upcase\">test update</h2>"],
Expand All @@ -849,6 +850,7 @@ describe("View Hooks", function(){
view.update({s: ["<div></div>"], fingerprint: 123}, [])
expect(upcaseWasDestroyed).toBe(true)
expect(hookLiveSocket).toBeDefined()
expect(Object.keys(view.viewHooks)).toEqual([])
})

test("createHook", (done) => {
Expand Down

0 comments on commit 95986aa

Please sign in to comment.