Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Handle unloading of deleted stimulus controllers" #33

Merged
merged 1 commit into from
Dec 20, 2024
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
32 changes: 5 additions & 27 deletions app/assets/javascripts/hotwire_spark.js
Original file line number Diff line number Diff line change
Expand Up @@ -3537,19 +3537,14 @@ var HotwireSpark = (function () {
async reload() {
log("Reload Stimulus controllers...");
this.application.stop();
await this.#reloadChangedStimulusControllers();
this.#unloadDeletedStimulusControllers();
await this.#reloadStimulusControllers();
this.application.start();
}
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;
async #reloadStimulusControllers() {
await Promise.all(this.#stimulusControllerPaths.map(async moduleName => this.#reloadStimulusController(moduleName)));
}
get #stimulusControllerPaths() {
return Object.keys(this.#stimulusPathsByModule).filter(path => path.endsWith("_controller"));
return Object.keys(this.#stimulusPathsByModule).filter(path => path.endsWith("_controller") && this.#shouldReloadController(path));
}
#shouldReloadController(path) {
return this.filePattern.test(path);
Expand All @@ -3569,32 +3564,15 @@ 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];
}
#extractControllerName(path) {
return path.replace(/^.*\//, "").replace("_controller", "").replace(/\//g, "--").replace(/_/g, "-");
}
#registerController(name, module) {
this.#deregisterController(name);
this.application.register(name, module.default);
}
#deregisterController(name) {
log(`\tRemoving controller ${name}`);
this.application.unload(name);
this.application.register(name, module.default);
}
}

Expand Down
2 changes: 1 addition & 1 deletion app/assets/javascripts/hotwire_spark.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion app/assets/javascripts/hotwire_spark.min.js.map

Large diffs are not rendered by default.

39 changes: 5 additions & 34 deletions app/javascript/hotwire/spark/reloaders/stimulus_reloader.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,18 @@ export class StimulusReloader {
log("Reload Stimulus controllers...")

this.application.stop()

await this.#reloadChangedStimulusControllers()
this.#unloadDeletedStimulusControllers()

await this.#reloadStimulusControllers()
this.application.start()
}

async #reloadChangedStimulusControllers() {
async #reloadStimulusControllers() {
await Promise.all(
this.#stimulusControllerPathsToReload.map(async moduleName => this.#reloadStimulusController(moduleName))
this.#stimulusControllerPaths.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"))
return Object.keys(this.#stimulusPathsByModule).filter(path => path.endsWith("_controller") && this.#shouldReloadController(path))
}

#shouldReloadController(path) {
Expand Down Expand Up @@ -65,22 +57,6 @@ 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]
}
Expand All @@ -94,12 +70,7 @@ export class StimulusReloader {
}

#registerController(name, module) {
this.#deregisterController(name)
this.application.register(name, module.default)
}

#deregisterController(name) {
log(`\tRemoving controller ${name}`)
this.application.unload(name)
this.application.register(name, module.default)
}
}
2 changes: 0 additions & 2 deletions test/dummy/app/javascript/controllers/dummy_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@ 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")
}
}
28 changes: 5 additions & 23 deletions test/helpers/files_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ module FilesHelper
ORIGINAL_EXTENSION = ".original"

def edit_file(path, replace:, with:)
path = expand_path(path)
path = Rails.application.root.join(path).to_s

raise ArgumentError, "File at '#{path}' does not exist." unless File.exist?(path)

original_path = remember_original_path_to_restore path

FileUtils.cp path, original_path
original_path = "#{path}#{ORIGINAL_EXTENSION}"
remember_path_to_restore original_path
system "cp", path, original_path

content = File.read(path)
updated_content = content.gsub(replace, with)
Expand All @@ -27,7 +27,7 @@ def edit_file(path, replace:, with:)
end

def add_file(path, content)
path = expand_path(path)
path = Rails.application.root.join(path).to_s

raise ArgumentError, "File at '#{path}' already exists." if File.exist?(path)

Expand All @@ -37,25 +37,7 @@ 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
Expand Down
14 changes: 2 additions & 12 deletions test/stimulus_reload_test.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
require "application_system_test_case"

class StimulusReloadTest < ApplicationSystemTestCase
setup do
visit root_path
end

test "reload Stimulus controller changes" do
visit root_path
assert_no_text "This was replaced!"

edit_file "app/javascript/controllers/dummy_controller.js", replace: "_REPLACE_", with: "This was replaced!"
Expand All @@ -14,6 +11,7 @@ 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"
Expand All @@ -31,12 +29,4 @@ 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
Loading