Skip to content
Merged
Show file tree
Hide file tree
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
57 changes: 46 additions & 11 deletions s3file/static/s3file/js/s3file.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,29 +34,59 @@ export class S3FileInput extends globalThis.HTMLElement {
}

connectedCallback() {
// Prevent creating duplicate inputs if already connected
if (this._fileInput) return

// Create a hidden file input for the file picker functionality
this._fileInput = document.createElement("input")
this._fileInput.type = "file"

// Sync attributes to hidden input
this._syncAttributesToHiddenInput()

// Listen for file selection on hidden input
this._fileInput.addEventListener("change", () => {
// Create bound handler references for cleanup
this._changeHandler = () => {
this._files = this._fileInput.files
this.dispatchEvent(new Event("change", { bubbles: true }))
this.changeHandler()
})
}
this._boundFormDataHandler = this.fromDataHandler.bind(this)
this._boundSubmitHandler = this.submitHandler.bind(this)
this._boundUploadHandler = this.uploadHandler.bind(this)

// Listen for file selection on hidden input
this._fileInput.addEventListener("change", this._changeHandler)

// Append elements
this.appendChild(this._fileInput)

// Setup form event listeners
this.form?.addEventListener("formdata", this.fromDataHandler.bind(this))
this.form?.addEventListener("submit", this.submitHandler.bind(this), {
this.form?.addEventListener("formdata", this._boundFormDataHandler)
this.form?.addEventListener("submit", this._boundSubmitHandler, {
once: true,
})
this.form?.addEventListener("upload", this.uploadHandler.bind(this))
this.form?.addEventListener("upload", this._boundUploadHandler)
}

disconnectedCallback() {
// Clean up event listeners
if (this._fileInput) {
this._fileInput.removeEventListener("change", this._changeHandler)
}

if (this.form) {
this.form.removeEventListener("formdata", this._boundFormDataHandler)
this.form.removeEventListener("submit", this._boundSubmitHandler)
this.form.removeEventListener("upload", this._boundUploadHandler)
}

// Remove the file input from DOM
if (this._fileInput && this._fileInput.parentNode) {
this._fileInput.parentNode.removeChild(this._fileInput)
}

// Clear reference to allow reconnection
this._fileInput = null
}
Comment on lines +71 to 90
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bound handler references (_changeHandler, _boundFormDataHandler, _boundSubmitHandler, _boundUploadHandler) should be nulled after removing event listeners to prevent memory leaks. These references will persist in memory even after the element is disconnected and the event listeners are removed.

Consider adding the following after line 81:

  • Set this._changeHandler = null
  • Set this._boundFormDataHandler = null
  • Set this._boundSubmitHandler = null
  • Set this._boundUploadHandler = null

Copilot uses AI. Check for mistakes.

/**
Expand Down Expand Up @@ -198,12 +228,17 @@ export class S3FileInput extends globalThis.HTMLElement {
changeHandler() {
this.keys = []
this.upload = null
try {
this.form?.removeEventListener("submit", this.submitHandler.bind(this))
} catch (error) {
console.debug(error)
// Remove previous submit handler and add a new one
if (this._boundSubmitHandler) {
try {
this.form?.removeEventListener("submit", this._boundSubmitHandler)
} catch (error) {
console.debug(error)
}
}
this.form?.addEventListener("submit", this.submitHandler.bind(this), {
// Create a new bound handler for this change
this._boundSubmitHandler = this.submitHandler.bind(this)
this.form?.addEventListener("submit", this._boundSubmitHandler, {
once: true,
})
}
Expand Down
37 changes: 37 additions & 0 deletions tests/__tests__/s3file.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,43 @@ describe("S3FileInput", () => {
assert(input._fileInput.type === "file")
})

test("disconnectedCallback", () => {
const form = document.createElement("form")
document.body.appendChild(form)
const input = new s3file.S3FileInput()
form.addEventListener = mock.fn(form.addEventListener)
form.appendChild(input)
const fileInput = input._fileInput
assert(fileInput !== null)
// Mock removeEventListener to verify cleanup
form.removeEventListener = mock.fn(form.removeEventListener)
fileInput.removeEventListener = mock.fn(fileInput.removeEventListener)
// Manually call disconnectedCallback since jsdom doesn't trigger it
input.disconnectedCallback()
assert(form.removeEventListener.mock.calls.length === 3)
assert(fileInput.removeEventListener.mock.calls.length === 1)
assert(fileInput.parentNode === null)
assert(input._fileInput === null)
})

test("connectedCallback prevents duplicate inputs on reconnection", () => {
const form = document.createElement("form")
document.body.appendChild(form)
const input = new s3file.S3FileInput()
// First connection
form.appendChild(input)
assert(input._fileInput !== null)
assert(input.querySelectorAll('input[type="file"]').length === 1)
// Disconnect and clear state
input.disconnectedCallback()
// Reconnect - should create a new input since old one was cleaned up
input.connectedCallback()
assert(input._fileInput !== null)
// Check only one input element exists after reconnection
const inputElements = input.querySelectorAll('input[type="file"]')
assert(inputElements.length === 1)
})

test("changeHandler", () => {
const form = document.createElement("form")
const input = new s3file.S3FileInput()
Expand Down
Loading