My take on Refactoring Rails' initializers

                August 11, 2024

            My take on Refactoring Rails' initializers

            Last week, I setup a refactoring challenge for a file in Rails' source code. You can read that post here.
There were no submissions from other people, so we'll just be looking at my take.
The original Rails initializable.rb
Here's the original initializable.rb from Rails.
require "tsort"

module Rails
  module Initializable
    def self.included(base) # :nodoc:
      base.extend ClassMethods
    end

    class Initializer
      attr_reader :name, :block

      def initialize(name, context, options, &block)
        options[:group] ||= :default
        @name, @context, @options, @block = name, context, options, block
      end

      def before
        @options[:before]
      end

      def after
        @options[:after]
      end

      def belongs_to?(group)
        @options[:group] == group || @options[:group] == :all
      end

      def run(*args)
        @context.instance_exec(*args, &block)
      end

      def bind(context)
        return self if @context
        Initializer.new(@name, context, @options, &block)
      end

      def context_class
        @context.class
      end
    end

    class Collection < Array
      include TSort

      alias :tsort_each_node :each
      def tsort_each_child(initializer, &block)
        select { |i| i.before == initializer.name || i.name == initializer.after }.each(&block)
      end

      def +(other)
        Collection.new(to_a + other.to_a)
      end
    end

    def run_initializers(group = :default, *args)
      return if instance_variable_defined?(:@ran)
      initializers.tsort_each do |initializer|
        initializer.run(*args) if initializer.belongs_to?(group)
      end
      @ran = true
    end

    def initializers
      @initializers ||= self.class.initializers_for(self)
    end

    module ClassMethods
      def initializers
        @initializers ||= Collection.new
      end

      def initializers_chain
        initializers = Collection.new
        ancestors.reverse_each do |klass|
          next unless klass.respond_to?(:initializers)
          initializers = initializers + klass.initializers
        end
        initializers
      end

      def initializers_for(binding)
        Collection.new(initializers_chain.map { |i| i.bind(binding) })
      end

      def initializer(name, opts = {}, &blk)
        raise ArgumentError, "A block must be passed when defining an initializer" unless blk
        opts[:after] ||= initializers.last.name unless initializers.empty? || initializers.find { |i| i.name == opts[:before] }
        initializers << Initializer.new(name, nil, opts, &blk)
      end
    end
  end
end

What stands out about this file's implementation?

Immediately, the klass.extend ClassMethods is a clue that we could be we using an ActiveSupport::Concern, since this is in Rails, to express the same idea more clearly.

Part of refactoring is restating your boilerplate or homegrown code with a more well-known/named concept.

We've got a custom Initializer class yet we're still mucking around with an internal @options hash and adding attr_reader-like methods for those.

We're doing an Array subclass for this relatively short file that seems fairly self-contained. There seems to be a bunch of related complexity, of packing and unpacking into this Collection class including the + override, stemming from this design choice. This class is just to get access to tsort_each, so I wonder if there's another way to do that?

The bind and returning a new Initializer looks like some old-school Ruby-vernacular that I don't quite understand why we're doing this. I wonder if there's a simpler way?

My therapeutic refactoring of initializable.rb
Originally, I explored refactoring this file 2 years ago. You can see the gist here.
I think I spent 2 hours on it? I don't quite remember.
Here's what I ended up with, you can see it tries to address my concerns with the code above.
require "tsort"

module Rails
  module Initializable
    extend ActiveSupport::Concern

    def run_initializers(group = :default, *arguments)
      @ran ||= true.tap do
        Initializer.tsort(initializers).each { instance_exec(*arguments, &_1.block) if _1.in?(group) }
      end
    end

    def initializers
      self.class.ancestors.reverse.flat_map { _1.initializers if _1.respond_to?(:initializers) }
    end

    class_methods do
      def initializer(name, group: :default, before: nil, after: nil, &block)
        raise ArgumentError, "A block must be passed when defining an initializer" unless block

        after ||= initializers.last&.name unless initializers.any? { _1.name == before }
        initializers << Initializer.new(name:, block:, group:, before:, after:)
      end

      def initializers = @initializers ||= []
    end

    class Initializer < Data.define(:name, :block, :group, :before, :after)
      def initialize(name:, block:, group: :default, before: nil, after: nil) = super

      def self.tsort(initializers)
        TSort.tsort initializers.to_enum,
          ->(initializer, &block) { initializer.pluck(initializers).each(&block) }
      end

      def pluck(initializers)
        initializers.select { _1.before == name || after == _1.name }
      end

      # Alternative to pluck above to try to name, but the original inner select makes no sense to me.
      def around?(other) = self < other || self > other
      def >(other) = other.before == name
      def <(other) = after == other.name

      def in?(group)
        self.group == group || self.group == :all
      end
    end
  end
end

To best understand this code and compare, I recommend retyping it manually (you'd be surprised!) or popping open a console to try play around with bits of it.

Let me know how you liked this and what stood out to you.
Do you feel more comfortable attempting a therapeutic refactoring in your app or in Rails itself?

                Don't miss what's next. Subscribe to kaspth:

My take on Refactoring Rails' initializers

The original Rails `initializable.rb`

What stands out about this file's implementation?

My therapeutic refactoring of `initializable.rb`