Skip to content

Commit

Permalink
Remove Zeitwerk and rely on Unreloader only
Browse files Browse the repository at this point in the history
As it turns out, Clover was started almost on the same day Unreloader
got a release that implemented autoload functionality.  Had it been
around a bit longer, maybe I would have done things like this to begin
with.

As it turns out, we had some issues with Zeitwerk being a bit too
clever -- or prescriptive -- about how it handled constants, and as a
result, code reloading didn't quite work right.  Now, this does:

    Unreloader.reload!

Also somewhat less idiomatic to Unreloader vs. Zeitwerk is setting up
autoload, and subsequently forcing them to load in production cases.
Doing things this way frees Clover code from having to require
dependencies within the project manually: otherwise, when
Unreloader.autoload is switched to a "require" when `autoload: false`
is passed, missing constant dependencies between files can crash.  I
considered whether we should abide the old Ruby ways and maintain a
correct -- and, more difficult still, minimal -- require order
embedded into each file, but decided the Zeitwerk style seemed more
practical.

Alternatively, I considered removing Unreloader in favor of Zeitwerk,
but that seemed unappealing: the in-place reloading funcionality it
was designed for with hash_branch and Roda is too good to give up.
See jeremyevans/roda-sequel-stack#21.

And Unreloader has some more advanced features that improve the
quality of reloading if one is willing to write bespoke code to help
Unreloader, which we are.

One piece of errata: Zeitwerk is good about figuring out if a
namespace should be a class or a module given files like this:

    a/dir/foo.rb
    a/dir.rb

In this case, Zeitwerk will notice that A::Dir should be a class, not
a module.  The code I wrote here is not so smart: if the list of files
is presented with a nested constant first, it'll create modules to
contain it, even if later a class is found that should define the
namespace.

But right now, we don't have this ambiguity, so I figure we can solve
it later as necessary.
  • Loading branch information
fdr committed Jun 10, 2023
1 parent 2ef6c1d commit 998ef05
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 48 deletions.
1 change: 0 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ gem "sequel", ">= 5.62"
gem "sequel_pg", ">= 1.8", require: "sequel"
gem "rack-unreloader", ">= 1.8"
gem "rake"
gem "zeitwerk"
gem "warning"
gem "pry"

Expand Down
2 changes: 0 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,6 @@ GEM
webrick (1.8.1)
xpath (3.2.0)
nokogiri (~> 1.8)
zeitwerk (2.6.8)

PLATFORMS
arm64-darwin-22
Expand Down Expand Up @@ -270,7 +269,6 @@ DEPENDENCIES
standard (>= 1.24.3)
tilt (>= 2.0.9)
warning
zeitwerk

RUBY VERSION
ruby 3.2.2p53
Expand Down
21 changes: 9 additions & 12 deletions clover.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,6 @@

require_relative "model"

unless defined?(Unreloader)
require "rack/unreloader"
Unreloader = Rack::Unreloader.new(reload: false, autoload: !ENV["NO_AUTOLOAD"])
end

require "mail"
require "roda"
require "tilt/sass"
Expand Down Expand Up @@ -146,14 +141,16 @@ def self.freeze
cookie_options: {secure: !%w[test development].include?(ENV["RACK_ENV"])},
secret: Config.clover_session_secret

if Unreloader.autoload?
plugin :autoload_hash_branches
autoload_hash_branch_dir("./routes")
end
# YYY: It'd be nice to use autoload, but it can't work while
# constants used across files are defined inside routes files and
# the autoload dependency cannot be tracked cheaply.
#
# if Unreloader.autoload?
# plugin :autoload_hash_branches
# autoload_hash_branch_dir("./routes")
# end

# rubocop:disable Performance/StringIdentifierArgument
Unreloader.autoload("routes", delete_hook: proc { |f| hash_branch(File.basename(f).delete_suffix(".rb")) }) {}
# rubocop:enable Performance/StringIdentifierArgument
Unreloader.require("routes", delete_hook: proc { |f| hash_branch(File.basename(f).delete_suffix(".rb")) }) {}

plugin :rodauth do
enable :argon2, :change_login, :change_password, :close_account, :create_account,
Expand Down
13 changes: 1 addition & 12 deletions config.ru
Original file line number Diff line number Diff line change
@@ -1,19 +1,8 @@
# frozen_string_literal: true

dev = ENV["RACK_ENV"] == "development"

if dev
require "logger"
logger = Logger.new($stdout)
end

require_relative "loader"

require "rack/unreloader"
Unreloader = Rack::Unreloader.new(subclasses: %w[Roda Sequel::Model], logger: logger, reload: dev) { Clover }
require_relative "model"
Unreloader.require("clover.rb") { "Clover" }
run(dev ? Unreloader : Clover.freeze.app)
run(Config.development? ? Unreloader : Clover.freeze.app)

freeze_core = false
# freeze_core = !dev # Uncomment to enable refrigerator
Expand Down
81 changes: 69 additions & 12 deletions loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,72 @@

require "bundler/setup"
Bundler.setup
require "zeitwerk"
Loader = Zeitwerk::Loader.new
Loader.push_dir("#{__dir__}/")
Loader.push_dir("#{__dir__}/model")
Loader.push_dir("#{__dir__}/lib")
Loader.ignore("#{__dir__}/routes")
Loader.ignore("#{__dir__}/migrate")
Loader.ignore("#{__dir__}/spec")
Loader.ignore("#{__dir__}/model.rb")
Loader.inflector.inflect("db" => "DB")
Loader.enable_reloading
Loader.setup

require_relative "./lib/casting_config_helpers"
require_relative "./config"

require "rack/unreloader"

Unreloader = Rack::Unreloader.new(
reload: Config.development?,
autoload: true,
logger: if Config.development?
require "logger"
Logger.new($stdout)
end
) { Clover }

Unreloader.autoload("#{__dir__}/clover.rb") { "Clover" }
Unreloader.autoload("#{__dir__}/db.rb") { "DB" }

def camelize s
s.gsub(/\/(.?)/) { |x| "::#{x[-1..].upcase}" }.gsub(/(^|_)(.)/) { |x| x[-1..].upcase }
end

AUTOLOAD_CONSTANTS = []

# Set up autoloads using Unreloader using a style much like Zeitwerk:
# directories are modules, file names are classes.
def autoload_normal(subdirectory, include_first: false)
prefix = File.join(__dir__, subdirectory)
rgx = Regexp.new('\A' + Regexp.escape(prefix + "/") + '(.*)\.rb\z')
last_namespace = nil

Unreloader.autoload(prefix) do |f|
full_name = camelize((include_first ? subdirectory + File::SEPARATOR : "") + rgx.match(f)[1])
parts = full_name.split("::")
namespace = parts[0..-2].freeze

# Skip namespace traversal if the last namespace handled has the
# same components, forming a fast-path that works well when output
# is the result of a depth-first traversal of the file system, as
# is normally the case.
unless namespace == last_namespace
scope = Object
namespace.each { |nested|
scope = if scope.const_defined?(nested, false)
scope.const_get(nested, false)
else
Module.new.tap { scope.const_set(nested, _1) }
end
}
last_namespace = namespace
end

# Reloading re-executes this block, which will crash on the
# subsequently frozen AUTOLOAD_CONSTANTS. It's also undesirable
# to have re-additions to the array.
AUTOLOAD_CONSTANTS << full_name unless AUTOLOAD_CONSTANTS.frozen?

full_name
end
end

%w[model lib].each { autoload_normal(_1) }
%w[scheduling prog].each { autoload_normal(_1, include_first: true) }

AUTOLOAD_CONSTANTS.freeze

if Config.production?
AUTOLOAD_CONSTANTS.each { Object.const_get(_1) }
end
9 changes: 0 additions & 9 deletions model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,6 @@ def semaphore(*names)
end
end

if ENV["RACK_ENV"] == "development"
unless defined?(Unreloader)
require "rack/unreloader"
Unreloader = Rack::Unreloader.new(reload: false)
end

Unreloader.require("model") { |f| Sequel::Model.send(:camelize, File.basename(f).delete_suffix(".rb")) }
end

if ENV["RACK_ENV"] == "development" || ENV["RACK_ENV"] == "test"
require "logger"
LOGGER = Logger.new($stdout)
Expand Down

0 comments on commit 998ef05

Please sign in to comment.