From 3ce59f714eab8bdaacb4ef35dad639ac2b08a4b0 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 | 38 +++++++ .../solidus/install/install_generator.rb | 11 --- guides/data/nav/developers.yml | 4 +- .../customizations/decorators.html.md | 4 + .../customizations/monkey_patches.html.md | 99 +++++++++++++++++++ 5 files changed, 143 insertions(+), 13 deletions(-) create mode 100644 guides/source/developers/customizations/monkey_patches.html.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 3099a73e27d..e7b9460e497 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,44 @@ - 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 +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 may also want to stop using the `decorator` naming, as it's no longer part +of Solidus recommendations (that files are monkey patches; they don't use the +[decorator pattern](https://en.wikipedia.org/wiki/Decorator_pattern)). E.g., +you can place those files in `app/overrides/` and remove the `decorator` +suffix. + ### 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 diff --git a/guides/data/nav/developers.yml b/guides/data/nav/developers.yml index e5fdf378bcc..53290b0bd1d 100644 --- a/guides/data/nav/developers.yml +++ b/guides/data/nav/developers.yml @@ -48,8 +48,8 @@ href: "/developers/customizations/customizing-storefront.html" - title: "Customizing the Admin Panel" href: "/developers/customizations/customizing-admin.html" - - title: "Decorators" - href: "/developers/customizations/decorators.html" + - title: "Monkey Patches" + href: "/developers/customizations/monkey_patches.html" - title: "State Machines" href: "/developers/customizations/state-machines.html" - title: "Customizing Assets" diff --git a/guides/source/developers/customizations/decorators.html.md b/guides/source/developers/customizations/decorators.html.md index caa9778836e..2e4243685ef 100644 --- a/guides/source/developers/customizations/decorators.html.md +++ b/guides/source/developers/customizations/decorators.html.md @@ -1,5 +1,8 @@ # Decorators +> Automatic autoloading will only work for Solidus versions minor than 3.2. +> Take a look at [monkey patches][monkey-patches] for the current recommended approach. + Solidus autoloads any file in the `/app` directory that has the suffix `_decorator.rb`, just like any other Rails models or controllers. This allows you to [monkey patch][monkey-patch] Solidus functionality for your store. @@ -25,6 +28,7 @@ method. With the code above live on your server, every call to `Spree::Order.total` will return the original total plus $10 (or whatever your currency is). +[monkey-patches]: monkey_patches.html [monkey-patch]: https://en.wikipedia.org/wiki/Monkey_patch ## Using class-level methods in decorators diff --git a/guides/source/developers/customizations/monkey_patches.html.md b/guides/source/developers/customizations/monkey_patches.html.md new file mode 100644 index 00000000000..6eca675aa88 --- /dev/null +++ b/guides/source/developers/customizations/monkey_patches.html.md @@ -0,0 +1,99 @@ +# Monkey patches + +> If you're using a Solidus version minor than 3.2, the content on this page is +> still applicable. However, you might want to look at the previously recommended +> approach through [decorators][decorators]. + +You can take advantage of Ruby's meta-programming features to [monkey +patch][monkey-patch] Solidus functionality for your store. + +As the first thing, you need to configure a directory where you'll place your +custom code. For instance, `app/overrides/`: + +```ruby +# config/application.rb +module MyStore + class Application < Rails::Application + # ... + overrides = "#{Rails.root}/app/overrides" + Rails.autoloaders.main.ignore(overrides) + config.to_prepare do + Dir.glob("#{overrides}/**/*.rb").each do |override| + load override + end + end + end +end +``` + +> If you're using the classic autoloader (the default before Rails 6), you +instead need to go with: +> +> ```ruby +> # config/application.rb +> module MyStore +> class Application < Rails::Application +> # ... +> config.to_prepare do +> Dir.glob("#{Rails.root}/app/overrides/**/*.rb").each do |override| +> require_dependency override +> end +> end +> end +> end +> ``` + +For example, if you want to add a method to the `Spree::Order` model, you could +create `/app/overrides/my_store/order_total_modifier.rb` with the following contents: + +```ruby +module MyStore::OrderTotalModifier + def total + super + BigDecimal(10.0) + end + + Spree::Order.prepend self +end +``` + +This creates a new module called `MyStore::OrderTotalModifier` that prepends +its methods early in the method lookup chain. So, for method calls on +`Spree::Order` objects, the monkey patch's `total` method would override the +original `total` method. + +With the code above live on your server, every call to `Spree::Order.total` will +return the original total plus $10 (or whatever your currency is). + +[monkey-patch]: https://en.wikipedia.org/wiki/Monkey_patch +[decorators]: decorators.html + +## Using class-level methods + +You'll need to define a special method in order to access some class-level +methods + +```ruby +module MyStore::ProductAdditions + + # This is the place to define custom associations, delegations, scopes and + # other ActiveRecord stuff + def self.prepended(base) + base.has_many :comments, dependent: :destroy + base.scope :sellable, -> { base.where(...).order(...) } + base.delegate :something, to: :something + end + + ... + + Spree::Product.prepend self +end +``` + +In this example, we're extending the functionality of `Spree::Product`. We +include an ActiveRecord association, scope, and delegation. + +## Monkey patches and Solidus upgrades + +Monkey patches can complicate your Solidus upgrades. If you depend on them, +[ensure](ensure) that you test them before upgrading in a production environment. Note +that Solidus's core classes may change with each release.