Implement disconnectedCallback and prevent duplicate inputs

Co-authored-by: codingjoe <1772890+codingjoe@users.noreply.github.com>

Author: Copilot

s3file/static/s3file/js/s3file.js

@@ -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
   }
 
   /**
@@ -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,
     })
   }

tests/__tests__/s3file.test.js

@@ -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()