Skip to content

Commit

Permalink
Add test coverage for event listener leaking
Browse files Browse the repository at this point in the history
Add test coverage to [hotwired#454][]. While the original fix was
important to ship, it was unclear how to guard against re-introducing
the leaks.

This commit introduces the `withoutChangingEventListenersCount()` test
helper to wrap a block within the test harness. While executing within
the block, the driven page's `document.addEventListener` and
`document.removeEventListener` functions are replaced with decorated
functions that increment and decrement a counter. At the start of the
block, capture and return how many times `addEventListener` has been
invoked without a matching `removeEventListener`. At the end, capture
and return the differential.

[hotwired#454]: hotwired#454
  • Loading branch information
seanpdoyle committed Nov 18, 2021
1 parent ce984f4 commit 774b4b9
Showing 1 changed file with 55 additions and 0 deletions.
55 changes: 55 additions & 0 deletions src/tests/functional/frame_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,20 @@ export class FrameTests extends TurboDriveTestCase {
await this.goToLocation("/src/tests/fixtures/frames.html")
}

async "test navigating a frame a second time does not leak event listeners"() {
await this.clickSelector("#outside-frame-form")
await this.nextEventOnTarget("frame", "turbo:frame-load")

await this.withoutChangingEventListenersCount(async () => {
await this.clickSelector("#outer-frame-link")
await this.nextEventOnTarget("frame", "turbo:frame-load")
await this.clickSelector("#outside-frame-form")
await this.nextEventOnTarget("frame", "turbo:frame-load")
await this.clickSelector("#outer-frame-link")
await this.nextEventOnTarget("frame", "turbo:frame-load")
})
}

async "test following a link preserves the current <turbo-frame> element's attributes"() {
const currentPath = await this.pathname

Expand Down Expand Up @@ -417,6 +431,47 @@ export class FrameTests extends TurboDriveTestCase {
this.assert.ok(await this.nextEventOnTarget("frame", "turbo:before-fetch-response"))
}

async withoutChangingEventListenersCount(callback: () => void) {
const name = "eventListenersAttachedToDocument"
const setup = () => {
return this.evaluate((name: string) => {
const context = window as any
context[name] = 0
context.originals = { addEventListener: document.addEventListener, removeEventListener: document.removeEventListener }

document.addEventListener = (type: string, listener: EventListenerOrEventListenerObject, options?: boolean | AddEventListenerOptions) => {
context.originals.addEventListener.call(document, type, listener, options)
context[name] += 1
}

document.removeEventListener = (type: string, listener: EventListenerOrEventListenerObject, options?: boolean | AddEventListenerOptions) => {
context.originals.removeEventListener.call(document, type, listener, options)
context[name] -= 1
}

return context[name] || 0
}, [name])
}

const teardown = () => {
return this.evaluate((name: string) => {
const context = window as any
const { addEventListener, removeEventListener } = context.originals

document.addEventListener = addEventListener
document.removeEventListener = removeEventListener

return context[name] || 0
}, [name])
}

const originalCount = await setup()
await callback()
const finalCount = await teardown()

this.assert.equal(finalCount, originalCount, "expected callback not to leak event listeners")
}

get frameScriptEvaluationCount(): Promise<number | undefined> {
return this.evaluate(() => window.frameScriptEvaluationCount)
}
Expand Down

0 comments on commit 774b4b9

Please sign in to comment.