Merge pull request #34 from basecamp/hadnle-unregister-controllers

Handle deletion of stimulus controllers

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    hotwire-spark (0.1.6)
+    hotwire-spark (0.1.7)
       listen
       rails (>= 8.0.0)
       zeitwerk

app/assets/javascripts/hotwire_spark.js

@@ -3537,14 +3537,19 @@ var HotwireSpark = (function () {
     async reload() {
       log("Reload Stimulus controllers...");
       this.application.stop();
-      await this.#reloadStimulusControllers();
+      await this.#reloadChangedStimulusControllers();
+      this.#unloadDeletedStimulusControllers();
       this.application.start();
     }
-    async #reloadStimulusControllers() {
-      await Promise.all(this.#stimulusControllerPaths.map(async moduleName => this.#reloadStimulusController(moduleName)));
+    async #reloadChangedStimulusControllers() {
+      await Promise.all(this.#stimulusControllerPathsToReload.map(async moduleName => this.#reloadStimulusController(moduleName)));
+    }
+    get #stimulusControllerPathsToReload() {
+      this.controllerPathsToReload = this.controllerPathsToReload || this.#stimulusControllerPaths.filter(path => this.#shouldReloadController(path));
+      return this.controllerPathsToReload;
     }
     get #stimulusControllerPaths() {
-      return Object.keys(this.#stimulusPathsByModule).filter(path => path.endsWith("_controller") && this.#shouldReloadController(path));
+      return Object.keys(this.#stimulusPathsByModule).filter(path => path.endsWith("_controller"));
     }
     #shouldReloadController(path) {
       return this.filePattern.test(path);
@@ -3564,6 +3569,19 @@ var HotwireSpark = (function () {
       const module = await import(path);
       this.#registerController(controllerName, module);
     }
+    #unloadDeletedStimulusControllers() {
+      this.#controllersToUnload.forEach(controller => this.#deregisterController(controller.identifier));
+    }
+    get #controllersToUnload() {
+      if (this.#didChangeTriggerAReload) {
+        return [];
+      } else {
+        return this.application.controllers.filter(controller => this.filePattern.test(`${controller.identifier}_controller`));
+      }
+    }
+    get #didChangeTriggerAReload() {
+      return this.#stimulusControllerPathsToReload.length > 0;
+    }
     #pathForModuleName(moduleName) {
       return this.#stimulusPathsByModule[moduleName];
     }
@@ -3574,6 +3592,10 @@ var HotwireSpark = (function () {
       this.application.unload(name);
       this.application.register(name, module.default);
     }
+    #deregisterController(name) {
+      log(`\tRemoving controller ${name}`);
+      this.application.unload(name);
+    }
   }
 
   class HtmlReloader {

app/javascript/hotwire/spark/reloaders/stimulus_reloader.js

@@ -18,18 +18,26 @@ export class StimulusReloader {
     log("Reload Stimulus controllers...")
 
     this.application.stop()
-    await this.#reloadStimulusControllers()
+
+    await this.#reloadChangedStimulusControllers()
+    this.#unloadDeletedStimulusControllers()
+
     this.application.start()
   }
 
-  async #reloadStimulusControllers() {
+  async #reloadChangedStimulusControllers() {
     await Promise.all(
-      this.#stimulusControllerPaths.map(async moduleName => this.#reloadStimulusController(moduleName))
+      this.#stimulusControllerPathsToReload.map(async moduleName => this.#reloadStimulusController(moduleName))
     )
   }
 
+  get #stimulusControllerPathsToReload() {
+    this.controllerPathsToReload = this.controllerPathsToReload || this.#stimulusControllerPaths.filter(path => this.#shouldReloadController(path))
+    return this.controllerPathsToReload
+  }
+
   get #stimulusControllerPaths() {
-    return Object.keys(this.#stimulusPathsByModule).filter(path => path.endsWith("_controller") && this.#shouldReloadController(path))
+    return Object.keys(this.#stimulusPathsByModule).filter(path => path.endsWith("_controller"))
   }
 
   #shouldReloadController(path) {
@@ -57,6 +65,22 @@ export class StimulusReloader {
     this.#registerController(controllerName, module)
   }
 
+  #unloadDeletedStimulusControllers() {
+    this.#controllersToUnload.forEach(controller => this.#deregisterController(controller.identifier))
+  }
+
+  get #controllersToUnload() {
+    if (this.#didChangeTriggerAReload) {
+      return []
+    } else {
+      return this.application.controllers.filter(controller => this.filePattern.test(`${controller.identifier}_controller`))
+    }
+  }
+
+  get #didChangeTriggerAReload() {
+    return this.#stimulusControllerPathsToReload.length > 0
+  }
+
   #pathForModuleName(moduleName) {
     return this.#stimulusPathsByModule[moduleName]
   }
@@ -73,4 +97,9 @@ export class StimulusReloader {
     this.application.unload(name)
     this.application.register(name, module.default)
   }
+
+  #deregisterController(name) {
+    log(`\tRemoving controller ${name}`)
+    this.application.unload(name)
+  }
 }

lib/hotwire/spark/version.rb

@@ -1,5 +1,5 @@
 module Hotwire
   module Spark
-    VERSION = "0.1.6"
+    VERSION = "0.1.7"
   end
 end

test/dummy/app/javascript/controllers/dummy_controller.js

@@ -8,9 +8,11 @@ export default class extends Controller {
   connect() {
     console.debug("Dummy controller connected ", this.version)
     this.element.querySelector("#replace").textContent = "_REPLACE_"
+    this.element.setAttribute("data-dummy-version", this.version)
   }
 
   disconnect() {
     console.debug("Dummy controller disconnected", this.version)
+    this.element.removeAttribute("data-dummy-version")
   }
 }

test/helpers/files_helper.rb

@@ -11,13 +11,13 @@ module FilesHelper
   ORIGINAL_EXTENSION = ".original"
 
   def edit_file(path, replace:, with:)
-    path = Rails.application.root.join(path).to_s
+    path = expand_path(path)
 
     raise ArgumentError, "File at '#{path}' does not exist." unless File.exist?(path)
 
-    original_path = "#{path}#{ORIGINAL_EXTENSION}"
-    remember_path_to_restore original_path
-    system "cp", path, original_path
+    original_path = remember_original_path_to_restore path
+
+    FileUtils.cp path, original_path
 
     content = File.read(path)
     updated_content = content.gsub(replace, with)
@@ -27,7 +27,7 @@ def edit_file(path, replace:, with:)
   end
 
   def add_file(path, content)
-    path = Rails.application.root.join(path).to_s
+    path = expand_path(path)
 
     raise ArgumentError, "File at '#{path}' already exists." if File.exist?(path)
 
@@ -37,7 +37,25 @@ def add_file(path, content)
     reload_rails_reloader
   end
 
+  def remove_file(path)
+    path = expand_path(path)
+
+    original_path = remember_original_path_to_restore path
+
+    FileUtils.mv path, original_path
+  end
+
   private
+    def expand_path(path)
+      Rails.application.root.join(path).to_s
+    end
+
+    def remember_original_path_to_restore(path)
+      "#{path}#{ORIGINAL_EXTENSION}".tap do |original_path|
+        remember_path_to_restore original_path
+      end
+    end
+
     def remember_path_to_restore(path)
       paths_to_restore << path
     end

test/stimulus_reload_test.rb

@@ -1,8 +1,11 @@
 require "application_system_test_case"
 
 class StimulusReloadTest < ApplicationSystemTestCase
-  test "reload Stimulus controller changes" do
+  setup do
     visit root_path
+  end
+
+  test "reload Stimulus controller changes" do
     assert_no_text "This was replaced!"
 
     edit_file "app/javascript/controllers/dummy_controller.js", replace: "_REPLACE_", with: "This was replaced!"
@@ -11,7 +14,6 @@ class StimulusReloadTest < ApplicationSystemTestCase
   end
 
   test "load new Stimulus controllers" do
-    visit root_path
     assert_no_text "This was replaced!"
 
     edit_file "app/views/home/show.html.erb", replace: "_REPLACE_CONTROLLER_", with: "other-dummy"
@@ -29,4 +31,12 @@ class StimulusReloadTest < ApplicationSystemTestCase
 
     assert_text "This was replaced!"
   end
+
+  test "unload removed Stimulus controllers" do
+    assert_css "[data-dummy-version]"
+
+    remove_file "app/javascript/controllers/dummy_controller.js"
+
+    assert_no_css "[data-dummy-version]"
+  end
 end