Fix autoload issue. Replace require/load with require_dependency.

[bitberry-dev]

Hello!

I developed the best practices for overriding solidus and found bug of the Order class autoload. It simply did not reload In development mode. After look at the sources it became clear that the problem in this two require (https://github.com/solidusio/solidus/blob/master/core/app/models/spree/order.rb#L3).

When you use require for loading constants rails can't mark them as autoloaded (for more information check https://guides.rubyonrails.org/autoloading_and_reloading_constants.html#autoloading-and-require).

I completely removed the require in order.rb, it is not needed there, although it could be replaced with require_dependency, but this is unnecessary. Also changed install generator because require_dependency is autoload friendly equivalent to
Rails.configuration.cache_classes ? require(c) : load(c).

As an example of issue, I have two files
models/spree/order_decorator.rb

Spree::Order.prepend(Spree::OrderExtension::Base)

models/spree/order_extension/base.rb

module Spree
  module OrderExtension
    module Base
      def self.prepended(base)
        # add callback or anything else
      end
    end
  end
end

Аfter few reloads you get an invalid ancestors chain

This can lead to adding 10 callbacks instead of one or something like that :)

Other constants are autoloading correctly.

[elia]

❤️ This is good practice, I guess the habit of using an explicit check for load vs. require started when either Rails was not reliable, require_dependency wasn't very well known, or Rails didn't have the feature at all.

This should be the advised approach, but for existing projects changing it could lead to odd behaviour, so it might be worth adding a note to the changelog / upgrade notes.

[bitberry-dev]

I would also suggest to stop using the *_decorator*.rb pattern for preloading overrides.
First, technically in these files are not decorators, rather patches/overrides.
Secondly, when you use gem draper rails preloads draper's decorators too. This is unlikely to lead to problems, but it does not look pretty :)

I suggest using *_override*.rb

[kennyadsl]

Thanks @bitberryru, this makes total sense and I did see the need of this in many projects.

I'm just wondering what happens with this Pull rails dependency out of core into solidus_rails PR that we want to merge before the 3.0 release. Will we need to change something there as well in your opinion?

For the _decorator "convention" change, I also totally agree. Not sure if we can just change this in the documentation and extensions. It should not break anything inside users stores since they we are not going to change their application.rb and if they have the previous convention in both config and directory structure they are safe. I think it's just matter of choosing the new pattern. I'll open an issue about it.

[bitberry-dev]

I'm just wondering what happens with this Pull rails dependency out of core into solidus_rails PR that we want to merge before the 3.0 release. Will we need to change something there as well in your opinion?

Autoloading provided by ActiveSupport and it definitely should be used in new solidus_core gem (without rails).

I think it's just matter of choosing the new pattern. I'll open an issue about it.

Yep, It is actual only for new projects. Thanks :)

[jacobherrington]

Thanks! Good points as well.