Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

properly clear viewHooks when destroying a hook #3628

Merged
merged 1 commit into from
Jan 11, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
properly clear viewHooks when destroying a hook
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

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit c36dd8c49655a71147b0e67a22ea6e66b1a0cdbc
5 changes: 4 additions & 1 deletion assets/js/phoenix_live_view/view.js
Original file line number Diff line number Diff line change
@@ -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(){
2 changes: 2 additions & 0 deletions assets/test/view_test.js
Original file line number Diff line number Diff line change
@@ -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>"],
@@ -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) => {
Loading