Skip to content

Commit

Permalink
Fix $magic causing detached element memory leaks
Browse files Browse the repository at this point in the history
  • Loading branch information
calebporzio committed Mar 1, 2022
1 parent 9430be3 commit 260679b
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 18 deletions.
20 changes: 13 additions & 7 deletions packages/alpinejs/src/directives.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,7 @@ export function deferHandlingDirectives(callback) {
stopDeferring()
}

export function getDirectiveHandler(el, directive) {
let noop = () => {}

let handler = directiveHandlers[directive.type] || noop

export function getElementBoundUtilities(el) {
let cleanups = []

let cleanup = callback => cleanups.push(callback)
Expand All @@ -88,7 +84,17 @@ export function getDirectiveHandler(el, directive) {

let doCleanup = () => cleanups.forEach(i => i())

onAttributeRemoved(el, directive.original, doCleanup)
return [utilities, doCleanup]
}

export function getDirectiveHandler(el, directive) {
let noop = () => {}

let handler = directiveHandlers[directive.type] || noop

let [utilities, cleanup] = getElementBoundUtilities(el)

onAttributeRemoved(el, directive.original, cleanup)

let fullHandler = () => {
if (el._x_ignore || el._x_ignoreSelf) return
Expand All @@ -100,7 +106,7 @@ export function getDirectiveHandler(el, directive) {
isDeferringHandlers ? directiveHandlerStacks.get(currentHandlerStackKey).push(handler) : handler()
}

fullHandler.runCleanups = doCleanup
fullHandler.runCleanups = cleanup

return fullHandler
}
Expand Down
7 changes: 3 additions & 4 deletions packages/alpinejs/src/directives/x-init.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import { directive, prefix } from "../directives";
import { addInitSelector } from "../lifecycle";
import { skipDuringClone } from "../clone";
import { evaluate } from "../evaluator";

addInitSelector(() => `[${prefix('init')}]`)

directive('init', skipDuringClone((el, { expression }) => {
directive('init', skipDuringClone((el, { expression }, { evaluate }) => {
if (typeof expression === 'string') {
return !! expression.trim() && evaluate(el, expression, {}, false)
return !! expression.trim() && evaluate(expression, {}, false)
}

return evaluate(el, expression, {}, false)
return evaluate(expression, {}, false)
}))
12 changes: 11 additions & 1 deletion packages/alpinejs/src/magics.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import Alpine from './alpine'
import { getElementBoundUtilities } from './directives'
import { interceptor } from './interceptor'
import { onElRemoved } from './mutation'

let magics = {}

Expand All @@ -10,7 +12,15 @@ export function magic(name, callback) {
export function injectMagics(obj, el) {
Object.entries(magics).forEach(([name, callback]) => {
Object.defineProperty(obj, `$${name}`, {
get() { return callback(el, { Alpine, interceptor }) },
get() {
let [utilities, cleanup] = getElementBoundUtilities(el)

utilities = {interceptor, ...utilities}

onElRemoved(el, cleanup)

return callback(el, utilities)
},

enumerable: false,
})
Expand Down
6 changes: 2 additions & 4 deletions packages/alpinejs/src/magics/$watch.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { evaluateLater } from '../evaluator'
import { effect } from '../reactivity'
import { magic } from '../magics'

magic('watch', el => (key, callback) => {
let evaluate = evaluateLater(el, key)
magic('watch', (el, { evaluateLater, effect }) => (key, callback) => {
let evaluate = evaluateLater(key)

let firstTime = true

Expand Down
14 changes: 12 additions & 2 deletions packages/alpinejs/src/mutation.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,14 @@ export function onElAdded(callback) {
onElAddeds.push(callback)
}

export function onElRemoved(callback) {
onElRemoveds.push(callback)
export function onElRemoved(el, callback) {
if (typeof callback === 'function') {
if (! el._x_cleanups) el._x_cleanups = []
el._x_cleanups.push(callback)
} else {
callback = el
onElRemoveds.push(callback)
}
}

export function onAttributesAdded(callback) {
Expand Down Expand Up @@ -166,6 +172,10 @@ function onMutate(mutations) {
if (addedNodes.includes(node)) continue

onElRemoveds.forEach(i => i(node))

if (node._x_cleanups) {
while (node._x_cleanups.length) node._x_cleanups.pop()()
}
}

// Mutations are bundled together by the browser but sometimes
Expand Down
25 changes: 25 additions & 0 deletions tests/cypress/integration/mutation.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,31 @@ test('nested element side effects are cleaned up after the parent is removed',
}
)

test('element magic-based side effects are cleaned up after the element is removed',
html`
<div x-data="{ foo: 1, bar: 1 }">
<button @click="foo++">foo</button>
<a href="#" @click.prevent="$refs.span.remove()">remove</a>
<span x-init="$watch('foo', () => bar++)" x-ref="span"></span>
<h1 x-text="foo"></h1>
<h2 x-text="bar"></h2>
</div>
`,
({ get }) => {
get('h1').should(haveText('1'))
get('h2').should(haveText('1'))
get('button').click()
get('h1').should(haveText('2'))
get('h2').should(haveText('2'))
get('a').click()
get('button').click()
get('h1').should(haveText('3'))
get('h2').should(haveText('2'))
}
)

test('can mutate directive value',
html`
<div x-data="{ foo: 'bar', bar: 'baz' }">
Expand Down

0 comments on commit 260679b

Please sign in to comment.