From ede67ae6df028752211b5b6d001d7c31fbf2e12c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Busqu=C3=A9?= Date: Mon, 27 Dec 2021 13:07:30 +0100 Subject: [PATCH] Stop generating code to autoload overrides Up to this PR, when installing Solidus, we're adding code to the host application to load files matching `app/**/*_decorator*.rb`. There exist a series of issues about that: - It goes against current recommendations on the Rails guides, where overrides are placed under `app/overrides/**/_*override.rb` ( see https://guides.rubyonrails.org/engines.html#improving-engine-functionality). - The `_decorator.rb` suffix is misleading, as that kind of overrides has to be seen as monkey patches and not related to the decorator pattern (see https://en.wikipedia.org/wiki/Decorator_pattern). - Although it's something needed a lot of times, monkey patching core classes should be seen as a last resort after ruling out other strategies (like configuring a custom class for some behavior). As such, we should not encourage it. - It may conflict with the naming used by other gems, like Draper (https://github.com/drapergem/draper). For all of that, it's something that we should address in the docs. This change is backward compatible, as the running code doesn't belong to the Solidus engine but to the previously generated applications, which will keep it after upgrading. The deleted code is, in fact, intended to work with the classic autoloader (removed on Rails 7). However, it still works because of https://github.com/rails/rails/blob/296ef7a17221e81881e38b51aa2b014d7a28bac5/activesupport/lib/active_support/dependencies/require_dependency.rb, which is deprecated. We add a notice to the CHANGELOG to let users known about that. On a related information, the recommended `app/overrides/` directory is the same used by `deface` (https://github.com/spree/deface), a common dependency in Solidus projects. However, both types of overrides can coexist without interfering with each other (see https://github.com/solidusio/solidus/issues/3010#issuecomment-844252252). We decided to tackle the issue upstream on `deface` (see https://github.com/solidusio/solidus/issues/3010#issuecomment-845034428). Closes #3010 --- CHANGELOG.md | 34 +++++++++++++++++++ .../solidus/install/install_generator.rb | 11 ------ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3099a73e27d..8f21aaf1a50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,40 @@ - Fix CSRF forgery protection bypass for Spree::OrdersController#populate [GHSA-h3fg-h5v3-vf8m](https://github.com/solidusio/solidus/security/advisories/GHSA-h3fg-h5v3-vf8m) +**Other important changes** + +New Solidus applications won't autoload files matching `app/**/*_decorator*.rb` +pattern anymore. For previous Solidus applications, it's something that will +keep working as the responsible code was added to your `config/application.rb` +when Solidus was installed. That code is intended to work with Rails' classic +autoloader, deprecated on Rails 6 and removed on Rails 7. It keeps working +because of a [compatibility +layer](https://github.com/rails/rails/blob/296ef7a17221e81881e38b51aa2b014d7a28bac5/activesupport/lib/active_support/dependencies/require_dependency.rb) +which is also deprecated. However, it may be eventually removed, so you're +better off updating your `application.rb` file. You should substitute: + +```ruby +config.to_prepare do + Dir.glob(Rails.root.join('app/**/*_decorator*.rb')) do |path| + require_dependency(path) + end +``` + +With: + +```ruby +overrides = "#{Rails.root}/app/overrides" # use your actual directory here +Rails.autoloaders.main.ignore(overrides) +config.to_prepare do + Dir.glob("#{overrides}/**/*_decorator*.rb").each do |override| + load override + end +end +``` + +You might also want to adopt the [current Rails recommendation but +overrides](https://guides.rubyonrails.org/engines.html#improving-engine-functionality). + ### Core - Add configuration option for `migration_path` [#4190](https://github.com/solidusio/solidus/pull/4190) ([SuperGoodSoft](https://github.com/supergoodsoft/)) diff --git a/core/lib/generators/solidus/install/install_generator.rb b/core/lib/generators/solidus/install/install_generator.rb index 9836abfd662..4a2157b4eda 100644 --- a/core/lib/generators/solidus/install/install_generator.rb +++ b/core/lib/generators/solidus/install/install_generator.rb @@ -103,17 +103,6 @@ def create_overrides_directory end def configure_application - application <<-RUBY - # Load application's model / class decorators - initializer 'spree.decorators' do |app| - config.to_prepare do - Dir.glob(Rails.root.join('app/**/*_decorator*.rb')) do |path| - require_dependency(path) - end - end - end - RUBY - if !options[:enforce_available_locales].nil? application <<-RUBY # Prevent this deprecation message: https://github.com/svenfuchs/i18n/commit/3b6e56e